gh-87092: reduce redundancy and repetition in compiler's optimization stage

Lib/test/test_dis.py

@@ -360,13 +360,13 @@ def bug42562():
     -->    BINARY_OP               11 (/)
            POP_TOP
 
-%3d     >> LOAD_FAST_CHECK          1 (tb)
+%3d        LOAD_FAST_CHECK          1 (tb)
            RETURN_VALUE
         >> PUSH_EXC_INFO
 
 %3d        LOAD_GLOBAL              0 (Exception)
            CHECK_EXC_MATCH
-           POP_JUMP_IF_FALSE       22 (to 80)
+           POP_JUMP_IF_FALSE       23 (to 82)
            STORE_FAST               0 (e)
 
 %3d        LOAD_FAST                0 (e)
@@ -376,7 +376,9 @@ def bug42562():
            LOAD_CONST               0 (None)
            STORE_FAST               0 (e)
            DELETE_FAST              0 (e)
-           JUMP_BACKWARD           29 (to 14)
+
+%3d        LOAD_FAST                1 (tb)
+           RETURN_VALUE
         >> LOAD_CONST               0 (None)
            STORE_FAST               0 (e)
            DELETE_FAST              0 (e)
@@ -394,6 +396,7 @@ def bug42562():
        TRACEBACK_CODE.co_firstlineno + 5,
        TRACEBACK_CODE.co_firstlineno + 3,
        TRACEBACK_CODE.co_firstlineno + 4,
+       TRACEBACK_CODE.co_firstlineno + 5,
        TRACEBACK_CODE.co_firstlineno + 3)
 
 def _fstring(a, b, c, d):
@@ -440,7 +443,7 @@ def _with(c):
            CALL                     2
            POP_TOP
 
-%3d     >> LOAD_CONST               2 (2)
+%3d        LOAD_CONST               2 (2)
            STORE_FAST               2 (y)
            LOAD_CONST               0 (None)
            RETURN_VALUE
@@ -453,7 +456,11 @@ def _with(c):
            POP_EXCEPT
            POP_TOP
            POP_TOP
-           JUMP_BACKWARD           13 (to 30)
+
+%3d        LOAD_CONST               2 (2)
+           STORE_FAST               2 (y)
+           LOAD_CONST               0 (None)
+           RETURN_VALUE
         >> COPY                     3
            POP_EXCEPT
            RERAISE                  1
@@ -465,6 +472,7 @@ def _with(c):
        _with.__code__.co_firstlineno + 1,
        _with.__code__.co_firstlineno + 3,
        _with.__code__.co_firstlineno + 1,
+       _with.__code__.co_firstlineno + 3,
        )
 
 async def _asyncwith(c):
@@ -502,7 +510,7 @@ async def _asyncwith(c):
            JUMP_BACKWARD_NO_INTERRUPT     4 (to 48)
         >> POP_TOP
 
-%3d     >> LOAD_CONST               2 (2)
+%3d        LOAD_CONST               2 (2)
            STORE_FAST               2 (y)
            LOAD_CONST               0 (None)
            RETURN_VALUE
@@ -526,7 +534,11 @@ async def _asyncwith(c):
            POP_EXCEPT
            POP_TOP
            POP_TOP
-           JUMP_BACKWARD           24 (to 58)
+
+%3d        LOAD_CONST               2 (2)
+           STORE_FAST               2 (y)
+           LOAD_CONST               0 (None)
+           RETURN_VALUE
         >> COPY                     3
            POP_EXCEPT
            RERAISE                  1
@@ -538,6 +550,7 @@ async def _asyncwith(c):
        _asyncwith.__code__.co_firstlineno + 1,
        _asyncwith.__code__.co_firstlineno + 3,
        _asyncwith.__code__.co_firstlineno + 1,
+       _asyncwith.__code__.co_firstlineno + 3,
        )
 
 

Lib/test/test_peepholer.py

@@ -344,8 +344,6 @@ def f(x):
         self.assertEqual(len(returns), 1)
         self.check_lnotab(f)
 
-    @unittest.skip("Following gh-92228 the return has two predecessors "
-                   "and that prevents jump elimination.")
     def test_elim_jump_to_return(self):
         # JUMP_FORWARD to RETURN -->  RETURN
         def f(cond, true_value, false_value):

Python/compile.c

@@ -498,7 +498,7 @@ static int compiler_match(struct compiler *, stmt_ty);
 static int compiler_pattern_subpattern(struct compiler *, pattern_ty,
                                        pattern_context *);
 
-static void clean_basic_block(basicblock *bb);
+static void remove_redundant_nops(basicblock *bb);
 
 static PyCodeObject *assemble(struct compiler *, int addNone);
 
@@ -7364,7 +7364,7 @@ mark_cold(basicblock *entryblock) {
         for (int i = 0; i < b->b_iused; i++) {
             struct instr *instr = &b->b_instr[i];
             if (is_jump(instr)) {
-                assert(i == b->b_iused-1);
+                assert(i == b->b_iused - 1);
                 basicblock *target = b->b_instr[i].i_target;
                 if (!target->b_warm && !target->b_visited) {
                     *sp++ = target;
@@ -8271,9 +8271,6 @@ dump_basicblock(const basicblock *b)
 #endif
 
 
-static int
-normalize_basic_block(basicblock *bb);
-
 static int
 calculate_jump_targets(basicblock *entryblock);
 
@@ -8325,7 +8322,7 @@ insert_instruction(basicblock *block, int pos, struct instr *instr) {
     if (basicblock_next_instr(block) < 0) {
         return -1;
     }
-    for (int i = block->b_iused-1; i > pos; i--) {
+    for (int i = block->b_iused - 1; i > pos; i--) {
         block->b_instr[i] = block->b_instr[i-1];
     }
     block->b_instr[pos] = *instr;
@@ -8488,23 +8485,32 @@ fix_cell_offsets(struct compiler *c, basicblock *entryblock, int *fixedmap)
 static void
 propagate_line_numbers(basicblock *entryblock);
 
-static void
-eliminate_empty_basic_blocks(cfg_builder *g);
-
 #ifndef NDEBUG
 static bool
 no_redundant_jumps(cfg_builder *g) {
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
         struct instr *last = basicblock_last_instr(b);
         if (last != NULL) {
-            if (last->i_opcode == JUMP || last->i_opcode == JUMP_NO_INTERRUPT) {
+            if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
                 assert(last->i_target != b->b_next);
-                return false;
+                if (last->i_target == b->b_next) {
+                    return false;
+                }
             }
         }
     }
     return true;
 }
+
+static bool
+no_empty_basic_blocks(cfg_builder *g) {
+    for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
+        if (b->b_iused == 0) {
+            return false;
+        }
+    }
+    return true;
+}
 #endif
 
 static int
@@ -8514,28 +8520,22 @@ remove_redundant_jumps(cfg_builder *g) {
      * of that jump. If it is, then the jump instruction is redundant and
      * can be deleted.
      */
-    int removed = 0;
+    assert(no_empty_basic_blocks(g));
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
         struct instr *last = basicblock_last_instr(b);
-        if (last != NULL) {
-            assert(!IS_ASSEMBLER_OPCODE(last->i_opcode));
-            if (last->i_opcode == JUMP ||
-                last->i_opcode == JUMP_NO_INTERRUPT) {
-                if (last->i_target == NULL) {
-                    PyErr_SetString(PyExc_SystemError, "jump with NULL target");
-                    return -1;
-                }
-                if (last->i_target == b->b_next) {
-                    assert(b->b_next->b_iused);
-                    last->i_opcode = NOP;
-                    removed++;
-                }
+        assert(last != NULL);
+        assert(!IS_ASSEMBLER_OPCODE(last->i_opcode));
+        if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
+            if (last->i_target == NULL) {
+                PyErr_SetString(PyExc_SystemError, "jump with NULL target");
+                return -1;
+            }
+            if (last->i_target == b->b_next) {
+                assert(b->b_next->b_iused);
+                last->i_opcode = NOP;
             }
         }
     }
-    if (removed) {
-        eliminate_empty_basic_blocks(g);
-    }
     return 0;
 }
 
@@ -8643,7 +8643,7 @@ assemble(struct compiler *c, int addNone)
         goto error;
     }
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        clean_basic_block(b);
+        remove_redundant_nops(b);
     }
 
     /* Order of basic blocks must have been determined by now */
@@ -9003,10 +9003,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
         int oparg = inst->i_oparg;
         int nextop = i+1 < bb->b_iused ? bb->b_instr[i+1].i_opcode : 0;
         if (HAS_TARGET(inst->i_opcode)) {
-            /* Skip over empty basic blocks. */
-            while (inst->i_target->b_iused == 0) {
-                inst->i_target = inst->i_target->b_next;
-            }
+            assert(inst->i_target->b_iused > 0);
             target = &inst->i_target->b_instr[0];
             assert(!IS_ASSEMBLER_OPCODE(target->i_opcode));
         }
@@ -9230,50 +9227,36 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
     return -1;
 }
 
-static bool
-basicblock_has_lineno(const basicblock *bb) {
-    for (int i = 0; i < bb->b_iused; i++) {
-        if (bb->b_instr[i].i_loc.lineno > 0) {
-            return true;
-        }
-    }
-    return false;
-}
-
-/* If this block ends with an unconditional jump to an exit block,
- * then remove the jump and extend this block with the target.
+/* If this block ends with an unconditional jump to a small exit block, then
+ * remove the jump and extend this block with the target.
+ * Returns 1 if extended, 0 if no change, and -1 on error.
  */
 static int
-extend_block(basicblock *bb) {
+inline_small_exit_blocks(basicblock *bb) {
     struct instr *last = basicblock_last_instr(bb);
     if (last == NULL) {
         return 0;
     }
-    if (last->i_opcode != JUMP &&
-        last->i_opcode != JUMP_FORWARD &&
-        last->i_opcode != JUMP_BACKWARD) {
+    if (!IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
         return 0;
     }
-    if (basicblock_exits_scope(last->i_target) && last->i_target->b_iused <= MAX_COPY_SIZE) {
-        basicblock *to_copy = last->i_target;
-        if (basicblock_has_lineno(to_copy)) {
-            /* copy only blocks without line number (like implicit 'return None's) */
-            return 0;
-        }
+    basicblock *target = last->i_target;
+    if (basicblock_exits_scope(target) && target->b_iused <= MAX_COPY_SIZE) {
         last->i_opcode = NOP;
-        for (int i = 0; i < to_copy->b_iused; i++) {
+        for (int i = 0; i < target->b_iused; i++) {
             int index = basicblock_next_instr(bb);
             if (index < 0) {
                 return -1;
             }
-            bb->b_instr[index] = to_copy->b_instr[i];
+            bb->b_instr[index] = target->b_instr[i];
         }
+        return 1;
     }
     return 0;
 }
 
 static void
-clean_basic_block(basicblock *bb) {
+remove_redundant_nops(basicblock *bb) {
     /* Remove NOPs when legal to do so. */
     int dest = 0;
     int prev_lineno = -1;
@@ -9324,24 +9307,17 @@ clean_basic_block(basicblock *bb) {
 }
 
 static int
-normalize_basic_block(basicblock *bb) {
-    /* Skip over empty blocks.
-     * Raise SystemError if jump or exit is not last instruction in the block. */
-    for (int i = 0; i < bb->b_iused; i++) {
-        int opcode = bb->b_instr[i].i_opcode;
-        assert(!IS_ASSEMBLER_OPCODE(opcode));
-        int is_jump = IS_JUMP_OPCODE(opcode);
-        int is_exit = IS_SCOPE_EXIT_OPCODE(opcode);
-        if (is_exit || is_jump) {
-            if (i != bb->b_iused-1) {
-                PyErr_SetString(PyExc_SystemError, "malformed control flow graph.");
-                return -1;
-            }
-        }
-        if (is_jump) {
-            /* Skip over empty basic blocks. */
-            while (bb->b_instr[i].i_target->b_iused == 0) {
-                bb->b_instr[i].i_target = bb->b_instr[i].i_target->b_next;
+check_cfg(cfg_builder *g) {
+    for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
+        /* Raise SystemError if jump or exit is not last instruction in the block. */
+        for (int i = 0; i < b->b_iused; i++) {
+            int opcode = b->b_instr[i].i_opcode;
+            assert(!IS_ASSEMBLER_OPCODE(opcode));
+            if (IS_TERMINATOR_OPCODE(opcode)) {
+                if (i != b->b_iused - 1) {
+                    PyErr_SetString(PyExc_SystemError, "malformed control flow graph.");
+                    return -1;
+                }
             }
         }
     }
@@ -9512,25 +9488,25 @@ static int
 optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
 {
     assert(PyDict_CheckExact(const_cache));
-    for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        if (normalize_basic_block(b)) {
-            return -1;
-        }
+    if (check_cfg(g) < 0) {
+        return -1;
     }
+    eliminate_empty_basic_blocks(g);
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        if (extend_block(b) < 0) {
+        if (inline_small_exit_blocks(b) < 0) {
             return -1;
         }
     }
+    assert(no_empty_basic_blocks(g));
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
         if (optimize_basic_block(const_cache, b, consts)) {
             return -1;
         }
-        clean_basic_block(b);
+        remove_redundant_nops(b);
         assert(b->b_predecessors == 0);
     }
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        if (extend_block(b) < 0) {
+        if (inline_small_exit_blocks(b) < 0) {
             return -1;
         }
     }
@@ -9545,7 +9521,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
     }
     eliminate_empty_basic_blocks(g);
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        clean_basic_block(b);
+        remove_redundant_nops(b);
     }
     if (remove_redundant_jumps(g) < 0) {
         return -1;
@@ -9627,12 +9603,9 @@ duplicate_exits_without_lineno(cfg_builder *g)
             }
         }
     }
-    /* Eliminate empty blocks */
-    for (basicblock *b = entryblock; b != NULL; b = b->b_next) {
-        while (b->b_next && b->b_next->b_iused == 0) {
-            b->b_next = b->b_next->b_next;
-        }
-    }
+
+    assert(no_empty_basic_blocks(g));
+
     /* Any remaining reachable exit blocks without line number can only be reached by
      * fall through, and thus can only have a single predecessor */
     for (basicblock *b = entryblock; b != NULL; b = b->b_next) {