From 7010e54ed6c4b727b50d62ba8cf9a86d6361a595 Mon Sep 17 00:00:00 2001 From: Wilko Nienhaus Date: Fri, 20 Aug 2021 14:28:24 +0300 Subject: [PATCH 1/2] esp32: fix negative immediate offset handling for relative jumps (jumpr, jumps) When providing negative immediate offset (step) values to the JUMPR and JUMPS opcodes, the resulting instruction contained an incorrect offset. This commit fixes that behaviour. According to the technical reference manual (TRM) [https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf] the magnitude of the relative shift from the current position is determined by bit 0-6 of the step field, and the direction is determined by bit 7, with 0 meaning PC+step, and 1 meaning PC-step. (For comparion, the ULP C macros in the ESP-IDF implement this as described in the TRM. All step values passed to the relevant JUMP macros will result in the instruction step field having bit 7 indicating the sign and bit 0-6 holding the absolute value of the offset.) This fix modifies the I_JUMP_REL{R,S} macros to set the step field correctly for negative immediate values. Since symbols, which are resolved later during the BFD relocation phase always evaluate to 0 at this stage (from EXPR_VALUE), this change to the macro only affects the case of immediate values (as can be seen from all previous test cases resulting in the same listing output as before). The relocation code (in function esp32ulp_jumprelr_reloc) already did this correctly for symbols, and thus remains unchanged. Example of the issue: For an offset of -2, the step field should have looked as follows: bit 0-6 = 2 # positive 2 bit 7 = 1 # 1 means negative However, the result was actually: bit 0-6 = 126 # negative 2 (two's compliment) bit 7 = 1 # 1 means negative --- .../esp32ulp/esp32/compare/esp32ulp_all.bin | Bin 5468 -> 5480 bytes .../esp32ulp/esp32/compare/esp32ulp_all.lst | 7 +++++++ .../esp32ulp/esp32/compare/esp32ulp_jumpr.bin | Bin 4932 -> 4956 bytes .../esp32ulp/esp32/compare/esp32ulp_jumpr.lst | 6 ++++++ .../gas/esp32ulp/esp32/esp32ulp_all.s | 6 ++++++ .../gas/esp32ulp/esp32/esp32ulp_jumpr.s | 5 +++++ include/opcode/esp32ulp_esp32.h | 4 ++-- 7 files changed, 26 insertions(+), 2 deletions(-) diff --git a/gas/testsuite/gas/esp32ulp/esp32/compare/esp32ulp_all.bin b/gas/testsuite/gas/esp32ulp/esp32/compare/esp32ulp_all.bin index 6842dd8233d5f36f287377913ecdfa876069d2ae..534d875bd18558b23e82660bed92c66a6ec6810d 100755 GIT binary patch delta 175 zcmcbk^+Icc0%OHQMP=CxMg|5DW@P{Z0R{#jlOaP8iZ>>j3kWW9Z7AYoVaVVVV7MU2 z&``8#@=AftJR%I7%_0meEg}rut&?K}br>@ycM8e_$(4fsj9HU^0@*o}C57r43ntGL zvKK4>8VLdd3``8Wfpo#>j3kVjuHWYEPFl2BFFkBF1 zXeer$yi#B@s}@5wL-oW)`N=(k9E>rOrwYmg$*qF^jB%4$h2$9%CaVh7Gp0;lDr7I1 x0yGW;1Q?hYb_3~@&0mG~GlG= + 185 + 186 // JUMPS negative offset + 187 0160 14000484 JUMPS -16, 20, EQ //Jump to address “position - 16” if value in S + 187 14000B85 diff --git a/gas/testsuite/gas/esp32ulp/esp32/compare/esp32ulp_jumpr.bin b/gas/testsuite/gas/esp32ulp/esp32/compare/esp32ulp_jumpr.bin index 197b044a46c33b99a902f24ae6855dc5339653d7..0a7d208ce48fe63051983b2c39f2497e79cbc252 100755 GIT binary patch delta 203 zcmX@2c1LZ31m`pX1_l!b28Nu8ipsK6fD9031!4xE1S5zKq``P&qO^b{3jKwLesQGW6q0S?9~la~s}1IeQT`GzxqT0ua7fr+6LNQ3MG0T6#8kVclD z4&rZ471U+~YMv-0&k3`a1;`Vgd`3usamM7kLh7tD1Q{6COg0o2o-89Q!00krS6CbX DopmN` delta 175 zcmcbkc0_H01ZSN91A_?z14GC}MP=C%-5D;KsV(0|YAag+g#GeSHk>#g@_?v45wHbk$FA51y-Xp}r e3bteNSs``ChRI*Sq^z(ytBw!@gZgAeVPOC;P#= 20 + +// JUMPS negative offset + JUMPS -16, 20, EQ //Jump to address “position - 16” if value in Stage_Cnt == 20 diff --git a/gas/testsuite/gas/esp32ulp/esp32/esp32ulp_jumpr.s b/gas/testsuite/gas/esp32ulp/esp32/esp32ulp_jumpr.s index 5ea2b4393c..8790313e1b 100644 --- a/gas/testsuite/gas/esp32ulp/esp32/esp32ulp_jumpr.s +++ b/gas/testsuite/gas/esp32ulp/esp32/esp32ulp_jumpr.s @@ -54,3 +54,8 @@ Check2: + JUMPR -0x04, 0x4, LT + JUMPR -0x04, 0x4, LE + JUMPR -0x04, 0x4, EQ + JUMPR -0x04, 0x4, GT + JUMPR -0x04, 0x4, GE diff --git a/include/opcode/esp32ulp_esp32.h b/include/opcode/esp32ulp_esp32.h index 7975948e55..2ebf9a67fc 100644 --- a/include/opcode/esp32ulp_esp32.h +++ b/include/opcode/esp32ulp_esp32.h @@ -110,7 +110,7 @@ typedef struct { #define I_JUMP_RELR(thresh, jud, stp) { *(unsigned int*)&(jump_alu_relr ){ \ .threshold = thresh, \ .judge = jud, \ - .step = stp, \ + .step = ((stp < 0) ? ((-stp) | (1 << 7)) : stp) , \ .sub_opcode = SUB_OPCODE_BR, \ .opcode = OPCODE_BRANCH } } @@ -129,7 +129,7 @@ typedef struct { #define I_JUMP_RELS(thresh, jud, stp) { *(unsigned int*)&(jump_alu_rels ){ \ .threshold = thresh, \ .judge = jud, \ - .step = stp, \ + .step = ((stp < 0) ? ((-stp) | (1 << 7)) : stp) , \ .sub_opcode = SUB_OPCODE_BS, \ .opcode = OPCODE_BRANCH } } From 249ec34cc2c9574a86f3f86bbb175a863f988bcf Mon Sep 17 00:00:00 2001 From: Wilko Nienhaus Date: Fri, 20 Aug 2021 14:30:28 +0300 Subject: [PATCH 2/2] esp32s2: fix negative immediate offset handling for relative jumps (jumpr, jumps) When providing negative immediate offset (step) values to the JUMPR and JUMPS opcodes, the resulting instruction contained an incorrect offset. This commit fixes that behaviour. This is the same issue that affected the ESP32 code. See previous commit for more technical detail on the issue. --- .../esp32s2/compare/esp32s2ulp_jump.bin | Bin 5064 -> 5112 bytes .../esp32s2/compare/esp32s2ulp_jump.lst | 15 +++++++++++++++ .../gas/esp32ulp/esp32s2/esp32s2ulp_jump.s | 13 +++++++++++++ include/opcode/esp32ulp_esp32s2.h | 4 ++-- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/gas/testsuite/gas/esp32ulp/esp32s2/compare/esp32s2ulp_jump.bin b/gas/testsuite/gas/esp32ulp/esp32s2/compare/esp32s2ulp_jump.bin index cc2275e62c5b11c6d5a27babd5933e3ff8cbbbb5..6a51da73f3e359da56d80078315223f688d12c19 100755 GIT binary patch delta 212 zcmX@1{zH9&1ec#61A_?z0|STfL?tCzA4UcS5M~96FbFU(0GSLvKowxTF>$wmAp-+j z69WSa5VJyQE+`vB3w1Fx2mrAl5QEslUDXpCIG^R6_#fOyJfPausUPFriAj}FBVGv+o05Tb5fGWUvW8!WB#_EZU z@{@A}IT&RomkP=Q$*F?zh6+F>ARxfN#83mIL56|=h~EOFk>$HU{LQLD@{B;uuEO%H bV6!J@3ac|JOr8iN1t#wVlItfw6cz^n%gz}s diff --git a/gas/testsuite/gas/esp32ulp/esp32s2/compare/esp32s2ulp_jump.lst b/gas/testsuite/gas/esp32ulp/esp32s2/compare/esp32s2ulp_jump.lst index 57dab6d461..d3cb015600 100644 --- a/gas/testsuite/gas/esp32ulp/esp32s2/compare/esp32s2ulp_jump.lst +++ b/gas/testsuite/gas/esp32ulp/esp32s2/compare/esp32s2ulp_jump.lst @@ -101,3 +101,18 @@ ESP32ULP GAS ./gas/testsuite/gas/esp32ulp/esp32s2/esp32s2ulp_jump.s page 1 83 0118 00800388 jumps 0x0, 0x00, GE 84 85 + 86 //jumpr with negative offset + 87 011c 00000682 jumpr -0x4, 0x00, EQ + 88 0120 00000482 jumpr -0x4, 0x00, LT + 89 0124 00000582 jumpr -0x4, 0x00, GT + 90 0128 00000482 jumpr -0x4, 0x00, LE + 90 00000A82 + 91 0130 00000582 jumpr -0x4, 0x00, GE + 91 00000A82 + 92 + 93 //jumps with negative offset + 94 0138 0000128A jumps -0x4, 0x00, EQ + 95 013c 0080108A jumps -0x4, 0x00, LT + 96 0140 0080118A jumps -0x4, 0x00, GT + 97 0144 0080128A jumps -0x4, 0x00, LE + 98 0148 0080138A jumps -0x4, 0x00, GE diff --git a/gas/testsuite/gas/esp32ulp/esp32s2/esp32s2ulp_jump.s b/gas/testsuite/gas/esp32ulp/esp32s2/esp32s2ulp_jump.s index 5d2f1ddd9c..62ef414d43 100644 --- a/gas/testsuite/gas/esp32ulp/esp32s2/esp32s2ulp_jump.s +++ b/gas/testsuite/gas/esp32ulp/esp32s2/esp32s2ulp_jump.s @@ -83,3 +83,16 @@ jmp2: nop jumps 0x0, 0x00, GE + //jumpr with negative offset + jumpr -0x4, 0x00, EQ + jumpr -0x4, 0x00, LT + jumpr -0x4, 0x00, GT + jumpr -0x4, 0x00, LE + jumpr -0x4, 0x00, GE + + //jumps with negative offset + jumps -0x4, 0x00, EQ + jumps -0x4, 0x00, LT + jumps -0x4, 0x00, GT + jumps -0x4, 0x00, LE + jumps -0x4, 0x00, GE diff --git a/include/opcode/esp32ulp_esp32s2.h b/include/opcode/esp32ulp_esp32s2.h index d7b453ee9b..3a17524f25 100644 --- a/include/opcode/esp32ulp_esp32s2.h +++ b/include/opcode/esp32ulp_esp32s2.h @@ -135,7 +135,7 @@ typedef struct { #define I_JUMP_RELR(thresh, jud, stp) { *(unsigned int*)&(jump_alu_relr ){ \ .threshold = thresh, \ .judge = jud, \ - .step = stp, \ + .step = ((stp < 0) ? ((-stp) | (1 << 7)) : stp) , \ .sub_opcode = SUB_OPCODE_BR, \ .opcode = OPCODE_BRANCH } } @@ -155,7 +155,7 @@ typedef struct { .threshold = thresh, \ .unused = 0, \ .judge = jud, \ - .step = stp, \ + .step = ((stp < 0) ? ((-stp) | (1 << 7)) : stp) , \ .sub_opcode = SUB_OPCODE_BS, \ .opcode = OPCODE_BRANCH } }