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

python · Oct 24, 2023 · e5168ff · e5168ff
1 parent c0ea67d
commit e5168ff
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 44 deletions.
diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-10-16-15-51-37.gh-issue-109214.-RGTFH.rst b/Misc/NEWS.d/next/Core and Builtins/2023-10-16-15-51-37.gh-issue-109214.-RGTFH.rst
@@ -0,0 +1 @@
+Remove unnecessary instruction pointer updates before returning from frames.
diff --git 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
diff --git a/Python/optimizer.c b/Python/optimizer.c
@@ -701,7 +701,9 @@ translate_bytecode_to_trace(
                             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;
             }
         }