bpo-32477: Move jumps optimization from the peepholer to the compiler.

Lib/test/test_dis.py

@@ -140,22 +140,22 @@ def bug708901():
 
 
 def bug1333982(x=[]):
-    assert 0, ([s for s in x] +
+    assert x, ([s for s in x] +
               1)
     pass
 
 dis_bug1333982 = """\
-%3d           0 LOAD_CONST               1 (0)
+%3d           0 LOAD_FAST                0 (x)
               2 POP_JUMP_IF_TRUE        26
               4 LOAD_ASSERTION_ERROR
-              6 LOAD_CONST               2 (<code object <listcomp> at 0x..., file "%s", line %d>)
-              8 LOAD_CONST               3 ('bug1333982.<locals>.<listcomp>')
+              6 LOAD_CONST               1 (<code object <listcomp> at 0x..., file "%s", line %d>)
+              8 LOAD_CONST               2 ('bug1333982.<locals>.<listcomp>')
              10 MAKE_FUNCTION            0
              12 LOAD_FAST                0 (x)
              14 GET_ITER
              16 CALL_FUNCTION            1
 
-%3d          18 LOAD_CONST               4 (1)
+%3d          18 LOAD_CONST               3 (1)
 
 %3d          20 BINARY_ADD
              22 CALL_FUNCTION            1

Lib/test/test_peepholer.py

@@ -334,25 +334,33 @@ def f(cond, true_value, false_value):
 
     def test_elim_jump_to_uncond_jump(self):
         # POP_JUMP_IF_FALSE to JUMP_FORWARD --> POP_JUMP_IF_FALSE to non-jump
+        # JUMP_FORWARD to JUMP_FORWARD --> JUMP_FORWARD to non-jump
         def f():
             if a:
-                # Intentionally use two-line expression to test issue37213.
-                if (c
-                    or d):
-                    foo()
+                if b:
+                    # Intentionally use two-line expression to test issue37213.
+                    if (c
+                        or d):
+                        foo()
+                else:
+                    bar()
             else:
                 baz()
         self.check_jump_targets(f)
         self.check_lnotab(f)
 
     def test_elim_jump_to_uncond_jump2(self):
         # POP_JUMP_IF_FALSE to JUMP_ABSOLUTE --> POP_JUMP_IF_FALSE to non-jump
+        # JUMP_FORWARD to JUMP_ABSOLUTE --> JUMP_FORWARD to non-jump
         def f():
             while a:
-                # Intentionally use two-line expression to test issue37213.
-                if (c
-                    or d):
-                    a = foo()
+                if b:
+                    # Intentionally use two-line expression to test issue37213.
+                    if (c
+                        or d):
+                        a = foo()
+                else:
+                    a = bar()
         self.check_jump_targets(f)
         self.check_lnotab(f)
 
@@ -415,10 +423,7 @@ def f(cond1, cond2):
             while 1:
                 if cond1: return 4
         self.assertNotInBytecode(f, 'JUMP_FORWARD')
-        # There should be one jump for the while loop.
-        returns = [instr for instr in dis.get_instructions(f)
-                          if instr.opname == 'JUMP_ABSOLUTE']
-        self.assertEqual(len(returns), 1)
+        self.assertNotInBytecode(f, 'JUMP_ABSOLUTE')
         returns = [instr for instr in dis.get_instructions(f)
                           if instr.opname == 'RETURN_VALUE']
         self.assertLessEqual(len(returns), 2)

Lib/test/test_sys_settrace.py

@@ -123,10 +123,10 @@ def no_pop_tops():      # 0
                       (1, 'line'),
                       (2, 'line'),
                       (3, 'line'),
-                      (6, 'line'),
                       (2, 'line'),
                       (3, 'line'),
                       (4, 'line'),
+                      (6, 'line'),
                       (2, 'line'),
                       (2, 'return')]
 

Python/compile.c

@@ -134,6 +134,7 @@ struct compiler_unit {
     /* Pointer to the most recently allocated block.  By following b_list
        members, you can reach all early allocated blocks. */
     basicblock *u_blocks;
+    basicblock *u_entryblock; /* pointer to first block */
     basicblock *u_curblock; /* pointer to current block */
 
     int u_nfblocks;
@@ -612,6 +613,7 @@ compiler_enter_scope(struct compiler *c, identifier name,
     }
 
     u->u_blocks = NULL;
+    u->u_entryblock = NULL;
     u->u_nfblocks = 0;
     u->u_firstlineno = lineno;
     u->u_lineno = 0;
@@ -779,6 +781,9 @@ compiler_new_block(struct compiler *c)
     /* Extend the singly linked list of blocks with new block. */
     b->b_list = u->u_blocks;
     u->u_blocks = b;
+    if (u->u_entryblock == NULL) {
+        u->u_entryblock = b;
+    }
     return b;
 }
 
@@ -2548,6 +2553,7 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond)
             VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Compare.comparators, n));
             ADDOP_COMPARE(c, asdl_seq_GET(e->v.Compare.ops, n));
             ADDOP_JABS(c, cond ? POP_JUMP_IF_TRUE : POP_JUMP_IF_FALSE, next);
+            NEXT_BLOCK(c);
             basicblock *end = compiler_new_block(c);
             if (end == NULL)
                 return 0;
@@ -2563,6 +2569,17 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond)
         /* fallback to general implementation */
         break;
     }
+    case Constant_kind: {
+        int is_true = PyObject_IsTrue(e->v.Constant.value);
+        if (is_true < 0) {
+            return 0;
+        }
+        if (!is_true == !cond) {
+            ADDOP_JREL(c, JUMP_FORWARD, next);
+            NEXT_BLOCK(c);
+        }
+        return 1;
+    }
     default:
         /* fallback to general implementation */
         break;
@@ -2571,6 +2588,7 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond)
     /* general implementation */
     VISIT(c, expr, e);
     ADDOP_JABS(c, cond ? POP_JUMP_IF_TRUE : POP_JUMP_IF_FALSE, next);
+    NEXT_BLOCK(c);
     return 1;
 }
 
@@ -2832,12 +2850,10 @@ compiler_while(struct compiler *c, stmt_ty s)
     VISIT_SEQ(c, stmt, s->v.While.body);
     ADDOP_JABS(c, JUMP_ABSOLUTE, loop);
 
-    /* XXX should the two POP instructions be in a separate block
-       if there is no else clause ?
-    */
-
     if (constant == -1)
         compiler_use_next_block(c, anchor);
+    else if (orelse != NULL)
+        NEXT_BLOCK(c);
     compiler_pop_fblock(c, WHILE_LOOP, loop);
 
     if (orelse != NULL) /* what if orelse is just pass? */
@@ -2890,6 +2906,7 @@ compiler_break(struct compiler *c)
         return 0;
     }
     ADDOP_JABS(c, JUMP_ABSOLUTE, loop->fb_exit);
+    NEXT_BLOCK(c);
     return 1;
 }
 
@@ -2904,6 +2921,7 @@ compiler_continue(struct compiler *c)
         return compiler_error(c, "'continue' not properly in loop");
     }
     ADDOP_JABS(c, JUMP_ABSOLUTE, loop->fb_block);
+    NEXT_BLOCK(c);
     return 1;
 }
 
@@ -3040,6 +3058,7 @@ compiler_try_except(struct compiler *c, stmt_ty s)
             ADDOP(c, DUP_TOP);
             VISIT(c, expr, handler->v.ExceptHandler.type);
             ADDOP_JABS(c, JUMP_IF_NOT_EXC_MATCH, except);
+            NEXT_BLOCK(c);
         }
         ADDOP(c, POP_TOP);
         if (handler->v.ExceptHandler.name) {
@@ -3614,6 +3633,7 @@ compiler_boolop(struct compiler *c, expr_ty e)
     for (i = 0; i < n; ++i) {
         VISIT(c, expr, (expr_ty)asdl_seq_GET(s, i));
         ADDOP_JABS(c, jumpi, end);
+        NEXT_BLOCK(c);
     }
     VISIT(c, expr, (expr_ty)asdl_seq_GET(s, n));
     compiler_use_next_block(c, end);
@@ -4363,7 +4383,6 @@ compiler_sync_comprehension_generator(struct compiler *c,
         depth++;
         compiler_use_next_block(c, start);
         ADDOP_JREL(c, FOR_ITER, anchor);
-        NEXT_BLOCK(c);
     }
     VISIT(c, expr, gen->target);
 
@@ -4699,6 +4718,7 @@ compiler_with_except_finish(struct compiler *c) {
     if (exit == NULL)
         return 0;
     ADDOP_JABS(c, POP_JUMP_IF_TRUE, exit);
+    NEXT_BLOCK(c);
     ADDOP(c, RERAISE);
     compiler_use_next_block(c, exit);
     ADDOP(c, POP_TOP);
@@ -5388,14 +5408,14 @@ stackdepth_push(basicblock ***sp, basicblock *b, int depth)
 static int
 stackdepth(struct compiler *c)
 {
-    basicblock *b, *entryblock = NULL;
+    basicblock *b, *entryblock;
     basicblock **stack, **sp;
     int nblocks = 0, maxdepth = 0;
     for (b = c->u->u_blocks; b != NULL; b = b->b_list) {
         b->b_startdepth = INT_MIN;
-        entryblock = b;
         nblocks++;
     }
+    entryblock = c->u->u_entryblock;
     if (!entryblock)
         return 0;
     stack = (basicblock **)PyObject_Malloc(sizeof(basicblock *) * nblocks);
@@ -5691,6 +5711,104 @@ assemble_jump_offsets(struct assembler *a, struct compiler *c)
     } while (extended_arg_recompile);
 }
 
+#define UNCONDITIONAL_JUMP(op)  (op==JUMP_ABSOLUTE || op==JUMP_FORWARD)
+#define CONDITIONAL_JUMP(op) (op==POP_JUMP_IF_FALSE || op==POP_JUMP_IF_TRUE \
+    || op==JUMP_IF_FALSE_OR_POP || op==JUMP_IF_TRUE_OR_POP)
+#define JUMPS_ON_TRUE(op) (op==POP_JUMP_IF_TRUE || op==JUMP_IF_TRUE_OR_POP)
+
+static void
+optimize_jumps(struct compiler *c)
+{
+    for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) {
+        for (int i = 0; i < b->b_iused; i++) {
+            struct instr *instr = &b->b_instr[i];
+            switch (instr->i_opcode) {
+            case JUMP_IF_FALSE_OR_POP:
+            case JUMP_IF_TRUE_OR_POP:
+            case POP_JUMP_IF_FALSE:
+            case POP_JUMP_IF_TRUE:
+            case JUMP_FORWARD:
+            case JUMP_ABSOLUTE:
+                assert(instr->i_target != NULL);
+                int count = 10;
+                while (count--) {
+                    /* Skip empty blocks. */
+                    while (!instr->i_target->b_iused) {
+                        assert(instr->i_target->b_next);
+                        instr->i_target = instr->i_target->b_next;
+                    }
+
+                    struct instr *tgt = &instr->i_target->b_instr[0];
+                    if (tgt == instr) {
+                        break;
+                    }
+
+                    /* Replace jumps to unconditional jumps. */
+                    if (UNCONDITIONAL_JUMP(tgt->i_opcode)) {
+                        if (instr->i_opcode == JUMP_FORWARD) {
+                            /* JUMP_ABSOLUTE can go backwards */
+                            *instr = *tgt;
+                        }
+                        else {
+                            instr->i_target = tgt->i_target;
+                        }
+                        continue;
+                    }
+
+                    /* Replace JUMP_* to a RETURN into just a RETURN.
+                       Arises in code like "return a if b else c". */
+                    if (UNCONDITIONAL_JUMP(instr->i_opcode)) {
+                        if (tgt->i_opcode == RETURN_VALUE) {
+                            *instr = *tgt;
+                            break;
+                        }
+                    }
+
+                    /* Simplify conditional jump to conditional jump where the
+                       result of the first test implies the success of a similar
+                       test or the failure of the opposite test.
+                       Arises in code like:
+                       "a and b or c"
+                       "(a and b) and c"
+                       "(a or b) or c"
+                       "(a or b) and c"
+                       x:JUMP_IF_FALSE_OR_POP y   y:JUMP_IF_FALSE_OR_POP z
+                          -->  x:JUMP_IF_FALSE_OR_POP z
+                       x:JUMP_IF_FALSE_OR_POP y   y:JUMP_IF_TRUE_OR_POP z
+                          -->  x:POP_JUMP_IF_FALSE y+1
+                       where y+1 is the instruction following the second test.
+                     */
+                    if ((instr->i_opcode == JUMP_IF_FALSE_OR_POP ||
+                         instr->i_opcode == JUMP_IF_TRUE_OR_POP) &&
+                        CONDITIONAL_JUMP(tgt->i_opcode))
+                    {
+                        /* NOTE: all possible jumps here are absolute. */
+                        if (JUMPS_ON_TRUE(tgt->i_opcode) == JUMPS_ON_TRUE(instr->i_opcode)) {
+                            /* The second jump will be taken iff the first is.
+                               The current opcode inherits its target's
+                               stack effect. */
+                            instr->i_target = tgt->i_target;
+                            continue;
+                        }
+                        else if (instr->i_target->b_iused == 1) {
+                            /* The second jump is not taken if the first is (so
+                               jump past it), and all conditional jumps pop their
+                               argument when they're not taken (so change the
+                               first jump to pop its argument when it's taken). */
+                            instr->i_target = instr->i_target->b_next;
+                            instr->i_opcode = instr->i_opcode == JUMP_IF_TRUE_OR_POP ?
+                                    POP_JUMP_IF_TRUE : POP_JUMP_IF_FALSE;
+                            continue;
+                        }
+                    }
+                    break;
+                }
+                break;
+            }
+        }
+    }
+}
+
 static PyObject *
 dict_keys_inorder(PyObject *dict, Py_ssize_t offset)
 {
@@ -5933,16 +6051,18 @@ assemble(struct compiler *c, int addNone)
         ADDOP(c, RETURN_VALUE);
     }
 
+    optimize_jumps(c);
+
     nblocks = 0;
-    entryblock = NULL;
     for (b = c->u->u_blocks; b != NULL; b = b->b_list) {
         nblocks++;
-        entryblock = b;
     }
 
+    entryblock = c->u->u_entryblock;
+    assert(entryblock != NULL);
     /* Set firstlineno if it wasn't explicitly set. */
     if (!c->u->u_firstlineno) {
-        if (entryblock && entryblock->b_instr && entryblock->b_instr->i_lineno)
+        if (entryblock->b_instr && entryblock->b_instr->i_lineno)
             c->u->u_firstlineno = entryblock->b_instr->i_lineno;
         else
             c->u->u_firstlineno = 1;