gh-106149: move unconditional jump direction resolution from optimize…

…r to assembler (#106291)
python · Jul 1, 2023 · 200f255 · 200f255
1 parent d3abc9b
commit 200f255
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 20 deletions.
diff --git a/Python/assemble.c b/Python/assemble.c
@@ -674,11 +674,45 @@ resolve_jump_offsets(instr_sequence *instrs)
     return SUCCESS;
 }
 
+static int
+resolve_unconditional_jumps(instr_sequence *instrs)
+{
+    /* Resolve directions of unconditional jumps */
+
+    for (int i = 0; i < instrs->s_used; i++) {
+        instruction *instr = &instrs->s_instrs[i];
+        bool is_forward = (instr->i_oparg > i);
+        switch(instr->i_opcode) {
+            case JUMP:
+                assert(SAME_OPCODE_METADATA(JUMP, JUMP_FORWARD));
+                assert(SAME_OPCODE_METADATA(JUMP, JUMP_BACKWARD));
+                instr->i_opcode = is_forward ? JUMP_FORWARD : JUMP_BACKWARD;
+                break;
+            case JUMP_NO_INTERRUPT:
+                assert(SAME_OPCODE_METADATA(JUMP_NO_INTERRUPT, JUMP_FORWARD));
+                assert(SAME_OPCODE_METADATA(JUMP_NO_INTERRUPT, JUMP_BACKWARD_NO_INTERRUPT));
+                instr->i_opcode = is_forward ?
+                    JUMP_FORWARD : JUMP_BACKWARD_NO_INTERRUPT;
+                break;
+            default:
+                if (OPCODE_HAS_JUMP(instr->i_opcode) &&
+                    IS_PSEUDO_INSTR(instr->i_opcode)) {
+                    Py_UNREACHABLE();
+                }
+        }
+    }
+    return SUCCESS;
+}
+
 PyCodeObject *
 _PyAssemble_MakeCodeObject(_PyCompile_CodeUnitMetadata *umd, PyObject *const_cache,
                            PyObject *consts, int maxdepth, instr_sequence *instrs,
                            int nlocalsplus, int code_flags, PyObject *filename)
 {
+
+    if (resolve_unconditional_jumps(instrs) < 0) {
+        return NULL;
+    }
     if (resolve_jump_offsets(instrs) < 0) {
         return NULL;
     }

diff --git a/Python/flowgraph.c b/Python/flowgraph.c
@@ -393,24 +393,17 @@ no_redundant_jumps(cfg_builder *g) {
 static int
 normalize_jumps_in_block(cfg_builder *g, basicblock *b) {
     cfg_instr *last = _PyCfg_BasicblockLastInstr(b);
-    if (last == NULL || !is_jump(last)) {
+    if (last == NULL || !is_jump(last) ||
+        IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
         return SUCCESS;
     }
     assert(!IS_ASSEMBLER_OPCODE(last->i_opcode));
+
     bool is_forward = last->i_target->b_visited == 0;
-    switch(last->i_opcode) {
-        case JUMP:
-            assert(SAME_OPCODE_METADATA(JUMP, JUMP_FORWARD));
-            assert(SAME_OPCODE_METADATA(JUMP, JUMP_BACKWARD));
-            last->i_opcode = is_forward ? JUMP_FORWARD : JUMP_BACKWARD;
-            return SUCCESS;
-        case JUMP_NO_INTERRUPT:
-            assert(SAME_OPCODE_METADATA(JUMP_NO_INTERRUPT, JUMP_FORWARD));
-            assert(SAME_OPCODE_METADATA(JUMP_NO_INTERRUPT, JUMP_BACKWARD_NO_INTERRUPT));
-            last->i_opcode = is_forward ?
-                JUMP_FORWARD : JUMP_BACKWARD_NO_INTERRUPT;
-            return SUCCESS;
+    if (is_forward) {
+        return SUCCESS;
     }
+
     int reversed_opcode = 0;
     switch(last->i_opcode) {
         case POP_JUMP_IF_NOT_NONE:
@@ -426,9 +419,6 @@ normalize_jumps_in_block(cfg_builder *g, basicblock *b) {
             reversed_opcode = POP_JUMP_IF_FALSE;
             break;
     }
-    if (is_forward) {
-        return SUCCESS;
-    }
     /* transform 'conditional jump T' to
      * 'reversed_jump b_next' followed by 'jump_backwards T'
      */

diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h
diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py
@@ -1310,10 +1310,10 @@ def write_metadata(self) -> None:
 
     def write_pseudo_instrs(self) -> None:
         """Write the IS_PSEUDO_INSTR macro"""
-        self.out.emit("\n\n#define IS_PSEUDO_INSTR(OP)  \\")
+        self.out.emit("\n\n#define IS_PSEUDO_INSTR(OP)  ( \\")
         for op in self.pseudos:
             self.out.emit(f"    ((OP) == {op}) || \\")
-        self.out.emit(f"    0")
+        self.out.emit(f"    0)")
 
     def write_uop_items(self, make_text: typing.Callable[[str, int], str]) -> None:
         """Write '#define XXX NNN' for each uop"""