bpo-47120: make POP_JUMP_IF_NONE/NOT_NONE relative

python · markshannon · Apr 11, 2022 · Apr 6, 2022 · Apr 6, 2022 · Apr 6, 2022
commit cf18a6d1aed8adb1c67b0f687ed68aff3a1120f5
diff --git a/Include/opcode.h b/Include/opcode.h
@@ -400,6 +400,7 @@ def _write_atomic(path, data, mode=0o666):
 #     Python 3.11a6 3490 (remove JUMP_IF_NOT_EXC_MATCH, add CHECK_EXC_MATCH)
 #     Python 3.11a6 3491 (remove JUMP_IF_NOT_EG_MATCH, add CHECK_EG_MATCH,
 #                         add JUMP_BACKWARD_NO_INTERRUPT, make JUMP_NO_INTERRUPT virtual)
+#     Python 3.11a7 3492 (make POP_JUMP_IF_NONE/NOT_NONE relative)

 #     Python 3.12 will start with magic number 3500

@@ -414,7 +415,7 @@ def _write_atomic(path, data, mode=0o666):
 # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
 # in PC/launcher.c must also be updated.

-MAGIC_NUMBER = (3491).to_bytes(2, 'little') + b'\r\n'
+MAGIC_NUMBER = (3492).to_bytes(2, 'little') + b'\r\n'
 _RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little')  # For import.c

 _PYCACHE = '__pycache__'

diff --git a/Lib/opcode.py b/Lib/opcode.py
@@ -148,8 +148,8 @@ def jabs_op(name, op, entries=0):
 haslocal.append(125)
 def_op('DELETE_FAST', 126)      # Local variable number
 haslocal.append(126)
-jabs_op('POP_JUMP_IF_NOT_NONE', 128)
-jabs_op('POP_JUMP_IF_NONE', 129)
+jrel_op('POP_JUMP_FORWARD_IF_NOT_NONE', 128)
+jrel_op('POP_JUMP_FORWARD_IF_NONE', 129)
 def_op('RAISE_VARARGS', 130)    # Number of raise arguments (1, 2, or 3)
 def_op('GET_AWAITABLE', 131)
 def_op('MAKE_FUNCTION', 132)    # Flags
@@ -197,6 +197,8 @@ def jabs_op(name, op, entries=0):
 def_op('KW_NAMES', 172)
 hasconst.append(172)

+jrel_op('POP_JUMP_BACKWARD_IF_NOT_NONE', 173)
+jrel_op('POP_JUMP_BACKWARD_IF_NONE', 174)

 del def_op, name_op, jrel_op, jabs_op


diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
@@ -685,7 +685,11 @@ def test_widths(self):
        for opcode, opname in enumerate(dis.opname):
            if opname in ('BUILD_MAP_UNPACK_WITH_CALL',
                          'BUILD_TUPLE_UNPACK_WITH_CALL',
-                          'JUMP_BACKWARD_NO_INTERRUPT'):
+                          'JUMP_BACKWARD_NO_INTERRUPT',
+                          'POP_JUMP_FORWARD_IF_NONE',
+                          'POP_JUMP_BACKWARD_IF_NONE',
+                          'POP_JUMP_FORWARD_IF_NOT_NONE',
+                          'POP_JUMP_BACKWARD_IF_NOT_NONE'):
                continue
            with self.subTest(opname=opname):
                width = dis._OPNAME_WIDTH

@@ -3972,23 +3972,35 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
            DISPATCH();
        }

-        TARGET(POP_JUMP_IF_NOT_NONE) {
+        TARGET(POP_JUMP_BACKWARD_IF_NOT_NONE) {
+            oparg = -oparg;
+            JUMP_TO_INSTRUCTION(POP_JUMP_FORWARD_IF_NOT_NONE);
+        }
+
+        TARGET(POP_JUMP_FORWARD_IF_NOT_NONE) {
+            PREDICTED(POP_JUMP_FORWARD_IF_NOT_NONE);
            PyObject *value = POP();
            if (!Py_IsNone(value)) {
                Py_DECREF(value);
-                JUMPTO(oparg);
+                JUMPBY(oparg);
                CHECK_EVAL_BREAKER();
                DISPATCH();
            }
            Py_DECREF(value);
            DISPATCH();
        }

-        TARGET(POP_JUMP_IF_NONE) {
+        TARGET(POP_JUMP_BACKWARD_IF_NONE) {
+            oparg = -oparg;
+            JUMP_TO_INSTRUCTION(POP_JUMP_FORWARD_IF_NONE);
+        }
+
+        TARGET(POP_JUMP_FORWARD_IF_NONE) {
+            PREDICTED(POP_JUMP_FORWARD_IF_NONE);
            PyObject *value = POP();
            if (Py_IsNone(value)) {
                Py_DECREF(value);
-                JUMPTO(oparg);
+                JUMPBY(oparg);
                CHECK_EVAL_BREAKER();
                DISPATCH();
            }

@@ -78,8 +78,10 @@
 #define POP_BLOCK -4
 #define JUMP -5
 #define JUMP_NO_INTERRUPT -6
+#define POP_JUMP_IF_NONE -7
+#define POP_JUMP_IF_NOT_NONE -8

-#define MIN_VIRTUAL_OPCODE -6
+#define MIN_VIRTUAL_OPCODE -8
 #define MAX_ALLOWED_OPCODE 254

 #define IS_WITHIN_OPCODE_RANGE(opcode) \
@@ -89,13 +91,25 @@

 #define IS_VIRTUAL_JUMP_OPCODE(opcode) \
        ((opcode) == JUMP || \
-         (opcode) == JUMP_NO_INTERRUPT)
+         (opcode) == JUMP_NO_INTERRUPT || \
+         (opcode) == POP_JUMP_IF_NONE || \
+         (opcode) == POP_JUMP_IF_NOT_NONE)

 /* opcodes which are not emitted in codegen stage, only by the assembler */
 #define IS_ASSEMBLER_OPCODE(opcode) \
        ((opcode) == JUMP_FORWARD || \
         (opcode) == JUMP_BACKWARD || \
-         (opcode) == JUMP_BACKWARD_NO_INTERRUPT)
+         (opcode) == JUMP_BACKWARD_NO_INTERRUPT || \
+         (opcode) == POP_JUMP_FORWARD_IF_NONE || \
+         (opcode) == POP_JUMP_BACKWARD_IF_NONE || \
+         (opcode) == POP_JUMP_FORWARD_IF_NOT_NONE || \
+         (opcode) == POP_JUMP_BACKWARD_IF_NOT_NONE)
+
+#define IS_BACKWARDS_JUMP_OPCODE(opcode) \
+        ((opcode) == JUMP_BACKWARD || \
+         (opcode) == JUMP_BACKWARD_NO_INTERRUPT || \
+         (opcode) == POP_JUMP_BACKWARD_IF_NONE || \
+         (opcode) == POP_JUMP_BACKWARD_IF_NOT_NONE)


 #define IS_TOP_LEVEL_AWAIT(c) ( \
@@ -1031,10 +1045,14 @@ stack_effect(int opcode, int oparg, int jump)
        case JUMP_IF_FALSE_OR_POP:
            return jump ? 0 : -1;

-        case POP_JUMP_IF_FALSE:
-        case POP_JUMP_IF_TRUE:
+        case POP_JUMP_BACKWARD_IF_NONE:
+        case POP_JUMP_FORWARD_IF_NONE:
        case POP_JUMP_IF_NONE:
+        case POP_JUMP_BACKWARD_IF_NOT_NONE:
+        case POP_JUMP_FORWARD_IF_NOT_NONE:
        case POP_JUMP_IF_NOT_NONE:
+        case POP_JUMP_IF_FALSE:
+        case POP_JUMP_IF_TRUE:
            return -1;

        case LOAD_GLOBAL:
@@ -7613,14 +7631,25 @@ normalize_jumps(struct assembler *a)
        }
        struct instr *last = &b->b_instr[b->b_iused-1];
        assert(!IS_ASSEMBLER_OPCODE(last->i_opcode));
-        if (last->i_opcode == JUMP) {
-            bool is_forward = last->i_target->b_visited == 0;
-            last->i_opcode = is_forward ? JUMP_FORWARD : JUMP_BACKWARD;
-        }
-        if (last->i_opcode == JUMP_NO_INTERRUPT) {
+        if (is_jump(last)) {
            bool is_forward = last->i_target->b_visited == 0;
-            last->i_opcode = is_forward ?
-                             JUMP_FORWARD : JUMP_BACKWARD_NO_INTERRUPT;
+            switch(last->i_opcode) {
+                case JUMP:
+                    last->i_opcode = is_forward ? JUMP_FORWARD : JUMP_BACKWARD;
+                    break;
+                case JUMP_NO_INTERRUPT:
+                    last->i_opcode = is_forward ?
+                                     JUMP_FORWARD : JUMP_BACKWARD_NO_INTERRUPT;
+                    break;
+                case POP_JUMP_IF_NOT_NONE:
+                    last->i_opcode = is_forward ?
+                        POP_JUMP_FORWARD_IF_NOT_NONE : POP_JUMP_BACKWARD_IF_NOT_NONE;
+                    break;
+                case POP_JUMP_IF_NONE:
+                    last->i_opcode = is_forward ?
+                        POP_JUMP_FORWARD_IF_NONE : POP_JUMP_BACKWARD_IF_NONE;
+                    break;
+            }
        }
    }
 }
@@ -7656,13 +7685,11 @@ assemble_jump_offsets(struct assembler *a, struct compiler *c)
                    instr->i_oparg = instr->i_target->b_offset;
                    if (is_relative_jump(instr)) {
                        if (instr->i_oparg < bsize) {
-                            assert(instr->i_opcode == JUMP_BACKWARD ||
-                                   instr->i_opcode == JUMP_BACKWARD_NO_INTERRUPT);
+                            assert(IS_BACKWARDS_JUMP_OPCODE(instr->i_opcode));
                            instr->i_oparg = bsize - instr->i_oparg;
                        }
                        else {
-                            assert(instr->i_opcode != JUMP_BACKWARD);
-                            assert(instr->i_opcode != JUMP_BACKWARD_NO_INTERRUPT);
+                            assert(!IS_BACKWARDS_JUMP_OPCODE(instr->i_opcode));
                            instr->i_oparg -= bsize;
                        }
                    }
@@ -8648,7 +8675,7 @@ apply_static_swaps(basicblock *block, int i)
 static bool
 jump_thread(struct instr *inst, struct instr *target, int opcode)
 {
-    assert(!IS_VIRTUAL_OPCODE(opcode) || opcode == JUMP);
+    assert(!IS_VIRTUAL_OPCODE(opcode) || IS_VIRTUAL_JUMP_OPCODE(opcode));
    assert(is_jump(inst));
    assert(is_jump(target));
    // bpo-45773: If inst->i_target == target->i_target, then nothing actually

diff --git a/Python/opcode_targets.h b/Python/opcode_targets.h