GH-113710: Improve `_SET_IP` and `_CHECK_VALIDITY` (GH-115248)

diff --git a/Include/internal/pycore_uop_ids.h b/Include/internal/pycore_uop_ids.h
index b2476e1c6e5c4b56a0614351d4203db800410bec..9bb537d355055dbc1539fa910595dfe50e091028 100644 (file)
--- a/Include/internal/pycore_uop_ids.h
+++ b/Include/internal/pycore_uop_ids.h
@@ -237,7 +237,8 @@ extern "C" {
 #define _CHECK_GLOBALS 384
 #define _CHECK_BUILTINS 385
 #define _INTERNAL_INCREMENT_OPT_COUNTER 386
-#define MAX_UOP_ID 386
+#define _CHECK_VALIDITY_AND_SET_IP 387
+#define MAX_UOP_ID 387
 
 #ifdef __cplusplus
 }

diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h
index 30dc5a881574e7963cf72e6be395c3014d674273..163a0320aa22985fce42c4ef23d8a4f5c55811c7 100644 (file)
--- a/Include/internal/pycore_uop_metadata.h
+++ b/Include/internal/pycore_uop_metadata.h
@@ -198,7 +198,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = {
     [_GUARD_IS_NONE_POP] = HAS_DEOPT_FLAG,
     [_GUARD_IS_NOT_NONE_POP] = HAS_DEOPT_FLAG,
     [_JUMP_TO_TOP] = HAS_EVAL_BREAK_FLAG,
-    [_SET_IP] = HAS_ARG_FLAG | HAS_ESCAPES_FLAG,
+    [_SET_IP] = 0,
     [_SAVE_RETURN_OFFSET] = HAS_ARG_FLAG,
     [_EXIT_TRACE] = HAS_DEOPT_FLAG,
     [_CHECK_VALIDITY] = HAS_DEOPT_FLAG,
@@ -209,6 +209,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = {
     [_CHECK_GLOBALS] = HAS_DEOPT_FLAG,
     [_CHECK_BUILTINS] = HAS_DEOPT_FLAG,
     [_INTERNAL_INCREMENT_OPT_COUNTER] = 0,
+    [_CHECK_VALIDITY_AND_SET_IP] = HAS_DEOPT_FLAG,
 };
 
 const char *const _PyOpcode_uop_name[MAX_UOP_ID+1] = {
@@ -264,6 +265,7 @@ const char *const _PyOpcode_uop_name[MAX_UOP_ID+1] = {
     [_CHECK_PEP_523] = "_CHECK_PEP_523",
     [_CHECK_STACK_SPACE] = "_CHECK_STACK_SPACE",
     [_CHECK_VALIDITY] = "_CHECK_VALIDITY",
+    [_CHECK_VALIDITY_AND_SET_IP] = "_CHECK_VALIDITY_AND_SET_IP",
     [_COMPARE_OP] = "_COMPARE_OP",
     [_COMPARE_OP_FLOAT] = "_COMPARE_OP_FLOAT",
     [_COMPARE_OP_INT] = "_COMPARE_OP_INT",

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 2ad5878f52e90b7917f798fad1dfee1359ff1a49..96b97ca4be6d939d1015d1ab602d368d9a2470ff 100644 (file)
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -4037,10 +4037,9 @@ dummy_func(
             CHECK_EVAL_BREAKER();
         }
 
-        op(_SET_IP, (--)) {
+        op(_SET_IP, (instr_ptr/4 --)) {
             TIER_TWO_ONLY
-            // TODO: Put the code pointer in `operand` to avoid indirection via `frame`
-            frame->instr_ptr = _PyCode_CODE(_PyFrame_GetCode(frame)) + oparg;
+            frame->instr_ptr = (_Py_CODEUNIT *)instr_ptr;
         }
 
         op(_SAVE_RETURN_OFFSET, (--)) {
@@ -4100,6 +4099,11 @@ dummy_func(
             exe->count++;
         }
 
+        op(_CHECK_VALIDITY_AND_SET_IP, (instr_ptr/4 --)) {
+            TIER_TWO_ONLY
+            DEOPT_IF(!current_executor->vm_data.valid);
+            frame->instr_ptr = (_Py_CODEUNIT *)instr_ptr;
+        }
 
 // END BYTECODES //
 

diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index 7d48d6a05a17b067828a893d103bd802413cefb5..58d238320276f4bbb5c6b38a3bb1cc288a8523dd 100644 (file)
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -3360,10 +3360,9 @@
         }
 
         case _SET_IP: {
-            oparg = CURRENT_OPARG();
+            PyObject *instr_ptr = (PyObject *)CURRENT_OPERAND();
             TIER_TWO_ONLY
-            // TODO: Put the code pointer in `operand` to avoid indirection via `frame`
-            frame->instr_ptr = _PyCode_CODE(_PyFrame_GetCode(frame)) + oparg;
+            frame->instr_ptr = (_Py_CODEUNIT *)instr_ptr;
             break;
         }
 
@@ -3459,4 +3458,12 @@
             break;
         }
 
+        case _CHECK_VALIDITY_AND_SET_IP: {
+            PyObject *instr_ptr = (PyObject *)CURRENT_OPERAND();
+            TIER_TWO_ONLY
+            if (!current_executor->vm_data.valid) goto deoptimize;
+            frame->instr_ptr = (_Py_CODEUNIT *)instr_ptr;
+            break;
+        }
+
 #undef TIER_TWO

diff --git a/Python/optimizer.c b/Python/optimizer.c
index 13df8c170a537c67a65cef6e1f452b26a0d78c3b..efa19680c9b1f30aaf9b628e9f7e64a54b32deb2 100644 (file)
--- a/Python/optimizer.c
+++ b/Python/optimizer.c
@@ -432,9 +432,8 @@ translate_bytecode_to_trace(
 top:  // Jump here after _PUSH_FRAME or likely branches
     for (;;) {
         target = INSTR_IP(instr, code);
-        RESERVE_RAW(3, "epilogue");  // Always need space for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE
-        ADD_TO_TRACE(_SET_IP, target, 0, target);
-        ADD_TO_TRACE(_CHECK_VALIDITY, 0, 0, target);
+        RESERVE_RAW(2, "epilogue");  // Always need space for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE
+        ADD_TO_TRACE(_CHECK_VALIDITY_AND_SET_IP, 0, (uintptr_t)instr, target);
 
         uint32_t opcode = instr->op.code;
         uint32_t oparg = instr->op.arg;

diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c
index e02ca4d6acf6c14de10b6c5c26b37052d25a1b17..49974520de924da4266d685525d6a3fa5d02ea00 100644 (file)
--- a/Python/optimizer_analysis.c
+++ b/Python/optimizer_analysis.c
@@ -652,35 +652,62 @@ error:
 static void
 remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size)
 {
+    /* Remove _SET_IP and _CHECK_VALIDITY where possible.
+     * _SET_IP is needed if the following instruction escapes or
+     * could error. _CHECK_VALIDITY is needed if the previous
+     * instruction could have escaped. */
     int last_set_ip = -1;
-    bool maybe_invalid = false;
+    bool may_have_escaped = false;
     for (int pc = 0; pc < buffer_size; pc++) {
         int opcode = buffer[pc].opcode;
-        if (opcode == _SET_IP) {
-            buffer[pc].opcode = NOP;
-            last_set_ip = pc;
-        }
-        else if (opcode == _CHECK_VALIDITY) {
-            if (maybe_invalid) {
-                maybe_invalid = false;
-            }
-            else {
+        switch (opcode) {
+            case _SET_IP:
                 buffer[pc].opcode = NOP;
-            }
-        }
-        else if (op_is_end(opcode)) {
-            break;
-        }
-        else {
-            if (_PyUop_Flags[opcode] & HAS_ESCAPES_FLAG) {
-                maybe_invalid = true;
-                if (last_set_ip >= 0) {
-                    buffer[last_set_ip].opcode = _SET_IP;
+                last_set_ip = pc;
+                break;
+            case _CHECK_VALIDITY:
+                if (may_have_escaped) {
+                    may_have_escaped = false;
                 }
-            }
-            if ((_PyUop_Flags[opcode] & HAS_ERROR_FLAG) || opcode == _PUSH_FRAME) {
-                if (last_set_ip >= 0) {
-                    buffer[last_set_ip].opcode = _SET_IP;
+                else {
+                    buffer[pc].opcode = NOP;
+                }
+                break;
+            case _CHECK_VALIDITY_AND_SET_IP:
+                if (may_have_escaped) {
+                    may_have_escaped = false;
+                    buffer[pc].opcode = _CHECK_VALIDITY;
+                }
+                else {
+                    buffer[pc].opcode = NOP;
+                }
+                last_set_ip = pc;
+                break;
+            case _JUMP_TO_TOP:
+            case _EXIT_TRACE:
+                return;
+            default:
+            {
+                bool needs_ip = false;
+                if (_PyUop_Flags[opcode] & HAS_ESCAPES_FLAG) {
+                    needs_ip = true;
+                    may_have_escaped = true;
+                }
+                if (_PyUop_Flags[opcode] & HAS_ERROR_FLAG) {
+                    needs_ip = true;
+                }
+                if (opcode == _PUSH_FRAME) {
+                    needs_ip = true;
+                }
+                if (needs_ip && last_set_ip >= 0) {
+                    if (buffer[last_set_ip].opcode == _CHECK_VALIDITY) {
+                        buffer[last_set_ip].opcode = _CHECK_VALIDITY_AND_SET_IP;
+                    }
+                    else {
+                        assert(buffer[last_set_ip].opcode == _NOP);
+                        buffer[last_set_ip].opcode = _SET_IP;
+                    }
+                    last_set_ip = -1;
                 }
             }
         }

diff --git a/Python/tier2_redundancy_eliminator_cases.c.h b/Python/tier2_redundancy_eliminator_cases.c.h
index 77a7f5b2360c3b584f5958d79c625c2463aed37c..c2b7bbaf1c4481080af0a11f614a7784152e8444 100644 (file)
--- a/Python/tier2_redundancy_eliminator_cases.c.h
+++ b/Python/tier2_redundancy_eliminator_cases.c.h
@@ -1674,3 +1674,7 @@
             break;
         }
 
+        case _CHECK_VALIDITY_AND_SET_IP: {
+            break;
+        }
+