From bf8686e1ea389427157c754c5e522ee040f166ad Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 9 Jul 2024 11:33:56 +0100 Subject: [PATCH] GH-118926: Better distinguish between pointer and arrays in interpreter generator (GH-121496) --- Lib/test/test_generated_cases.py | 44 +++++++++++++++++++++- Tools/cases_generator/analyzer.py | 8 ++-- Tools/cases_generator/generators_common.py | 14 ++++++- Tools/cases_generator/parsing.py | 1 - Tools/cases_generator/stack.py | 11 +++--- Tools/cases_generator/tier1_generator.py | 11 +++--- Tools/cases_generator/tier2_generator.py | 3 +- 7 files changed, 73 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index 30e39e7720e6d1..00def509a219c3 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -815,7 +815,6 @@ def test_annotated_op(self): """ self.run_cases_test(input, output) - def test_deopt_and_exit(self): input = """ pure op(OP, (arg1 -- out)) { @@ -827,6 +826,49 @@ def test_deopt_and_exit(self): with self.assertRaises(Exception): self.run_cases_test(input, output) + def test_array_of_one(self): + input = """ + inst(OP, (arg[1] -- out[1])) { + out[0] = arg[0]; + } + """ + output = """ + TARGET(OP) { + frame->instr_ptr = next_instr; + next_instr += 1; + INSTRUCTION_STATS(OP); + _PyStackRef *arg; + _PyStackRef *out; + arg = &stack_pointer[-1]; + out = &stack_pointer[-1]; + out[0] = arg[0]; + DISPATCH(); + } + """ + self.run_cases_test(input, output) + + def test_pointer_to_stackref(self): + input = """ + inst(OP, (arg: _PyStackRef * -- out)) { + out = *arg; + } + """ + output = """ + TARGET(OP) { + frame->instr_ptr = next_instr; + next_instr += 1; + INSTRUCTION_STATS(OP); + _PyStackRef *arg; + _PyStackRef out; + arg = (_PyStackRef *)stack_pointer[-1].bits; + out = *arg; + stack_pointer[-1] = out; + DISPATCH(); + } + """ + self.run_cases_test(input, output) + + class TestGeneratedAbstractCases(unittest.TestCase): def setUp(self) -> None: super().setUp() diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index f92560bd2b76b3..ec365bad3992d5 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -106,13 +106,15 @@ class StackItem: def __str__(self) -> str: cond = f" if ({self.condition})" if self.condition else "" - size = f"[{self.size}]" if self.size != "1" else "" + size = f"[{self.size}]" if self.size else "" type = "" if self.type is None else f"{self.type} " return f"{type}{self.name}{size}{cond} {self.peek}" def is_array(self) -> bool: - return self.type == "_PyStackRef *" + return self.size != "" + def get_size(self) -> str: + return self.size if self.size else "1" @dataclass class StackEffect: @@ -293,7 +295,7 @@ def convert_stack_item(item: parser.StackEffect, replace_op_arg_1: str | None) - if replace_op_arg_1 and OPARG_AND_1.match(item.cond): cond = replace_op_arg_1 return StackItem( - item.name, item.type, cond, (item.size or "1") + item.name, item.type, cond, item.size ) def analyze_stack(op: parser.InstDef | parser.Pseudo, replace_op_arg_1: str | None = None) -> StackEffect: diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index e4e0c9b658c19d..9314bb9e79687f 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -5,9 +5,10 @@ Instruction, Uop, Properties, + StackItem, ) from cwriter import CWriter -from typing import Callable, Mapping, TextIO, Iterator +from typing import Callable, Mapping, TextIO, Iterator, Tuple from lexer import Token from stack import Stack @@ -24,6 +25,15 @@ def root_relative_path(filename: str) -> str: return filename +def type_and_null(var: StackItem) -> Tuple[str, str]: + if var.type: + return var.type, "NULL" + elif var.is_array(): + return "_PyStackRef *", "NULL" + else: + return "_PyStackRef", "PyStackRef_NULL" + + def write_header( generator: str, sources: list[str], outfile: TextIO, comment: str = "//" ) -> None: @@ -126,7 +136,7 @@ def replace_decrefs( for var in uop.stack.inputs: if var.name == "unused" or var.name == "null" or var.peek: continue - if var.size != "1": + if var.size: out.emit(f"for (int _i = {var.size}; --_i >= 0;) {{\n") out.emit(f"PyStackRef_CLOSE({var.name}[_i]);\n") out.emit("}\n") diff --git a/Tools/cases_generator/parsing.py b/Tools/cases_generator/parsing.py index 0bd4229e2beaf2..8957838f7a90a1 100644 --- a/Tools/cases_generator/parsing.py +++ b/Tools/cases_generator/parsing.py @@ -285,7 +285,6 @@ def stack_effect(self) -> StackEffect | None: if not (size := self.expression()): raise self.make_syntax_error("Expected expression") self.require(lx.RBRACKET) - type_text = "_PyStackRef *" size_text = size.text.strip() return StackEffect(tkn.text, type_text, cond_text, size_text) return None diff --git a/Tools/cases_generator/stack.py b/Tools/cases_generator/stack.py index c0e1278e519143..ebe62df537f15f 100644 --- a/Tools/cases_generator/stack.py +++ b/Tools/cases_generator/stack.py @@ -28,14 +28,15 @@ def var_size(var: StackItem) -> str: if var.condition == "0": return "0" elif var.condition == "1": - return var.size - elif var.condition == "oparg & 1" and var.size == "1": + return var.get_size() + elif var.condition == "oparg & 1" and not var.size: return f"({var.condition})" else: - return f"(({var.condition}) ? {var.size} : 0)" - else: + return f"(({var.condition}) ? {var.get_size()} : 0)" + elif var.size: return var.size - + else: + return "1" @dataclass class StackOffset: diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py index c9dce1d5f1804e..85be673b1c396c 100644 --- a/Tools/cases_generator/tier1_generator.py +++ b/Tools/cases_generator/tier1_generator.py @@ -13,12 +13,14 @@ analyze_files, Skip, analysis_error, + StackItem, ) from generators_common import ( DEFAULT_INPUT, ROOT, write_header, emit_tokens, + type_and_null, ) from cwriter import CWriter from typing import TextIO @@ -38,19 +40,16 @@ def declare_variables(inst: Instruction, out: CWriter) -> None: for var in reversed(uop.stack.inputs): if var.name not in variables: variables.add(var.name) - type, null = (var.type, "NULL") if var.type else ("_PyStackRef", "PyStackRef_NULL") + 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: - if var.is_array(): - out.emit(f"{var.type}{space}{var.name};\n") - else: - out.emit(f"{type}{space}{var.name};\n") + 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 = (var.type, "NULL") if var.type else ("_PyStackRef", "PyStackRef_NULL") + type, null = type_and_null(var) space = " " if type[-1].isalnum() else "" if var.condition: out.emit(f"{type}{space}{var.name} = {null};\n") diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index f3769bd31c295d..7a69aa6e121fa7 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -20,6 +20,7 @@ emit_tokens, emit_to, REPLACEMENT_FUNCTIONS, + type_and_null, ) from cwriter import CWriter from typing import TextIO, Iterator @@ -35,7 +36,7 @@ def declare_variable( if var.name in variables: return variables.add(var.name) - type, null = (var.type, "NULL") if var.type else ("_PyStackRef", "PyStackRef_NULL") + type, null = type_and_null(var) space = " " if type[-1].isalnum() else "" if var.condition: out.emit(f"{type}{space}{var.name} = {null};\n")