From 70272dc230de05eb204b62c70a5fdafd6a1fe2ab Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 6 Aug 2024 17:20:22 +0000 Subject: [PATCH 1/5] gh-118926: Spill deferred references to stack in cases generator This automatically spills the results from `_PyStackRef_FromPyObjectNew` to the in-memory stack so that the deferred references are visible to the GC before we make any possibly escaping call. Co-authored-by: Ken Jin --- Python/bytecodes.c | 2 +- Python/executor_cases.c.h | 34 ++++++------ Python/generated_cases.c.h | 40 ++++++++------ Tools/cases_generator/analyzer.py | 45 ++++++++++++++++ Tools/cases_generator/generators_common.py | 25 +++++++++ Tools/cases_generator/lexer.py | 2 +- Tools/cases_generator/optimizer_generator.py | 16 +++--- Tools/cases_generator/stack.py | 56 +++++++++++++++----- Tools/cases_generator/tier1_generator.py | 22 ++++---- Tools/cases_generator/tier2_generator.py | 16 +++--- 10 files changed, 184 insertions(+), 74 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 9a1af0e920188b..6edac1f6870af2 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1424,7 +1424,7 @@ dummy_func( "no locals found"); ERROR_IF(true, error); } - locals = PyStackRef_FromPyObjectNew(l);; + locals = PyStackRef_FromPyObjectNew(l); } inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) { diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index afc7786c9e434d..9f77e4b9b5883e 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1468,10 +1468,10 @@ } STAT_INC(UNPACK_SEQUENCE, hit); val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + stack_pointer[0] = val0; val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); - PyStackRef_CLOSE(seq); stack_pointer[-1] = val1; - stack_pointer[0] = val0; + PyStackRef_CLOSE(seq); stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; @@ -1611,7 +1611,7 @@ "no locals found"); if (true) JUMP_TO_ERROR(); } - locals = PyStackRef_FromPyObjectNew(l);; + locals = PyStackRef_FromPyObjectNew(l); stack_pointer[0] = locals; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -2421,8 +2421,8 @@ STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; attr = PyStackRef_FromPyObjectNew(attr_o); - PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; + PyStackRef_CLOSE(owner); break; } @@ -2443,8 +2443,8 @@ STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; attr = PyStackRef_FromPyObjectNew(attr_o); - PyStackRef_CLOSE(owner); stack_pointer[-1] = attr; + PyStackRef_CLOSE(owner); stack_pointer[0] = null; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -2480,9 +2480,9 @@ STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; null = PyStackRef_NULL; PyStackRef_CLOSE(owner); - stack_pointer[-1] = attr; break; } @@ -2496,9 +2496,9 @@ STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; null = PyStackRef_NULL; PyStackRef_CLOSE(owner); - stack_pointer[-1] = attr; stack_pointer[0] = null; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3436,8 +3436,8 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - self = owner; stack_pointer[-1] = attr; + self = owner; stack_pointer[0] = self; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3457,8 +3457,8 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - self = owner; stack_pointer[-1] = attr; + self = owner; stack_pointer[0] = self; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3522,8 +3522,8 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); - self = owner; stack_pointer[-1] = attr; + self = owner; stack_pointer[0] = self; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -3545,8 +3545,10 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); PyObject *self = ((PyMethodObject *)callable_o)->im_self; maybe_self = PyStackRef_FromPyObjectNew(self); + stack_pointer[-1 - oparg] = maybe_self; PyObject *method = ((PyMethodObject *)callable_o)->im_func; func = PyStackRef_FromPyObjectNew(method); + stack_pointer[-2 - oparg] = func; /* Make sure that callable and all args are in memory */ args[-2] = func; args[-1] = maybe_self; @@ -3556,8 +3558,6 @@ func = callable; maybe_self = self_or_null; } - stack_pointer[-2 - oparg] = func; - stack_pointer[-1 - oparg] = maybe_self; break; } @@ -3665,11 +3665,11 @@ assert(PyStackRef_IsNull(null)); assert(Py_TYPE(callable_o) == &PyMethod_Type); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + stack_pointer[-1 - oparg] = self; method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + stack_pointer[-2 - oparg] = method; assert(PyStackRef_FunctionCheck(method)); PyStackRef_CLOSE(callable); - stack_pointer[-2 - oparg] = method; - stack_pointer[-1 - oparg] = self; break; } @@ -3762,10 +3762,10 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); STAT_INC(CALL, hit); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + stack_pointer[-1 - oparg] = self; func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); - PyStackRef_CLOSE(callable); stack_pointer[-2 - oparg] = func; - stack_pointer[-1 - oparg] = self; + PyStackRef_CLOSE(callable); break; } @@ -5087,8 +5087,8 @@ _PyStackRef null; PyObject *ptr = (PyObject *)CURRENT_OPERAND(); value = PyStackRef_FromPyObjectNew(ptr); - null = PyStackRef_NULL; stack_pointer[0] = value; + null = PyStackRef_NULL; stack_pointer[1] = null; stack_pointer += 2; assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index f670353cdbde56..07f4518c741801 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -867,8 +867,10 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); PyObject *self = ((PyMethodObject *)callable_o)->im_self; maybe_self = PyStackRef_FromPyObjectNew(self); + stack_pointer[-1 - oparg] = maybe_self; PyObject *method = ((PyMethodObject *)callable_o)->im_func; func = PyStackRef_FromPyObjectNew(method); + stack_pointer[-2 - oparg] = func; /* Make sure that callable and all args are in memory */ args[-2] = func; args[-1] = maybe_self; @@ -1063,12 +1065,12 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); STAT_INC(CALL, hit); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + stack_pointer[-1 - oparg] = self; func = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + stack_pointer[-2 - oparg] = func; PyStackRef_CLOSE(callable); } // flush - stack_pointer[-2 - oparg] = func; - stack_pointer[-1 - oparg] = self; // _CHECK_FUNCTION_VERSION callable = stack_pointer[-2 - oparg]; { @@ -1172,13 +1174,13 @@ assert(PyStackRef_IsNull(null)); assert(Py_TYPE(callable_o) == &PyMethod_Type); self = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_self); + stack_pointer[-1 - oparg] = self; method = PyStackRef_FromPyObjectNew(((PyMethodObject *)callable_o)->im_func); + stack_pointer[-2 - oparg] = method; assert(PyStackRef_FunctionCheck(method)); PyStackRef_CLOSE(callable); } // flush - stack_pointer[-2 - oparg] = method; - stack_pointer[-1 - oparg] = self; // _PY_FRAME_GENERAL args = &stack_pointer[-oparg]; self_or_null = stack_pointer[-1 - oparg]; @@ -2534,6 +2536,7 @@ int matches = PyErr_GivenExceptionMatches(exc_value, PyExc_StopIteration); if (matches) { value = PyStackRef_FromPyObjectNew(((PyStopIterationObject *)exc_value)->value); + stack_pointer[-2] = value; PyStackRef_CLOSE(sub_iter_st); PyStackRef_CLOSE(last_sent_val_st); PyStackRef_CLOSE(exc_value_st); @@ -2545,7 +2548,6 @@ goto exception_unwind; } stack_pointer[-3] = none; - stack_pointer[-2] = value; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); @@ -3330,8 +3332,8 @@ assert(seq); assert(it->it_index < PyList_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, it->it_index++)); + stack_pointer[0] = next; } - stack_pointer[0] = next; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); @@ -3423,8 +3425,8 @@ assert(seq); assert(it->it_index < PyTuple_GET_SIZE(seq)); next = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq, it->it_index++)); + stack_pointer[0] = next; } - stack_pointer[0] = next; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); @@ -3634,8 +3636,10 @@ PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable); PyObject *self = ((PyMethodObject *)callable_o)->im_self; maybe_self = PyStackRef_FromPyObjectNew(self); + stack_pointer[-1 - oparg] = maybe_self; PyObject *method = ((PyMethodObject *)callable_o)->im_func; func = PyStackRef_FromPyObjectNew(method); + stack_pointer[-2 - oparg] = func; /* Make sure that callable and all args are in memory */ args[-2] = func; args[-1] = maybe_self; @@ -4071,6 +4075,7 @@ // _LOAD_CONST { value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); + stack_pointer[0] = value; } // _RETURN_VALUE_EVENT val = value; @@ -4453,10 +4458,10 @@ STAT_INC(LOAD_ATTR, hit); assert(descr != NULL); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; null = PyStackRef_NULL; PyStackRef_CLOSE(owner); } - stack_pointer[-1] = attr; if (oparg & 1) stack_pointer[0] = null; stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); @@ -4577,9 +4582,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; self = owner; } - stack_pointer[-1] = attr; stack_pointer[0] = self; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -4613,9 +4618,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; self = owner; } - stack_pointer[-1] = attr; stack_pointer[0] = self; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -4661,9 +4666,9 @@ assert(descr != NULL); assert(_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; self = owner; } - stack_pointer[-1] = attr; stack_pointer[0] = self; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -4739,8 +4744,8 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; } - stack_pointer[-1] = attr; DISPATCH(); } @@ -4781,8 +4786,8 @@ assert(descr != NULL); PyStackRef_CLOSE(owner); attr = PyStackRef_FromPyObjectNew(descr); + stack_pointer[-1] = attr; } - stack_pointer[-1] = attr; DISPATCH(); } @@ -4878,10 +4883,10 @@ STAT_INC(LOAD_ATTR, hit); null = PyStackRef_NULL; attr = PyStackRef_FromPyObjectNew(attr_o); + stack_pointer[-1] = attr; PyStackRef_CLOSE(owner); } /* Skip 5 cache entries */ - stack_pointer[-1] = attr; if (oparg & 1) stack_pointer[0] = null; stack_pointer += (oparg & 1); assert(WITHIN_STACK_BOUNDS()); @@ -5290,7 +5295,7 @@ "no locals found"); if (true) goto error; } - locals = PyStackRef_FromPyObjectNew(l);; + locals = PyStackRef_FromPyObjectNew(l); stack_pointer[0] = locals; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -5959,6 +5964,7 @@ // _LOAD_CONST { value = PyStackRef_FromPyObjectNew(GETITEM(FRAME_CO_CONSTS, oparg)); + stack_pointer[0] = value; } // _RETURN_VALUE retval = value; @@ -7018,10 +7024,10 @@ DEOPT_IF(PyTuple_GET_SIZE(seq_o) != 2, UNPACK_SEQUENCE); STAT_INC(UNPACK_SEQUENCE, hit); val0 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 0)); + stack_pointer[0] = val0; val1 = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(seq_o, 1)); - PyStackRef_CLOSE(seq); stack_pointer[-1] = val1; - stack_pointer[0] = val0; + PyStackRef_CLOSE(seq); stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index f6625a3f7322d5..a3b3c23ca5f089 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -157,6 +157,7 @@ class Uop: annotations: list[str] stack: StackEffect caches: list[CacheEntry] + deferred_refs: dict[lexer.Token, str | None] body: list[lexer.Token] properties: Properties _size: int = -1 @@ -352,6 +353,47 @@ def analyze_caches(inputs: list[parser.InputEffect]) -> list[CacheEntry]: return [CacheEntry(i.name, int(i.size)) for i in caches] +def analyze_deferred_refs(node: parser.InstDef) -> dict[lexer.Token, str | None]: + """Look for PyStackRef_FromPyObjectNew() calls""" + + def find_assignment_target(idx) -> list[lexer.Token]: + """Find the tokens that make up the left-hand side of an assignment""" + offset = 1 + for tkn in reversed(node.block.tokens[:idx-1]): + if tkn.kind == "SEMI" or tkn.kind == "LBRACE" or tkn.kind == "RBRACE": + return node.block.tokens[idx-offset:idx-1] + offset += 1 + return [] + + refs: dict[lexer.Token, str | None] = {} + for idx, tkn in enumerate(node.block.tokens): + if tkn.kind != "IDENTIFIER" or tkn.text != "PyStackRef_FromPyObjectNew": + continue + + if node.block.tokens[idx-1].kind != "EQUALS": + raise analysis_error("Expected '=' before PyStackRef_FromPyObjectNew", tkn) + + lhs = find_assignment_target(idx) + if len(lhs) == 0: + raise analysis_error("PyStackRef_FromPyObjectNew() must be assigned to an output", tkn) + + if lhs[0].kind == "TIMES" or any(t.kind == "ARROW" or t.kind == "LBRACKET" for t in lhs[1:]): + # Don't handle: *ptr = ..., ptr->field = ..., or ptr[field] = ... + # Assume that they are visible to the GC. + refs[tkn] = None + continue + + if len(lhs) != 1 or lhs[0].kind != "IDENTIFIER": + raise analysis_error("PyStackRef_FromPyObjectNew() must be assigned to an output", tkn) + + name = lhs[0].text + if not any(var.name == name for var in node.outputs): + raise analysis_error(f"PyStackRef_FromPyObjectNew() must be assigned to an output, not '{text}'", tkn) + + refs[tkn] = name + + return refs + def variable_used(node: parser.InstDef, name: str) -> bool: """Determine whether a variable with a given name is used in a node.""" return any( @@ -632,6 +674,7 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo annotations=op.annotations, stack=analyze_stack(op), caches=analyze_caches(inputs), + deferred_refs=analyze_deferred_refs(op), body=op.block.tokens, properties=compute_properties(op), ) @@ -649,6 +692,7 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo annotations=op.annotations, stack=analyze_stack(op, bit), caches=analyze_caches(inputs), + deferred_refs=analyze_deferred_refs(op), body=op.block.tokens, properties=properties, ) @@ -671,6 +715,7 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo annotations=op.annotations, stack=analyze_stack(op), caches=analyze_caches(inputs), + deferred_refs=analyze_deferred_refs(op), body=op.block.tokens, properties=properties, ) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 2a339f8cd6bb66..4e2be53cd9d90a 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -6,6 +6,7 @@ Uop, Properties, StackItem, + analysis_error, ) from cwriter import CWriter from typing import Callable, Mapping, TextIO, Iterator @@ -75,6 +76,7 @@ def __init__(self, out: CWriter): "DECREF_INPUTS": self.decref_inputs, "CHECK_EVAL_BREAKER": self.check_eval_breaker, "SYNC_SP": self.sync_sp, + "PyStackRef_FromPyObjectNew": self.py_stack_ref_from_py_object_new, } self.out = out @@ -203,6 +205,29 @@ def check_eval_breaker( if not uop.properties.ends_with_eval_breaker: self.out.emit_at("CHECK_EVAL_BREAKER();", tkn) + def py_stack_ref_from_py_object_new( + self, + tkn: Token, + tkn_iter: Iterator[Token], + uop: Uop, + stack: Stack, + inst: Instruction | None, + ) -> None: + self.out.emit(tkn) + emit_to(self.out, tkn_iter, "SEMI") + self.out.emit(";\n") + + target = uop.deferred_refs[tkn] + if target is None: + # An assignment we don't handle, such as to a pointer or array. + return + + # Flush the assignment to the stack. Note that we don't flush the + # stack pointer here, and instead are currently relying on initializing + # unused portions of the stack to NULL. + stack.flush_single_var(self.out, target) + + def emit_tokens( self, uop: Uop, diff --git a/Tools/cases_generator/lexer.py b/Tools/cases_generator/lexer.py index 13aee94f2b957c..d5831593215f76 100644 --- a/Tools/cases_generator/lexer.py +++ b/Tools/cases_generator/lexer.py @@ -242,7 +242,7 @@ def make_syntax_error( return SyntaxError(message, (filename, line, column, line_text)) -@dataclass(slots=True) +@dataclass(slots=True, frozen=True) class Token: filename: str kind: str diff --git a/Tools/cases_generator/optimizer_generator.py b/Tools/cases_generator/optimizer_generator.py index e192b76b23319c..912280037b82e0 100644 --- a/Tools/cases_generator/optimizer_generator.py +++ b/Tools/cases_generator/optimizer_generator.py @@ -112,7 +112,14 @@ def write_uop( out.emit(code) if local.defined: locals[local.name] = local - out.emit(stack.define_output_arrays(prototype.stack.outputs)) + outputs: list[Local] = [] + for var in prototype.stack.outputs: + if var.name in locals: + local = locals[var.name] + else: + local = Local.local(var) + outputs.append(local) + out.emit(stack.define_outputs(outputs)) if debug: args = [] for var in prototype.stack.inputs: @@ -134,12 +141,7 @@ def write_uop( else: emit_default(out, uop) - for var in prototype.stack.outputs: - if var.name in locals: - local = locals[var.name] - else: - local = Local.local(var) - out.emit(stack.push(local)) + out.emit(stack.push_outputs()) out.start_line() stack.flush(out, cast_type="_Py_UopsSymbol *", extract_bits=True) except StackError as ex: diff --git a/Tools/cases_generator/stack.py b/Tools/cases_generator/stack.py index d2d598a120892d..01216a16d031c8 100644 --- a/Tools/cases_generator/stack.py +++ b/Tools/cases_generator/stack.py @@ -169,6 +169,7 @@ def __init__(self) -> None: self.top_offset = StackOffset.empty() self.base_offset = StackOffset.empty() self.variables: list[Local] = [] + self.outputs: list[Local] = [] self.defined: set[str] = set() def pop(self, var: StackItem, extract_bits: bool = False) -> tuple[str, Local]: @@ -244,33 +245,44 @@ def push(self, var: Local) -> str: self.defined.add(var.name) return "" - def define_output_arrays(self, outputs: list[StackItem]) -> str: + def define_outputs(self, outputs: list[Local]) -> str: res = [] top_offset = self.top_offset.copy() for var in outputs: - if var.is_array() and var.used and not var.peek: + if var.is_array() and var.item.used and not var.item.peek: c_offset = top_offset.to_c() - top_offset.push(var) res.append(f"{var.name} = &stack_pointer[{c_offset}];\n") - else: - top_offset.push(var) + self.outputs.append(var) + top_offset.push(var.item) return "\n".join(res) + def push_outputs(self) -> str: + res = [] + for var in self.outputs: + res.append(self.push(var)) + self.outputs = [] + return "".join(res) + + @staticmethod + def _do_emit(out: CWriter, var: Local, base_offset: StackOffset, + cast_type: str = "uintptr_t", extract_bits: bool = False) -> None: + cast = f"({cast_type})" if var.item.type else "" + bits = ".bits" if cast and not extract_bits else "" + if var.condition == "0": + return + if var.condition and var.condition != "1": + out.emit(f"if ({var.condition}) ") + out.emit( + f"stack_pointer[{base_offset.to_c()}]{bits} = {cast}{var.name};\n" + ) + @staticmethod def _do_flush(out: CWriter, variables: list[Local], base_offset: StackOffset, top_offset: StackOffset, cast_type: str = "uintptr_t", extract_bits: bool = False) -> None: out.start_line() for var in variables: if var.cached and not var.in_memory and not var.item.peek and not var.name in UNUSED: - cast = f"({cast_type})" if var.item.type else "" - bits = ".bits" if cast and not extract_bits else "" - if var.condition == "0": - continue - if var.condition and var.condition != "1": - out.emit(f"if ({var.condition}) ") - out.emit( - f"stack_pointer[{base_offset.to_c()}]{bits} = {cast}{var.name};\n" - ) + Stack._do_emit(out, var, base_offset, cast_type, extract_bits) base_offset.push(var.item) if base_offset.to_c() != top_offset.to_c(): print("base", base_offset, "top", top_offset) @@ -290,6 +302,22 @@ def flush(self, out: CWriter, cast_type: str = "uintptr_t", extract_bits: bool = self.base_offset.clear() self.top_offset.clear() + def flush_single_var(self, out: CWriter, var_name: str, cast_type: str = "uintptr_t", extract_bits: bool = False) -> None: + assert any(var.name == var_name for var in self.outputs) + base_offset = self.base_offset.copy() + top_offset = self.top_offset.copy() + for var in self.variables: + base_offset.push(var.item) + for var in self.outputs: + if var.name == var_name: + var.cached = False + Stack._do_emit(out, var, base_offset, cast_type, extract_bits) + base_offset.push(var.item) + top_offset.push(var.item) + if base_offset.to_c() != top_offset.to_c(): + print("base", base_offset, "top", top_offset) + assert False + def peek_offset(self) -> str: return self.top_offset.to_c() diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py index 6c13d1f10b39f9..720a4dcd81181c 100644 --- a/Tools/cases_generator/tier1_generator.py +++ b/Tools/cases_generator/tier1_generator.py @@ -92,7 +92,17 @@ def write_uop( stack.push(peeks.pop()) if braces: emitter.emit("{\n") - emitter.out.emit(stack.define_output_arrays(uop.stack.outputs)) + outputs: list[Local] = [] + for var in uop.stack.outputs: + if not var.peek: + if var.name in locals: + local = locals[var.name] + elif var.name == "unused": + local = Local.unused(var) + else: + local = Local.local(var) + outputs.append(local) + emitter.emit(stack.define_outputs(outputs)) for cache in uop.caches: if cache.name != "unused": @@ -109,15 +119,7 @@ def write_uop( emitter.emit(f"(void){cache.name};\n") offset += cache.size emitter.emit_tokens(uop, stack, inst) - for i, var in enumerate(uop.stack.outputs): - if not var.peek: - if var.name in locals: - local = locals[var.name] - elif var.name == "unused": - local = Local.unused(var) - else: - local = Local.local(var) - emitter.emit(stack.push(local)) + emitter.emit(stack.push_outputs()) if braces: emitter.out.start_line() emitter.emit("}\n") diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 8c212f75878984..b575a8f356975d 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -165,7 +165,14 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> None: emitter.emit(code) if local.defined: locals[local.name] = local - emitter.emit(stack.define_output_arrays(uop.stack.outputs)) + outputs: list[Local] = [] + for var in uop.stack.outputs: + if var.name in locals: + local = locals[var.name] + else: + local = Local.local(var) + outputs.append(local) + emitter.emit(stack.define_outputs(outputs)) for cache in uop.caches: if cache.name != "unused": if cache.size == 4: @@ -175,12 +182,7 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> None: cast = f"uint{cache.size*16}_t" emitter.emit(f"{type}{cache.name} = ({cast})CURRENT_OPERAND();\n") emitter.emit_tokens(uop, stack, None) - for i, var in enumerate(uop.stack.outputs): - if var.name in locals: - local = locals[var.name] - else: - local = Local.local(var) - emitter.emit(stack.push(local)) + emitter.emit(stack.push_outputs()) except StackError as ex: raise analysis_error(ex.args[0], uop.body[0]) from None From a2e60e4426d973b2dfc4f07253b0866024b0944f Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 6 Aug 2024 18:30:40 +0000 Subject: [PATCH 2/5] Fix analysis error message --- Tools/cases_generator/analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index a3b3c23ca5f089..23a598685bd6a6 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -388,7 +388,7 @@ def find_assignment_target(idx) -> list[lexer.Token]: name = lhs[0].text if not any(var.name == name for var in node.outputs): - raise analysis_error(f"PyStackRef_FromPyObjectNew() must be assigned to an output, not '{text}'", tkn) + raise analysis_error(f"PyStackRef_FromPyObjectNew() must be assigned to an output, not '{name}'", tkn) refs[tkn] = name From b8c83c2983b38459f541f097da8fa481b60cdb36 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 6 Aug 2024 18:32:50 +0000 Subject: [PATCH 3/5] Add missing type annotation --- Tools/cases_generator/analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 23a598685bd6a6..d33abc9743a9f5 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -356,7 +356,7 @@ def analyze_caches(inputs: list[parser.InputEffect]) -> list[CacheEntry]: def analyze_deferred_refs(node: parser.InstDef) -> dict[lexer.Token, str | None]: """Look for PyStackRef_FromPyObjectNew() calls""" - def find_assignment_target(idx) -> list[lexer.Token]: + def find_assignment_target(idx: int) -> list[lexer.Token]: """Find the tokens that make up the left-hand side of an assignment""" offset = 1 for tkn in reversed(node.block.tokens[:idx-1]): From 479ca70044d7c462d02b4c36a4e2a30719ec6b9b Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 7 Aug 2024 14:54:59 +0000 Subject: [PATCH 4/5] Remove 'outputs' from Stack --- Tools/cases_generator/analyzer.py | 2 +- Tools/cases_generator/generators_common.py | 2 +- Tools/cases_generator/optimizer_generator.py | 16 ++++---- Tools/cases_generator/stack.py | 41 +++++++++----------- Tools/cases_generator/tier1_generator.py | 8 +++- Tools/cases_generator/tier2_generator.py | 8 +++- 6 files changed, 40 insertions(+), 37 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index d33abc9743a9f5..3dc9838d75fa0c 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -370,7 +370,7 @@ def find_assignment_target(idx: int) -> list[lexer.Token]: if tkn.kind != "IDENTIFIER" or tkn.text != "PyStackRef_FromPyObjectNew": continue - if node.block.tokens[idx-1].kind != "EQUALS": + if idx == 0 or node.block.tokens[idx-1].kind != "EQUALS": raise analysis_error("Expected '=' before PyStackRef_FromPyObjectNew", tkn) lhs = find_assignment_target(idx) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 4e2be53cd9d90a..37060e2d7e4f50 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -225,7 +225,7 @@ def py_stack_ref_from_py_object_new( # Flush the assignment to the stack. Note that we don't flush the # stack pointer here, and instead are currently relying on initializing # unused portions of the stack to NULL. - stack.flush_single_var(self.out, target) + stack.flush_single_var(self.out, target, uop.stack.outputs) def emit_tokens( diff --git a/Tools/cases_generator/optimizer_generator.py b/Tools/cases_generator/optimizer_generator.py index 912280037b82e0..e192b76b23319c 100644 --- a/Tools/cases_generator/optimizer_generator.py +++ b/Tools/cases_generator/optimizer_generator.py @@ -112,14 +112,7 @@ def write_uop( out.emit(code) if local.defined: locals[local.name] = local - outputs: list[Local] = [] - for var in prototype.stack.outputs: - if var.name in locals: - local = locals[var.name] - else: - local = Local.local(var) - outputs.append(local) - out.emit(stack.define_outputs(outputs)) + out.emit(stack.define_output_arrays(prototype.stack.outputs)) if debug: args = [] for var in prototype.stack.inputs: @@ -141,7 +134,12 @@ def write_uop( else: emit_default(out, uop) - out.emit(stack.push_outputs()) + for var in prototype.stack.outputs: + if var.name in locals: + local = locals[var.name] + else: + local = Local.local(var) + out.emit(stack.push(local)) out.start_line() stack.flush(out, cast_type="_Py_UopsSymbol *", extract_bits=True) except StackError as ex: diff --git a/Tools/cases_generator/stack.py b/Tools/cases_generator/stack.py index 01216a16d031c8..35639a40a3a658 100644 --- a/Tools/cases_generator/stack.py +++ b/Tools/cases_generator/stack.py @@ -169,7 +169,6 @@ def __init__(self) -> None: self.top_offset = StackOffset.empty() self.base_offset = StackOffset.empty() self.variables: list[Local] = [] - self.outputs: list[Local] = [] self.defined: set[str] = set() def pop(self, var: StackItem, extract_bits: bool = False) -> tuple[str, Local]: @@ -245,28 +244,22 @@ def push(self, var: Local) -> str: self.defined.add(var.name) return "" - def define_outputs(self, outputs: list[Local]) -> str: + def define_output_arrays(self, outputs: list[StackItem]) -> str: res = [] top_offset = self.top_offset.copy() for var in outputs: - if var.is_array() and var.item.used and not var.item.peek: + if var.is_array() and var.used and not var.peek: c_offset = top_offset.to_c() + top_offset.push(var) res.append(f"{var.name} = &stack_pointer[{c_offset}];\n") - self.outputs.append(var) - top_offset.push(var.item) + else: + top_offset.push(var) return "\n".join(res) - def push_outputs(self) -> str: - res = [] - for var in self.outputs: - res.append(self.push(var)) - self.outputs = [] - return "".join(res) - @staticmethod - def _do_emit(out: CWriter, var: Local, base_offset: StackOffset, + def _do_emit(out: CWriter, var: StackItem, base_offset: StackOffset, cast_type: str = "uintptr_t", extract_bits: bool = False) -> None: - cast = f"({cast_type})" if var.item.type else "" + cast = f"({cast_type})" if var.type else "" bits = ".bits" if cast and not extract_bits else "" if var.condition == "0": return @@ -282,7 +275,7 @@ def _do_flush(out: CWriter, variables: list[Local], base_offset: StackOffset, to out.start_line() for var in variables: if var.cached and not var.in_memory and not var.item.peek and not var.name in UNUSED: - Stack._do_emit(out, var, base_offset, cast_type, extract_bits) + Stack._do_emit(out, var.item, base_offset, cast_type, extract_bits) base_offset.push(var.item) if base_offset.to_c() != top_offset.to_c(): print("base", base_offset, "top", top_offset) @@ -302,18 +295,22 @@ def flush(self, out: CWriter, cast_type: str = "uintptr_t", extract_bits: bool = self.base_offset.clear() self.top_offset.clear() - def flush_single_var(self, out: CWriter, var_name: str, cast_type: str = "uintptr_t", extract_bits: bool = False) -> None: - assert any(var.name == var_name for var in self.outputs) + def flush_single_var(self, out: CWriter, var_name: str, outputs: list[StackItem], + cast_type: str = "uintptr_t", extract_bits: bool = False) -> None: + assert any(var.name == var_name for var in outputs) base_offset = self.base_offset.copy() top_offset = self.top_offset.copy() for var in self.variables: - base_offset.push(var.item) - for var in self.outputs: + base_offset.push(var) + for var in outputs: + if any(var == v.item for v in self.variables): + # The variable is already on the stack, such as a peeked value + # in the tier1 generator + continue if var.name == var_name: - var.cached = False Stack._do_emit(out, var, base_offset, cast_type, extract_bits) - base_offset.push(var.item) - top_offset.push(var.item) + base_offset.push(var) + top_offset.push(var) if base_offset.to_c() != top_offset.to_c(): print("base", base_offset, "top", top_offset) assert False diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py index 720a4dcd81181c..9ffe7745b88c54 100644 --- a/Tools/cases_generator/tier1_generator.py +++ b/Tools/cases_generator/tier1_generator.py @@ -92,6 +92,7 @@ def write_uop( stack.push(peeks.pop()) if braces: emitter.emit("{\n") + emitter.out.emit(stack.define_output_arrays(uop.stack.outputs)) outputs: list[Local] = [] for var in uop.stack.outputs: if not var.peek: @@ -102,7 +103,6 @@ def write_uop( else: local = Local.local(var) outputs.append(local) - emitter.emit(stack.define_outputs(outputs)) for cache in uop.caches: if cache.name != "unused": @@ -119,7 +119,11 @@ def write_uop( emitter.emit(f"(void){cache.name};\n") offset += cache.size emitter.emit_tokens(uop, stack, inst) - emitter.emit(stack.push_outputs()) + for var in outputs: + if var.name in uop.deferred_refs.values(): + # We've already spilled this when emitting tokens + var.cached = False + emitter.emit(stack.push(var)) if braces: emitter.out.start_line() emitter.emit("}\n") diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index b575a8f356975d..6e3b6c9c226b21 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -165,6 +165,7 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> None: emitter.emit(code) if local.defined: locals[local.name] = local + emitter.emit(stack.define_output_arrays(uop.stack.outputs)) outputs: list[Local] = [] for var in uop.stack.outputs: if var.name in locals: @@ -172,7 +173,6 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> None: else: local = Local.local(var) outputs.append(local) - emitter.emit(stack.define_outputs(outputs)) for cache in uop.caches: if cache.name != "unused": if cache.size == 4: @@ -182,7 +182,11 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> None: cast = f"uint{cache.size*16}_t" emitter.emit(f"{type}{cache.name} = ({cast})CURRENT_OPERAND();\n") emitter.emit_tokens(uop, stack, None) - emitter.emit(stack.push_outputs()) + for var in outputs: + if var.name in uop.deferred_refs.values(): + # We've already spilled this when emitting tokens + var.cached = False + emitter.emit(stack.push(var)) except StackError as ex: raise analysis_error(ex.args[0], uop.body[0]) from None From ae1c018e7caa8196c9733c2dcaf9f73528309a3e Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 7 Aug 2024 15:16:58 +0000 Subject: [PATCH 5/5] Fix type checking errors --- Tools/cases_generator/stack.py | 2 +- Tools/cases_generator/tier1_generator.py | 8 ++++---- Tools/cases_generator/tier2_generator.py | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Tools/cases_generator/stack.py b/Tools/cases_generator/stack.py index 35639a40a3a658..b44e48af09b3f0 100644 --- a/Tools/cases_generator/stack.py +++ b/Tools/cases_generator/stack.py @@ -301,7 +301,7 @@ def flush_single_var(self, out: CWriter, var_name: str, outputs: list[StackItem] base_offset = self.base_offset.copy() top_offset = self.top_offset.copy() for var in self.variables: - base_offset.push(var) + base_offset.push(var.item) for var in outputs: if any(var == v.item for v in self.variables): # The variable is already on the stack, such as a peeked value diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py index 9ffe7745b88c54..1ea31a041ce3ae 100644 --- a/Tools/cases_generator/tier1_generator.py +++ b/Tools/cases_generator/tier1_generator.py @@ -119,11 +119,11 @@ def write_uop( emitter.emit(f"(void){cache.name};\n") offset += cache.size emitter.emit_tokens(uop, stack, inst) - for var in outputs: - if var.name in uop.deferred_refs.values(): + for output in outputs: + if output.name in uop.deferred_refs.values(): # We've already spilled this when emitting tokens - var.cached = False - emitter.emit(stack.push(var)) + output.cached = False + emitter.emit(stack.push(output)) if braces: emitter.out.start_line() emitter.emit("}\n") diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 6e3b6c9c226b21..461375c71fae83 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -182,11 +182,11 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> None: cast = f"uint{cache.size*16}_t" emitter.emit(f"{type}{cache.name} = ({cast})CURRENT_OPERAND();\n") emitter.emit_tokens(uop, stack, None) - for var in outputs: - if var.name in uop.deferred_refs.values(): + for output in outputs: + if output.name in uop.deferred_refs.values(): # We've already spilled this when emitting tokens - var.cached = False - emitter.emit(stack.push(var)) + output.cached = False + emitter.emit(stack.push(output)) except StackError as ex: raise analysis_error(ex.args[0], uop.body[0]) from None