GH-109214: Convert _SAVE_CURRENT_IP to _SET_IP in tier 2 trace creati…

…on. (GH-110755)
python · Oct 12, 2023 · 19b7ead · 19b7ead
1 parent fb7843e
commit 19b7ead
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 59 deletions.
diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h
diff --git a/Python/abstract_interp_cases.c.h b/Python/abstract_interp_cases.c.h
diff --git 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
@@ -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
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
diff --git a/Python/optimizer.c b/Python/optimizer.c
@@ -648,6 +648,7 @@ translate_bytecode_to_trace(
                     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 @@ translate_bytecode_to_trace(
                                 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 @@ translate_bytecode_to_trace(
                                         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 @@ translate_bytecode_to_trace(
                                 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
@@ -658,7 +658,7 @@ def write_macro_expansions(
         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 @@ def write_macro_expansions(
                         )
                     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: