gh-107901: make compiler inline basic blocks with no line number and no fallthrough (#114750)

Lib/test/test_compile.py

@@ -1104,6 +1104,17 @@ class TestSpecifics(unittest.TestCase):
         code_lines = self.get_code_lines(test.__code__)
         self.assertEqual(expected_lines, code_lines)
 
+    def check_line_numbers(self, code, opnames=None):
+        # Check that all instructions whose op matches opnames
+        # have a line number. opnames can be a single name, or
+        # a sequence of names. If it is None, match all ops.
+
+        if isinstance(opnames, str):
+            opnames = (opnames, )
+        for inst in dis.Bytecode(code):
+            if opnames and inst.opname in opnames:
+                self.assertIsNotNone(inst.positions.lineno)
+
     def test_line_number_synthetic_jump_multiple_predecessors(self):
         def f():
             for x in it:
@@ -1113,25 +1124,52 @@ class TestSpecifics(unittest.TestCase):
                 except OSError:
                     pass
 
-        # Ensure that all JUMP_BACKWARDs have line number
+        self.check_line_numbers(f.__code__, 'JUMP_BACKWARD')
-        code = f.__code__
-        for inst in dis.Bytecode(code):
-            if inst.opname == 'JUMP_BACKWARD':
-                self.assertIsNotNone(inst.positions.lineno)
 
-    def test_lineno_of_backward_jump(self):
+    def test_line_number_synthetic_jump_multiple_predecessors_nested(self):
+        def f():
+            for x in it:
+                try:
+                    X = 3
+                except OSError:
+                    try:
+                        if C3:
+                            X = 4
+                    except OSError:
+                        pass
+            return 42
+
+        self.check_line_numbers(f.__code__, 'JUMP_BACKWARD')
+
+    def test_line_number_synthetic_jump_multiple_predecessors_more_nested(self):
+        def f():
+            for x in it:
+                try:
+                    X = 3
+                except OSError:
+                    try:
+                        if C3:
+                            if C4:
+                                X = 4
+                    except OSError:
+                        try:
+                            if C3:
+                                if C4:
+                                    X = 5
+                        except OSError:
+                            pass
+            return 42
+
+        self.check_line_numbers(f.__code__, 'JUMP_BACKWARD')
+
+    def test_lineno_of_backward_jump_conditional_in_loop(self):
         # Issue gh-107901
         def f():
             for i in x:
                 if y:
                     pass
 
-        linenos = list(inst.positions.lineno
+        self.check_line_numbers(f.__code__, 'JUMP_BACKWARD')
-                       for inst in dis.get_instructions(f.__code__)
-                       if inst.opname == 'JUMP_BACKWARD')
-
-        self.assertTrue(len(linenos) > 0)
-        self.assertTrue(all(l is not None for l in linenos))
 
     def test_big_dict_literal(self):
         # The compiler has a flushing point in "compiler_dict" that calls compiles

Lib/test/test_monitoring.py

@@ -1466,9 +1466,8 @@ class TestBranchAndJumpEvents(CheckEvents):
             ('branch', 'func', 4, 4),
             ('line', 'func', 5),
             ('line', 'meth', 1),
-            ('jump', 'func', 5, 5),
+            ('jump', 'func', 5, '[offset=118]'),
-            ('jump', 'func', 5, '[offset=114]'),
+            ('branch', 'func', '[offset=122]', '[offset=126]'),
-            ('branch', 'func', '[offset=120]', '[offset=124]'),
             ('line', 'get_events', 11)])
 
         self.check_events(func, recorders = FLOW_AND_LINE_RECORDERS, expected = [
@@ -1482,9 +1481,8 @@ class TestBranchAndJumpEvents(CheckEvents):
             ('line', 'func', 5),
             ('line', 'meth', 1),
             ('return', 'meth', None),
-            ('jump', 'func', 5, 5),
+            ('jump', 'func', 5, '[offset=118]'),
-            ('jump', 'func', 5, '[offset=114]'),
+            ('branch', 'func', '[offset=122]', '[offset=126]'),
-            ('branch', 'func', '[offset=120]', '[offset=124]'),
             ('return', 'func', None),
             ('line', 'get_events', 11)])
 

Python/flowgraph.c

@@ -212,14 +212,14 @@ basicblock_add_jump(basicblock *b, int opcode, basicblock *target, location loc)
 }
 
 static inline int
-basicblock_append_instructions(basicblock *target, basicblock *source)
+basicblock_append_instructions(basicblock *to, basicblock *from)
 {
-    for (int i = 0; i < source->b_iused; i++) {
+    for (int i = 0; i < from->b_iused; i++) {
-        int n = basicblock_next_instr(target);
+        int n = basicblock_next_instr(to);
         if (n < 0) {
             return ERROR;
         }
-        target->b_instr[n] = source->b_instr[i];
+        to->b_instr[n] = from->b_instr[i];
     }
     return SUCCESS;
 }
@@ -292,9 +292,9 @@ static void
 dump_basicblock(const basicblock *b)
 {
     const char *b_return = basicblock_returns(b) ? "return " : "";
-    fprintf(stderr, "%d: [EH=%d CLD=%d WRM=%d NO_FT=%d %p] used: %d, depth: %d, %s\n",
+    fprintf(stderr, "%d: [EH=%d CLD=%d WRM=%d NO_FT=%d %p] used: %d, depth: %d, preds: %d %s\n",
         b->b_label.id, b->b_except_handler, b->b_cold, b->b_warm, BB_NO_FALLTHROUGH(b), b, b->b_iused,
-        b->b_startdepth, b_return);
+        b->b_startdepth, b->b_predecessors, b_return);
     if (b->b_instr) {
         int i;
         for (i = 0; i < b->b_iused; i++) {
@@ -1165,15 +1165,26 @@ remove_redundant_jumps(cfg_builder *g) {
     return changes;
 }
 
+static inline bool
+basicblock_has_no_lineno(basicblock *b) {
+    for (int i = 0; i < b->b_iused; i++) {
+        if (b->b_instr[i].i_loc.lineno >= 0) {
+            return false;
+        }
+    }
+    return true;
+}
+
 /* Maximum size of basic block that should be copied in optimizer */
 #define MAX_COPY_SIZE 4
 
-/* If this block ends with an unconditional jump to a small exit block, then
+/* If this block ends with an unconditional jump to a small exit block or
+ * a block that has no line numbers (and no fallthrough), 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
-inline_small_exit_blocks(basicblock *bb) {
+basicblock_inline_small_or_no_lineno_blocks(basicblock *bb) {
     cfg_instr *last = basicblock_last_instr(bb);
     if (last == NULL) {
         return 0;
@@ -1182,14 +1193,46 @@ inline_small_exit_blocks(basicblock *bb) {
         return 0;
     }
     basicblock *target = last->i_target;
-    if (basicblock_exits_scope(target) && target->b_iused <= MAX_COPY_SIZE) {
+    bool small_exit_block = (basicblock_exits_scope(target) &&
+                             target->b_iused <= MAX_COPY_SIZE);
+    bool no_lineno_no_fallthrough = (basicblock_has_no_lineno(target) &&
+                                     !BB_HAS_FALLTHROUGH(target));
+    if (small_exit_block || no_lineno_no_fallthrough) {
+        assert(is_jump(last));
+        int removed_jump_opcode = last->i_opcode;
         INSTR_SET_OP0(last, NOP);
         RETURN_IF_ERROR(basicblock_append_instructions(bb, target));
+        if (no_lineno_no_fallthrough) {
+            last = basicblock_last_instr(bb);
+            if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode) &&
+                removed_jump_opcode == JUMP)
+            {
+                /* Make sure we don't lose eval breaker checks */
+                last->i_opcode = JUMP;
+            }
+        }
+        target->b_predecessors--;
         return 1;
     }
     return 0;
 }
 
+static int
+inline_small_or_no_lineno_blocks(basicblock *entryblock) {
+    bool changes;
+    do {
+        changes = false;
+        for (basicblock *b = entryblock; b != NULL; b = b->b_next) {
+            int res = basicblock_inline_small_or_no_lineno_blocks(b);
+            RETURN_IF_ERROR(res);
+            if (res) {
+                changes = true;
+            }
+        }
+    } while(changes); /* every change removes a jump, ensuring convergence */
+    return changes;
+}
+
 // Attempt to eliminate jumps to jumps by updating inst to jump to
 // target->i_target using the provided opcode. Return whether or not the
 // optimization was successful.
@@ -1804,9 +1847,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache, int firstl
 {
     assert(PyDict_CheckExact(const_cache));
     RETURN_IF_ERROR(check_cfg(g));
-    for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
+    RETURN_IF_ERROR(inline_small_or_no_lineno_blocks(g->g_entryblock));
-        RETURN_IF_ERROR(inline_small_exit_blocks(b));
-    }
     RETURN_IF_ERROR(remove_unreachable(g->g_entryblock));
     RETURN_IF_ERROR(resolve_line_numbers(g, firstlineno));
     RETURN_IF_ERROR(optimize_load_const(const_cache, g, consts));
@@ -1814,9 +1855,6 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache, int firstl
         RETURN_IF_ERROR(optimize_basic_block(const_cache, b, consts));
     }
     RETURN_IF_ERROR(remove_redundant_nops_and_pairs(g->g_entryblock));
-    for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
-        RETURN_IF_ERROR(inline_small_exit_blocks(b));
-    }
     RETURN_IF_ERROR(remove_unreachable(g->g_entryblock));
 
     int removed_nops, removed_jumps;
@@ -2333,12 +2371,7 @@ convert_pseudo_ops(cfg_builder *g)
 static inline bool
 is_exit_or_eval_check_without_lineno(basicblock *b) {
     if (basicblock_exits_scope(b) || basicblock_has_eval_break(b)) {
-        for (int i = 0; i < b->b_iused; i++) {
+        return basicblock_has_no_lineno(b);
-            if (b->b_instr[i].i_loc.lineno >= 0) {
-                return false;
-            }
-        }
-        return true;
     }
     else {
         return false;