GH-109214: Convert _SAVE_CURRENT_IP to _SET_IP in tier 2 trace creation. (GH-110755)

8 files changed, 22 insertions, 59 deletions

diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h
index 8ef398c..926c004 100644
--- a/Include/internal/pycore_opcode_metadata.h
+++ b/Include/internal/pycore_opcode_metadata.h
@@ -1644,8 +1644,8 @@ const struct opcode_macro_expansion _PyOpcode_macro_expansion[OPCODE_MACRO_EXPAN
     [DELETE_SUBSCR] = { .nuops = 1, .uops = { { DELETE_SUBSCR, 0, 0 } } },
     [CALL_INTRINSIC_1] = { .nuops = 1, .uops = { { CALL_INTRINSIC_1, 0, 0 } } },
     [CALL_INTRINSIC_2] = { .nuops = 1, .uops = { { CALL_INTRINSIC_2, 0, 0 } } },
-    [RETURN_VALUE] = { .nuops = 3, .uops = { { _SET_IP, 7, 0 }, { _SAVE_CURRENT_IP, 0, 0 }, { _POP_FRAME, 0, 0 } } },
-    [RETURN_CONST] = { .nuops = 4, .uops = { { LOAD_CONST, 0, 0 }, { _SET_IP, 7, 0 }, { _SAVE_CURRENT_IP, 0, 0 }, { _POP_FRAME, 0, 0 } } },
+    [RETURN_VALUE] = { .nuops = 2, .uops = { { _SAVE_CURRENT_IP, 7, -1 }, { _POP_FRAME, 0, 0 } } },
+    [RETURN_CONST] = { .nuops = 3, .uops = { { LOAD_CONST, 0, 0 }, { _SAVE_CURRENT_IP, 7, -1 }, { _POP_FRAME, 0, 0 } } },
     [GET_AITER] = { .nuops = 1, .uops = { { GET_AITER, 0, 0 } } },
     [GET_ANEXT] = { .nuops = 1, .uops = { { GET_ANEXT, 0, 0 } } },
     [GET_AWAITABLE] = { .nuops = 1, .uops = { { GET_AWAITABLE, 0, 0 } } },
@@ -1719,8 +1719,8 @@ const struct opcode_macro_expansion _PyOpcode_macro_expansion[OPCODE_MACRO_EXPAN
     [LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES] = { .nuops = 4, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _GUARD_DORV_VALUES_INST_ATTR_FROM_DICT, 0, 0 }, { _GUARD_KEYS_VERSION, 2, 3 }, { _LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES, 4, 5 } } },
     [LOAD_ATTR_NONDESCRIPTOR_NO_DICT] = { .nuops = 2, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _LOAD_ATTR_NONDESCRIPTOR_NO_DICT, 4, 5 } } },
     [LOAD_ATTR_METHOD_LAZY_DICT] = { .nuops = 3, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _CHECK_ATTR_METHOD_LAZY_DICT, 0, 0 }, { _LOAD_ATTR_METHOD_LAZY_DICT, 4, 5 } } },
-    [CALL_BOUND_METHOD_EXACT_ARGS] = { .nuops = 9, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _INIT_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SET_IP, 7, 3 }, { _SAVE_CURRENT_IP, 0, 0 }, { _PUSH_FRAME, 0, 0 } } },
-    [CALL_PY_EXACT_ARGS] = { .nuops = 7, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SET_IP, 7, 3 }, { _SAVE_CURRENT_IP, 0, 0 }, { _PUSH_FRAME, 0, 0 } } },
+    [CALL_BOUND_METHOD_EXACT_ARGS] = { .nuops = 8, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _INIT_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SAVE_CURRENT_IP, 7, 2 }, { _PUSH_FRAME, 0, 0 } } },
+    [CALL_PY_EXACT_ARGS] = { .nuops = 6, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SAVE_CURRENT_IP, 7, 2 }, { _PUSH_FRAME, 0, 0 } } },
     [CALL_TYPE_1] = { .nuops = 1, .uops = { { CALL_TYPE_1, 0, 0 } } },
     [CALL_STR_1] = { .nuops = 1, .uops = { { CALL_STR_1, 0, 0 } } },
     [CALL_TUPLE_1] = { .nuops = 1, .uops = { { CALL_TUPLE_1, 0, 0 } } },
diff --git a/Python/abstract_interp_cases.c.h b/Python/abstract_interp_cases.c.h
index 04fe07f..44115da 100644
--- a/Python/abstract_interp_cases.c.h
+++ b/Python/abstract_interp_cases.c.h
@@ -920,10 +920,6 @@
             break;
         }
 
-        case _SAVE_CURRENT_IP: {
-            break;
-        }
-
         case _EXIT_TRACE: {
             break;
         }
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 9b733ce..62dc548 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -803,7 +803,6 @@ dummy_func(
         }
 
         macro(RETURN_VALUE) =
-            _SET_IP +  // Tier 2 only; special-cased oparg
             _SAVE_CURRENT_IP +  // Sets frame->prev_instr
             _POP_FRAME;
 
@@ -828,7 +827,6 @@ dummy_func(
 
         macro(RETURN_CONST) =
             LOAD_CONST +
-            _SET_IP +  // Tier 2 only; special-cased oparg
             _SAVE_CURRENT_IP +  // Sets frame->prev_instr
             _POP_FRAME;
 
@@ -3099,7 +3097,6 @@ dummy_func(
             _CHECK_FUNCTION_EXACT_ARGS +
             _CHECK_STACK_SPACE +
             _INIT_CALL_PY_EXACT_ARGS +
-            _SET_IP +  // Tier 2 only; special-cased oparg
             _SAVE_CURRENT_IP +  // Sets frame->prev_instr
             _PUSH_FRAME;
 
@@ -3109,7 +3106,6 @@ dummy_func(
             _CHECK_FUNCTION_EXACT_ARGS +
             _CHECK_STACK_SPACE +
             _INIT_CALL_PY_EXACT_ARGS +
-            _SET_IP +  // Tier 2 only; special-cased oparg
             _SAVE_CURRENT_IP +  // Sets frame->prev_instr
             _PUSH_FRAME;
 
@@ -3948,17 +3944,13 @@ dummy_func(
         }
 
         op(_SET_IP, (--)) {
+            TIER_TWO_ONLY
             frame->prev_instr = ip_offset + oparg;
         }
 
         op(_SAVE_CURRENT_IP, (--)) {
-            #if TIER_ONE
+            TIER_ONE_ONLY
             frame->prev_instr = next_instr - 1;
-            #endif
-            #if TIER_TWO
-            // Relies on a preceding _SET_IP
-            frame->prev_instr--;
-            #endif
         }
 
         op(_EXIT_TRACE, (--)) {
diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h
index 872e0a2..bd28126 100644
--- a/Python/ceval_macros.h
+++ b/Python/ceval_macros.h
@@ -372,6 +372,9 @@ static inline void _Py_LeaveRecursiveCallPy(PyThreadState *tstate)  {
 /* Marker to specify tier 1 only instructions */
 #define TIER_ONE_ONLY
 
+/* Marker to specify tier 2 only instructions */
+#define TIER_TWO_ONLY
+
 /* Implementation of "macros" that modify the instruction pointer,
  * stack pointer, or frame pointer.
  * These need to treated differently by tier 1 and 2. */
diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index e2f4f98..119e77b 100644
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -3270,21 +3270,11 @@
         }
 
         case _SET_IP: {
+            TIER_TWO_ONLY
             frame->prev_instr = ip_offset + oparg;
             break;
         }
 
-        case _SAVE_CURRENT_IP: {
-            #if TIER_ONE
-            frame->prev_instr = next_instr - 1;
-            #endif
-            #if TIER_TWO
-            // Relies on a preceding _SET_IP
-            frame->prev_instr--;
-            #endif
-            break;
-        }
-
         case _EXIT_TRACE: {
             frame->prev_instr--;  // Back up to just before destination
             _PyFrame_SetStackPointer(frame, stack_pointer);
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index eac1368..8ae9bd2 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -989,13 +989,8 @@
             PyObject *retval;
             // _SAVE_CURRENT_IP
             {
-                #if TIER_ONE
+                TIER_ONE_ONLY
                 frame->prev_instr = next_instr - 1;
-                #endif
-                #if TIER_TWO
-                // Relies on a preceding _SET_IP
-                frame->prev_instr--;
-                #endif
             }
             // _POP_FRAME
             retval = stack_pointer[-1];
@@ -1056,13 +1051,8 @@
             }
             // _SAVE_CURRENT_IP
             {
-                #if TIER_ONE
+                TIER_ONE_ONLY
                 frame->prev_instr = next_instr - 1;
-                #endif
-                #if TIER_TWO
-                // Relies on a preceding _SET_IP
-                frame->prev_instr--;
-                #endif
             }
             // _POP_FRAME
             retval = value;
@@ -3941,13 +3931,8 @@
             // _SAVE_CURRENT_IP
             next_instr += 3;
             {
-                #if TIER_ONE
+                TIER_ONE_ONLY
                 frame->prev_instr = next_instr - 1;
-                #endif
-                #if TIER_TWO
-                // Relies on a preceding _SET_IP
-                frame->prev_instr--;
-                #endif
             }
             // _PUSH_FRAME
             STACK_SHRINK(oparg);
@@ -4019,13 +4004,8 @@
             // _SAVE_CURRENT_IP
             next_instr += 3;
             {
-                #if TIER_ONE
+                TIER_ONE_ONLY
                 frame->prev_instr = next_instr - 1;
-                #endif
-                #if TIER_TWO
-                // Relies on a preceding _SET_IP
-                frame->prev_instr--;
-                #endif
             }
             // _PUSH_FRAME
             STACK_SHRINK(oparg);
diff --git a/Python/optimizer.c b/Python/optimizer.c
index 65b9638..955ac81 100644
--- a/Python/optimizer.c
+++ b/Python/optimizer.c
@@ -648,6 +648,7 @@ pop_jump_if_bool:
                     uint32_t orig_oparg = oparg;  // For OPARG_TOP/BOTTOM
                     for (int i = 0; i < nuops; i++) {
                         oparg = orig_oparg;
+                        uint32_t uop = expansion->uops[i].uop;
                         uint64_t operand = 0;
                         // Add one to account for the actual opcode/oparg pair:
                         int offset = expansion->uops[i].offset + 1;
@@ -680,6 +681,7 @@ pop_jump_if_bool:
                                 break;
                             case OPARG_SET_IP:  // op==_SET_IP; oparg=next instr
                                 oparg = INSTR_IP(instr + offset, code);
+                                uop = _SET_IP;
                                 break;
 
                             default:
@@ -690,8 +692,8 @@ pop_jump_if_bool:
                                         expansion->uops[i].offset);
                                 Py_FatalError("garbled expansion");
                         }
-                        ADD_TO_TRACE(expansion->uops[i].uop, oparg, operand);
-                        if (expansion->uops[i].uop == _POP_FRAME) {
+                        ADD_TO_TRACE(uop, oparg, operand);
+                        if (uop == _POP_FRAME) {
                             TRACE_STACK_POP();
                             DPRINTF(2,
                                 "Returning to %s (%s:%d) at byte offset %d\n",
@@ -701,7 +703,7 @@ pop_jump_if_bool:
                                 2 * INSTR_IP(instr, code));
                             goto top;
                         }
-                        if (expansion->uops[i].uop == _PUSH_FRAME) {
+                        if (uop == _PUSH_FRAME) {
                             assert(i + 1 == nuops);
                             int func_version_offset =
                                 offsetof(_PyCallCache, func_version)/sizeof(_Py_CODEUNIT)
diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py
index dbb1641..04d617b 100644
--- a/Tools/cases_generator/generate_cases.py
+++ b/Tools/cases_generator/generate_cases.py
@@ -658,7 +658,7 @@ class Generator(Analyzer):
         for part in parts:
             if isinstance(part, Component):
                 # All component instructions must be viable uops
-                if not part.instr.is_viable_uop():
+                if not part.instr.is_viable_uop() and part.instr.name != "_SAVE_CURRENT_IP":
                     # This note just reminds us about macros that cannot
                     # be expanded to Tier 2 uops. It is not an error.
                     # It is sometimes emitted for macros that have a
@@ -671,8 +671,8 @@ class Generator(Analyzer):
                         )
                     return
                 if not part.active_caches:
-                    if part.instr.name == "_SET_IP":
-                        size, offset = OPARG_SIZES["OPARG_SET_IP"], cache_offset
+                    if part.instr.name == "_SAVE_CURRENT_IP":
+                        size, offset = OPARG_SIZES["OPARG_SET_IP"], cache_offset - 1
                     else:
                         size, offset = OPARG_SIZES["OPARG_FULL"], 0
                 else: