Add no_save_ip annotation to indicate that an instruction shouldn't s…

…ave the IP to frame->instr_ptr
python · Jan 3, 2025 · 511733c · 511733c
1 parent 29da92e
commit 511733c
Show file tree

Hide file tree

Showing 11 changed files with 31 additions and 15 deletions.
diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h
diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
@@ -60,6 +60,8 @@
 #define specializing
 #define split
 #define replicate(TIMES)
+#define tier1
+#define no_save_ip
 
 // Dummy variables for stack effects.
 static PyObject *value, *value1, *value2, *left, *right, *res, *sum, *prod, *sub;
@@ -336,19 +338,18 @@ dummy_func(
             res = PyStackRef_NULL;
         }
 
-        inst(END_FOR, (value -- )) {
+        no_save_ip inst(END_FOR, (value -- )) {
             /* Don't update instr_ptr, so that POP_ITER sees
              * the FOR_ITER as the previous instruction.
              * This has the benign side effect that if value is
              * finalized it will see the location as the FOR_ITER's.
              */
-            frame->instr_ptr = prev_instr;
             PyStackRef_CLOSE(value);
         }
 
         macro(POP_ITER) = POP_TOP;
 
-        tier1 inst(INSTRUMENTED_END_FOR, (receiver, value -- receiver)) {
+        no_save_ip tier1 inst(INSTRUMENTED_END_FOR, (receiver, value -- receiver)) {
             /* Need to create a fake StopIteration error here,
              * to conform to PEP 380 */
             if (PyStackRef_GenCheck(receiver)) {

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
@@ -622,8 +622,14 @@ translate_bytecode_to_trace(
             goto done;
         }
         assert(opcode != ENTER_EXECUTOR && opcode != EXTENDED_ARG);
-        RESERVE_RAW(2, "_CHECK_VALIDITY_AND_SET_IP");
-        ADD_TO_TRACE(_CHECK_VALIDITY_AND_SET_IP, 0, (uintptr_t)instr, target);
+        if (OPCODE_HAS_NO_SAVE_IP(opcode)) {
+            RESERVE_RAW(2, "_CHECK_VALIDITY");
+            ADD_TO_TRACE(_CHECK_VALIDITY, 0, 0, target);
+        }
+        else {
+            RESERVE_RAW(2, "_CHECK_VALIDITY_AND_SET_IP");
+            ADD_TO_TRACE(_CHECK_VALIDITY_AND_SET_IP, 0, (uintptr_t)instr, target);
+        }
 
         /* Special case the first instruction,
          * so that we can guarantee forward progress */

diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py
@@ -27,6 +27,7 @@ class Properties:
     oparg_and_1: bool = False
     const_oparg: int = -1
     needs_prev: bool = False
+    no_save_ip: bool = False
 
     def dump(self, indent: str) -> None:
         simple_properties = self.__dict__.copy()
@@ -60,6 +61,7 @@ def from_list(properties: list["Properties"]) -> "Properties":
             side_exit=any(p.side_exit for p in properties),
             pure=all(p.pure for p in properties),
             needs_prev=any(p.needs_prev for p in properties),
+            no_save_ip=all(p.no_save_ip for p in properties),
         )
 
     @property
@@ -87,6 +89,7 @@ def escapes(self) -> bool:
     has_free=False,
     side_exit=False,
     pure=True,
+    no_save_ip=False,
 )
 
 
@@ -829,6 +832,7 @@ def compute_properties(op: parser.InstDef) -> Properties:
         and not has_free,
         has_free=has_free,
         pure="pure" in op.annotations,
+        no_save_ip="no_save_ip" in op.annotations,
         tier=tier_variable(op),
         needs_prev=variable_used(op, "prev_instr"),
     )

diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py
@@ -583,6 +583,8 @@ def cflags(p: Properties) -> str:
         flags.append("HAS_ESCAPES_FLAG")
     if p.pure:
         flags.append("HAS_PURE_FLAG")
+    if p.no_save_ip:
+        flags.append("HAS_NO_SAVE_IP_FLAG")
     if p.oparg_and_1:
         flags.append("HAS_OPARG_AND_1_FLAG")
     if flags:

diff --git a/Tools/cases_generator/lexer.py b/Tools/cases_generator/lexer.py
@@ -226,6 +226,7 @@ def choice(*opts: str) -> str:
     "replicate",
     "tier1",
     "tier2",
+    "no_save_ip",
 }
 
 __all__ = []

diff --git a/Tools/cases_generator/opcode_metadata_generator.py b/Tools/cases_generator/opcode_metadata_generator.py
@@ -53,6 +53,7 @@
     "PASSTHROUGH",
     "OPARG_AND_1",
     "ERROR_NO_POP",
+    "NO_SAVE_IP",
 ]
 
 

diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py
@@ -151,9 +151,12 @@ def generate_tier1(
         if inst.properties.needs_prev:
             out.emit(f"_Py_CODEUNIT* const prev_instr = frame->instr_ptr;\n")
         if needs_this and not inst.is_target:
-            out.emit(f"_Py_CODEUNIT* const this_instr = frame->instr_ptr = next_instr;\n")
+            if inst.properties.no_save_ip:
+                out.emit(f"_Py_CODEUNIT* const this_instr = next_instr;\n")
+            else:
+                out.emit(f"_Py_CODEUNIT* const this_instr = frame->instr_ptr = next_instr;\n")
             out.emit(unused_guard)
-        else:
+        elif not inst.properties.no_save_ip:
             out.emit(f"frame->instr_ptr = next_instr;\n")
         out.emit(f"next_instr += {inst.size};\n")
         out.emit(f"INSTRUCTION_STATS({name});\n")