gh-93678: apply remove_redundant_jumps in optimize_cfg (GH-96274)

python · Sep 1, 2022 · 894cafd · 894cafd
1 parent e93d1bd
commit 894cafd
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 14 deletions.
diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py
@@ -892,7 +892,8 @@ def test_conditional_jump_forward_non_const_condition(self):
         self.cfg_optimization_test(insts, expected, consts=list(range(5)))
 
     def test_conditional_jump_forward_const_condition(self):
-        # The unreachable branch of the jump is removed
+        # The unreachable branch of the jump is removed, the jump
+        # becomes redundant and is replaced by a NOP (for the lineno)
 
         insts = [
             ('LOAD_CONST', 3, 11),
@@ -903,8 +904,7 @@ def test_conditional_jump_forward_const_condition(self):
         ]
         expected = [
             ('NOP', None, 11),
-            ('JUMP', lbl := self.Label(), 12),
-            lbl,
+            ('NOP', None, 12),
             ('LOAD_CONST', '3', 14)
         ]
         self.cfg_optimization_test(insts, expected, consts=list(range(5)))

diff --git a/Python/compile.c b/Python/compile.c
@@ -7396,6 +7396,9 @@ mark_cold(basicblock *entryblock) {
     return 0;
 }
 
+static int
+remove_redundant_jumps(cfg_builder *g);
+
 static int
 push_cold_blocks_to_end(cfg_builder *g, int code_flags) {
     basicblock *entryblock = g->g_entryblock;
@@ -7465,6 +7468,12 @@ push_cold_blocks_to_end(cfg_builder *g, int code_flags) {
     }
     assert(b != NULL && b->b_next == NULL);
     b->b_next = cold_blocks;
+
+    if (cold_blocks != NULL) {
+        if (remove_redundant_jumps(g) < 0) {
+            return -1;
+        }
+    }
     return 0;
 }
 
@@ -8269,9 +8278,6 @@ trim_unused_consts(basicblock *entryblock, PyObject *consts);
 static int
 duplicate_exits_without_lineno(cfg_builder *g);
 
-static int
-extend_block(basicblock *bb);
-
 static int *
 build_cellfixedoffsets(struct compiler *c)
 {
@@ -8476,6 +8482,21 @@ 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) {
+                assert(last->i_target != b->b_next);
+                return false;
+            }
+        }
+    }
+    return true;
+}
+#endif
 
 static int
 remove_redundant_jumps(cfg_builder *g) {
@@ -8592,8 +8613,8 @@ assemble(struct compiler *c, int addNone)
     if (trim_unused_consts(g->g_entryblock, consts)) {
         goto error;
     }
-    if (duplicate_exits_without_lineno(g)) {
-        return NULL;
+    if (duplicate_exits_without_lineno(g) < 0) {
+        goto error;
     }
     propagate_line_numbers(g->g_entryblock);
     guarantee_lineno_for_exits(g->g_entryblock, c->u->u_firstlineno);
@@ -8612,10 +8633,6 @@ assemble(struct compiler *c, int addNone)
     if (push_cold_blocks_to_end(g, code_flags) < 0) {
         goto error;
     }
-
-    if (remove_redundant_jumps(g) < 0) {
-        goto error;
-    }
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
         clean_basic_block(b);
     }
@@ -8627,6 +8644,8 @@ assemble(struct compiler *c, int addNone)
         goto error;
     }
 
+    assert(no_redundant_jumps(g));
+
     /* Can't modify the bytecode after computing jump offsets. */
     assemble_jump_offsets(g->g_entryblock);
 
@@ -9488,7 +9507,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
         }
     }
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        if (extend_block(b)) {
+        if (extend_block(b) < 0) {
             return -1;
         }
     }
@@ -9500,7 +9519,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
         assert(b->b_predecessors == 0);
     }
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        if (extend_block(b)) {
+        if (extend_block(b) < 0) {
             return -1;
         }
     }
@@ -9517,6 +9536,9 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
     for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
         clean_basic_block(b);
     }
+    if (remove_redundant_jumps(g) < 0) {
+        return -1;
+    }
     return 0;
 }