GH-109214: _SET_IP before _PUSH_FRAME (but not _POP_FRAME) (GH-111001)

3 files changed, 25 insertions, 42 deletions

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index d7e2ecd..2ff3805 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -803,7 +803,6 @@ dummy_func(
         }
 
         macro(RETURN_VALUE) =
-            _SAVE_CURRENT_IP +  // Sets frame->prev_instr
             _POP_FRAME;
 
         inst(INSTRUMENTED_RETURN_VALUE, (retval --)) {
@@ -827,7 +826,6 @@ dummy_func(
 
         macro(RETURN_CONST) =
             LOAD_CONST +
-            _SAVE_CURRENT_IP +  // Sets frame->prev_instr
             _POP_FRAME;
 
         inst(INSTRUMENTED_RETURN_CONST, (--)) {
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index 6fbe80f..506c02a 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -987,36 +987,28 @@
 
         TARGET(RETURN_VALUE) {
             PyObject *retval;
-            // _SAVE_CURRENT_IP
-            {
-                TIER_ONE_ONLY
-                frame->prev_instr = next_instr - 1;
-            }
-            // _POP_FRAME
             retval = stack_pointer[-1];
             STACK_SHRINK(1);
-            {
-                assert(EMPTY());
-                #if TIER_ONE
-                assert(frame != &entry_frame);
-                #endif
-                STORE_SP();
-                _Py_LeaveRecursiveCallPy(tstate);
-                // GH-99729: We need to unlink the frame *before* clearing it:
-                _PyInterpreterFrame *dying = frame;
-                frame = tstate->current_frame = dying->previous;
-                _PyEval_FrameClearAndPop(tstate, dying);
-                frame->prev_instr += frame->return_offset;
-                _PyFrame_StackPush(frame, retval);
-                LOAD_SP();
-                LOAD_IP();
-    #if LLTRACE && TIER_ONE
-                lltrace = maybe_lltrace_resume_frame(frame, &entry_frame, GLOBALS());
-                if (lltrace < 0) {
-                    goto exit_unwind;
-                }
-    #endif
+            assert(EMPTY());
+            #if TIER_ONE
+            assert(frame != &entry_frame);
+            #endif
+            STORE_SP();
+            _Py_LeaveRecursiveCallPy(tstate);
+            // GH-99729: We need to unlink the frame *before* clearing it:
+            _PyInterpreterFrame *dying = frame;
+            frame = tstate->current_frame = dying->previous;
+            _PyEval_FrameClearAndPop(tstate, dying);
+            frame->prev_instr += frame->return_offset;
+            _PyFrame_StackPush(frame, retval);
+            LOAD_SP();
+            LOAD_IP();
+#if LLTRACE && TIER_ONE
+            lltrace = maybe_lltrace_resume_frame(frame, &entry_frame, GLOBALS());
+            if (lltrace < 0) {
+                goto exit_unwind;
             }
+#endif
             DISPATCH();
         }
 
@@ -1049,11 +1041,6 @@
                 value = GETITEM(FRAME_CO_CONSTS, oparg);
                 Py_INCREF(value);
             }
-            // _SAVE_CURRENT_IP
-            {
-                TIER_ONE_ONLY
-                frame->prev_instr = next_instr - 1;
-            }
             // _POP_FRAME
             retval = value;
             {
diff --git a/Python/optimizer.c b/Python/optimizer.c
index 8d19de2..3dfd4f3 100644
--- a/Python/optimizer.c
+++ b/Python/optimizer.c
@@ -701,7 +701,9 @@ pop_jump_if_bool:
                             case OPARG_BOTTOM:  // Second half of super-instr
                                 oparg = orig_oparg & 0xF;
                                 break;
-                            case OPARG_SET_IP:  // op==_SET_IP; oparg=next instr
+                            case OPARG_SET_IP:  // uop=_SET_IP; oparg=next_instr-1
+                                // The number of caches is smuggled in via offset:
+                                assert(offset == _PyOpcode_Caches[_PyOpcode_Deopt[opcode]]);
                                 oparg = INSTR_IP(instr + offset, code);
                                 uop = _SET_IP;
                                 break;
@@ -850,11 +852,7 @@ remove_unneeded_uops(_PyUOpInstruction *trace, int trace_length)
     bool need_ip = true;
     for (int pc = 0; pc < trace_length; pc++) {
         int opcode = trace[pc].opcode;
-        if (opcode == _SAVE_CURRENT_IP) {
-            // Special case: never remove preceding _SET_IP
-            last_set_ip = -1;
-        }
-        else if (opcode == _SET_IP) {
+        if (opcode == _SET_IP) {
             if (!need_ip && last_set_ip >= 0) {
                 trace[last_set_ip].opcode = NOP;
             }
@@ -866,8 +864,8 @@ remove_unneeded_uops(_PyUOpInstruction *trace, int trace_length)
             break;
         }
         else {
-            // If opcode has ERROR or DEOPT, set need_up to true
-            if (_PyOpcode_opcode_metadata[opcode].flags & (HAS_ERROR_FLAG | HAS_DEOPT_FLAG)) {
+            // If opcode has ERROR or DEOPT, set need_ip to true
+            if (_PyOpcode_opcode_metadata[opcode].flags & (HAS_ERROR_FLAG | HAS_DEOPT_FLAG) || opcode == _PUSH_FRAME) {
                 need_ip = true;
             }
         }