Stop special-casing _PUSH_FRAME altogether

Instead, we special-case SAVE_IP: - Its Tier 2 expansion sets oparg to the instruction offset - In Tier 1 it is a no-op (and skipped if present in a macro)
python · gvanrossum · Aug 5, 2023 · Aug 9, 2023 · Aug 9, 2023 · Aug 10, 2023
commit 94630d49b3283657953d44b7fbe2a8a36d624721
@@ -3005,6 +3005,7 @@ dummy_func(
             _CHECK_FUNCTION_EXACT_ARGS +
             _CHECK_STACK_SPACE +
             _INIT_CALL_PY_EXACT_ARGS +
+            SAVE_IP +  // Tier 2 only; special-cased oparg
             _PUSH_FRAME;
 
         inst(CALL_PY_WITH_DEFAULTS, (unused/1, func_version/2, callable, self_or_null, args[oparg] -- unused)) {

@@ -443,13 +443,6 @@ def write_macro_expansions(
         expansions: list[tuple[str, int, int]] = []  # [(name, size, offset), ...]
         for part in parts:
             if isinstance(part, Component):
-                # _PUSH_FRAME is super special; it expands to SAVE_IP(next_instr) + _PUSH_FRAME
-                if part.instr.name == "_PUSH_FRAME":
-                    expansions.append(
-                        ("SAVE_IP", OPARG_SIZES["OPARG_SAVE_IP"], cache_offset)
-                    )
-                    expansions.append(("_PUSH_FRAME", OPARG_SIZES["OPARG_FULL"], 0))
-                    continue
                 # All component instructions must be viable uops
                 if not part.instr.is_viable_uop():
                     # This note just reminds us about macros that cannot
@@ -463,7 +456,10 @@ def write_macro_expansions(
                     )
                     return
                 if not part.active_caches:
-                    size, offset = OPARG_SIZES["OPARG_FULL"], 0
+                    if part.instr.name == "SAVE_IP":
+                        size, offset = OPARG_SIZES["OPARG_SAVE_IP"], cache_offset
+                    else:
+                        size, offset = OPARG_SIZES["OPARG_FULL"], 0
                 else:
                     # If this assert triggers, is_viable_uops() lied
                     assert len(part.active_caches) == 1, (name, part.instr.name)

@@ -147,6 +147,8 @@ class EffectManager:
     # Track offsets from stack pointer
     min_offset: StackOffset
     final_offset: StackOffset
+    # Link to previous manager
+    pred: "EffectManager | None" = None
 
     def __init__(
         self,
@@ -168,7 +170,8 @@ def __init__(
             self.pokes.append(StackItem(offset=self.final_offset.clone(), effect=eff))
             self.final_offset.higher(eff)
 
-        if pred:
+        self.pred = pred
+        while pred:
             # Replace push(x) + pop(y) with copy(x, y).
             # Check that the sources and destinations are disjoint.
             sources: set[str] = set()
@@ -193,6 +196,11 @@ def __init__(
                 sources,
                 destinations,
             )
+            # See if we can get more copies of a earlier predecessor.
+            if self.peeks and not pred.pokes and not pred.peeks:
+                pred = pred.pred
+            else:
+                pred = None  # Break
 
     def adjust_deeper(self, eff: StackEffect) -> None:
         for peek in self.peeks:
@@ -305,7 +313,11 @@ def write_single_instr(
 def write_macro_instr(
     mac: MacroInstruction, out: Formatter, family: Family | None
 ) -> None:
-    parts = [part for part in mac.parts if isinstance(part, Component)]
+    parts = [
+        part
+        for part in mac.parts
+        if isinstance(part, Component) and part.instr.name != "SAVE_IP"
+    ]
     out.emit("")
     with out.block(f"TARGET({mac.name})"):
         if mac.predicted: