GH-121784: Generate an error during code gen if a variable is marked `unused`, but is used and thus cached in a prior uop. (#121788)

* Reject uop definitions that declare values as 'unused' that are already cached by prior uops

* Track which variables are defined and only load from memory when needed

* Support explicit `flush` in macro definitions. 

* Make sure stack is flushed in where needed.

5 files changed, 112 insertions, 57 deletions

diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py
index ec365ba..c6f044e 100644
--- a/Tools/cases_generator/analyzer.py
+++ b/Tools/cases_generator/analyzer.py
@@ -78,7 +78,7 @@ SKIP_PROPERTIES = Properties(
     uses_locals=False,
     has_free=False,
     side_exit=False,
-    pure=False,
+    pure=True,
 )
 
 
@@ -96,6 +96,20 @@ class Skip:
         return SKIP_PROPERTIES
 
 
+class Flush:
+
+    @property
+    def properties(self) -> Properties:
+        return SKIP_PROPERTIES
+
+    @property
+    def name(self) -> str:
+        return "flush"
+
+    @property
+    def size(self) -> int:
+        return 0
+
 @dataclass
 class StackItem:
     name: str
@@ -103,6 +117,7 @@ class StackItem:
     condition: str | None
     size: str
     peek: bool = False
+    used: bool = False
 
     def __str__(self) -> str:
         cond = f" if ({self.condition})" if self.condition else ""
@@ -133,7 +148,6 @@ class CacheEntry:
     def __str__(self) -> str:
         return f"{self.name}/{self.size}"
 
-
 @dataclass
 class Uop:
     name: str
@@ -195,7 +209,7 @@ class Uop:
         return False
 
 
-Part = Uop | Skip
+Part = Uop | Skip | Flush
 
 
 @dataclass
@@ -306,6 +320,16 @@ def analyze_stack(op: parser.InstDef | parser.Pseudo, replace_op_arg_1: str | No
     for input, output in zip(inputs, outputs):
         if input.name == output.name:
             input.peek = output.peek = True
+    if isinstance(op, parser.InstDef):
+        output_names = [out.name for out in outputs]
+        for input in inputs:
+            if (variable_used(op, input.name) or
+                variable_used(op, "DECREF_INPUTS") or
+                (not input.peek and input.name in output_names)):
+                input.used = True
+        for output in outputs:
+            if variable_used(op, output.name):
+                output.used = True
     return StackEffect(inputs, outputs)
 
 
@@ -324,7 +348,13 @@ def analyze_caches(inputs: list[parser.InputEffect]) -> list[CacheEntry]:
 def variable_used(node: parser.InstDef, name: str) -> bool:
     """Determine whether a variable with a given name is used in a node."""
     return any(
-        token.kind == "IDENTIFIER" and token.text == name for token in node.tokens
+        token.kind == "IDENTIFIER" and token.text == name for token in node.block.tokens
+    )
+
+def oparg_used(node: parser.InstDef) -> bool:
+    """Determine whether `oparg` is used in a node."""
+    return any(
+        token.kind == "IDENTIFIER" and token.text == "oparg" for token in node.tokens
     )
 
 def tier_variable(node: parser.InstDef) -> int | None:
@@ -570,7 +600,7 @@ def compute_properties(op: parser.InstDef) -> Properties:
         error_without_pop=error_without_pop,
         deopts=deopts_if,
         side_exit=exits_if,
-        oparg=variable_used(op, "oparg"),
+        oparg=oparg_used(op),
         jumps=variable_used(op, "JUMPBY"),
         eval_breaker=variable_used(op, "CHECK_EVAL_BREAKER"),
         ends_with_eval_breaker=eval_breaker_at_end(op),
@@ -689,13 +719,16 @@ def desugar_inst(
 def add_macro(
     macro: parser.Macro, instructions: dict[str, Instruction], uops: dict[str, Uop]
 ) -> None:
-    parts: list[Uop | Skip] = []
+    parts: list[Part] = []
     for part in macro.uops:
         match part:
             case parser.OpName():
-                if part.name not in uops:
-                    analysis_error(f"No Uop named {part.name}", macro.tokens[0])
-                parts.append(uops[part.name])
+                if part.name == "flush":
+                    parts.append(Flush())
+                else:
+                    if part.name not in uops:
+                        raise analysis_error(f"No Uop named {part.name}", macro.tokens[0])
+                    parts.append(uops[part.name])
             case parser.CacheEffect():
                 parts.append(Skip(part.size))
             case _:
diff --git a/Tools/cases_generator/optimizer_generator.py b/Tools/cases_generator/optimizer_generator.py
index 2775216..6a66693 100644
--- a/Tools/cases_generator/optimizer_generator.py
+++ b/Tools/cases_generator/optimizer_generator.py
@@ -23,7 +23,7 @@ from generators_common import (
 from cwriter import CWriter
 from typing import TextIO, Iterator
 from lexer import Token
-from stack import Stack, SizeMismatch
+from stack import Stack, StackError
 
 DEFAULT_OUTPUT = ROOT / "Python/optimizer_cases.c.h"
 DEFAULT_ABSTRACT_INPUT = (ROOT / "Python/optimizer_bytecodes.c").absolute().as_posix()
@@ -141,7 +141,7 @@ def write_uop(
                     out.emit(stack.push(var))
         out.start_line()
         stack.flush(out, cast_type="_Py_UopsSymbol *", extract_bits=True)
-    except SizeMismatch as ex:
+    except StackError as ex:
         raise analysis_error(ex.args[0], uop.body[0])
 
 
diff --git a/Tools/cases_generator/stack.py b/Tools/cases_generator/stack.py
index ebe62df..f497fa3 100644
--- a/Tools/cases_generator/stack.py
+++ b/Tools/cases_generator/stack.py
@@ -114,7 +114,7 @@ class StackOffset:
         self.pushed = []
 
 
-class SizeMismatch(Exception):
+class StackError(Exception):
     pass
 
 
@@ -134,27 +134,29 @@ class Stack:
         if self.variables:
             popped = self.variables.pop()
             if popped.size != var.size:
-                raise SizeMismatch(
+                raise StackError(
                     f"Size mismatch when popping '{popped.name}' from stack to assign to {var.name}. "
                     f"Expected {var.size} got {popped.size}"
                 )
-            if popped.name == var.name:
+            if var.name in UNUSED:
+                if popped.name not in UNUSED and popped.name in self.defined:
+                    raise StackError(f"Value is declared unused, but is already cached by prior operation")
                 return ""
-            elif popped.name in UNUSED:
+            if popped.name in UNUSED or popped.name not in self.defined:
                 self.defined.add(var.name)
                 return (
                     f"{var.name} = {indirect}stack_pointer[{self.top_offset.to_c()}];\n"
                 )
-            elif var.name in UNUSED:
-                return ""
             else:
                 self.defined.add(var.name)
-                return f"{var.name} = {popped.name};\n"
+                if popped.name == var.name:
+                    return ""
+                else:
+                    return f"{var.name} = {popped.name};\n"
         self.base_offset.pop(var)
-        if var.name in UNUSED:
+        if var.name in UNUSED or not var.used:
             return ""
-        else:
-            self.defined.add(var.name)
+        self.defined.add(var.name)
         cast = f"({var.type})" if (not indirect and var.type) else ""
         bits = ".bits" if cast and not extract_bits else ""
         assign = (
@@ -178,6 +180,8 @@ class Stack:
             return f"{var.name} = &stack_pointer[{c_offset}];\n"
         else:
             self.top_offset.push(var)
+            if var.used:
+                self.defined.add(var.name)
             return ""
 
     def flush(self, out: CWriter, cast_type: str = "uintptr_t", extract_bits: bool = False) -> None:
diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py
index 85be673..5dec66e 100644
--- a/Tools/cases_generator/tier1_generator.py
+++ b/Tools/cases_generator/tier1_generator.py
@@ -12,6 +12,7 @@ from analyzer import (
     Part,
     analyze_files,
     Skip,
+    Flush,
     analysis_error,
     StackItem,
 )
@@ -24,7 +25,7 @@ from generators_common import (
 )
 from cwriter import CWriter
 from typing import TextIO
-from stack import Stack, SizeMismatch
+from stack import Stack, StackError
 
 
 DEFAULT_OUTPUT = ROOT / "Python/generated_cases.c.h"
@@ -32,30 +33,39 @@ DEFAULT_OUTPUT = ROOT / "Python/generated_cases.c.h"
 
 FOOTER = "#undef TIER_ONE\n"
 
+def declare_variable(var: StackItem, out: CWriter) -> None:
+    type, null = type_and_null(var)
+    space = " " if type[-1].isalnum() else ""
+    if var.condition:
+        out.emit(f"{type}{space}{var.name} = {null};\n")
+    else:
+        out.emit(f"{type}{space}{var.name};\n")
 
-def declare_variables(inst: Instruction, out: CWriter) -> None:
-    variables = {"unused"}
-    for uop in inst.parts:
-        if isinstance(uop, Uop):
-            for var in reversed(uop.stack.inputs):
-                if var.name not in variables:
-                    variables.add(var.name)
-                    type, null = type_and_null(var)
-                    space = " " if type[-1].isalnum() else ""
-                    if var.condition:
-                        out.emit(f"{type}{space}{var.name} = {null};\n")
-                    else:
-                        out.emit(f"{type}{space}{var.name};\n")
-            for var in uop.stack.outputs:
-                if var.name not in variables:
-                    variables.add(var.name)
-                    type, null = type_and_null(var)
-                    space = " " if type[-1].isalnum() else ""
-                    if var.condition:
-                        out.emit(f"{type}{space}{var.name} = {null};\n")
-                    else:
-                        out.emit(f"{type}{space}{var.name};\n")
 
+def declare_variables(inst: Instruction, out: CWriter) -> None:
+    stack = Stack()
+    for part in inst.parts:
+        if not isinstance(part, Uop):
+            continue
+        try:
+            for var in reversed(part.stack.inputs):
+                stack.pop(var)
+            for var in part.stack.outputs:
+                 stack.push(var)
+        except StackError as ex:
+            raise analysis_error(ex.args[0], part.body[0]) from None
+    required = set(stack.defined)
+    for part in inst.parts:
+        if not isinstance(part, Uop):
+            continue
+        for var in part.stack.inputs:
+            if var.name in required:
+                required.remove(var.name)
+                declare_variable(var, out)
+        for var in part.stack.outputs:
+            if var.name in required:
+                required.remove(var.name)
+                declare_variable(var, out)
 
 def write_uop(
     uop: Part, out: CWriter, offset: int, stack: Stack, inst: Instruction, braces: bool
@@ -65,6 +75,10 @@ def write_uop(
         entries = "entries" if uop.size > 1 else "entry"
         out.emit(f"/* Skip {uop.size} cache {entries} */\n")
         return offset + uop.size
+    if isinstance(uop, Flush):
+        out.emit(f"// flush\n")
+        stack.flush(out)
+        return offset
     try:
         out.start_line()
         if braces:
@@ -99,15 +113,15 @@ def write_uop(
             out.emit("}\n")
         # out.emit(stack.as_comment() + "\n")
         return offset
-    except SizeMismatch as ex:
-        raise analysis_error(ex.args[0], uop.body[0])
+    except StackError as ex:
+        raise analysis_error(ex.args[0], uop.body[0]) from None
 
 
 def uses_this(inst: Instruction) -> bool:
     if inst.properties.needs_this:
         return True
     for uop in inst.parts:
-        if isinstance(uop, Skip):
+        if not isinstance(uop, Uop):
             continue
         for cache in uop.caches:
             if cache.name != "unused":
diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py
index 7a69aa6..88ad0fd 100644
--- a/Tools/cases_generator/tier2_generator.py
+++ b/Tools/cases_generator/tier2_generator.py
@@ -25,17 +25,17 @@ from generators_common import (
 from cwriter import CWriter
 from typing import TextIO, Iterator
 from lexer import Token
-from stack import Stack, SizeMismatch
+from stack import Stack, StackError
 
 DEFAULT_OUTPUT = ROOT / "Python/executor_cases.c.h"
 
 
 def declare_variable(
-    var: StackItem, uop: Uop, variables: set[str], out: CWriter
+    var: StackItem, uop: Uop, required: set[str], out: CWriter
 ) -> None:
-    if var.name in variables:
+    if var.name not in required:
         return
-    variables.add(var.name)
+    required.remove(var.name)
     type, null = type_and_null(var)
     space = " " if type[-1].isalnum() else ""
     if var.condition:
@@ -49,12 +49,16 @@ def declare_variable(
 
 
 def declare_variables(uop: Uop, out: CWriter) -> None:
-    variables = {"unused"}
+    stack = Stack()
     for var in reversed(uop.stack.inputs):
-        declare_variable(var, uop, variables, out)
+        stack.pop(var)
     for var in uop.stack.outputs:
-        declare_variable(var, uop, variables, out)
-
+            stack.push(var)
+    required = set(stack.defined)
+    for var in reversed(uop.stack.inputs):
+        declare_variable(var, uop, required, out)
+    for var in uop.stack.outputs:
+        declare_variable(var, uop, required, out)
 
 def tier2_replace_error(
     out: CWriter,
@@ -177,8 +181,8 @@ def write_uop(uop: Uop, out: CWriter, stack: Stack) -> None:
         if uop.properties.stores_sp:
             for i, var in enumerate(uop.stack.outputs):
                 out.emit(stack.push(var))
-    except SizeMismatch as ex:
-        raise analysis_error(ex.args[0], uop.body[0])
+    except StackError as ex:
+        raise analysis_error(ex.args[0], uop.body[0]) from None
 
 
 SKIPS = ("_EXTENDED_ARG",)