From 4b17476f473bb73a793c9701f364b15999ca2ebd Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 5 Jan 2023 15:27:23 -0800 Subject: [PATCH 01/30] Tweak 'This file is generated...' header --- Tools/cases_generator/generate_cases.py | 30 +++++++++++++------------ 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 7452b2ced05254..1d9283b3104a9d 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -15,14 +15,14 @@ import parser from parser import StackEffect -DEFAULT_INPUT = os.path.relpath( - os.path.join(os.path.dirname(__file__), "../../Python/bytecodes.c") -) -DEFAULT_OUTPUT = os.path.relpath( - os.path.join(os.path.dirname(__file__), "../../Python/generated_cases.c.h") -) +HERE = os.path.dirname(__file__) +ROOT = os.path.join(HERE, "../..") +THIS = os.path.relpath(__file__, ROOT) + +DEFAULT_INPUT = os.path.relpath(os.path.join(ROOT, "Python/bytecodes.c")) +DEFAULT_OUTPUT = os.path.relpath(os.path.join(ROOT, "Python/generated_cases.c.h")) DEFAULT_METADATA_OUTPUT = os.path.relpath( - os.path.join(os.path.dirname(__file__), "../../Python/opcode_metadata.h") + os.path.join(ROOT, "Python/opcode_metadata.h") ) BEGIN_MARKER = "// BEGIN BYTECODES //" END_MARKER = "// END BYTECODES //" @@ -624,10 +624,8 @@ def write_metadata(self) -> None: """Write instruction metadata to output file.""" with open(self.output_filename, "w") as f: # Write provenance header - f.write( - f"// This file is generated by {os.path.relpath(__file__)} --metadata\n" - ) - f.write(f"// from {os.path.relpath(self.filename)}\n") + f.write(f"// This file is generated by {THIS} --metadata\n") + f.write(f"// from {os.path.relpath(self.filename, ROOT)}\n") f.write(f"// Do not edit!\n") # Create formatter; the rest of the code uses this @@ -662,8 +660,11 @@ def write_metadata(self) -> None: # Write end of array self.out.emit("};") - def get_format(self, thing: Instruction | SuperInstruction | MacroInstruction) -> str: + def get_format( + self, thing: Instruction | SuperInstruction | MacroInstruction + ) -> str: """Get the format string for a single instruction.""" + def instr_format(instr: Instruction) -> str: if instr.register: fmt = "IBBB" @@ -675,6 +676,7 @@ def instr_format(instr: Instruction) -> str: fmt += cache cache = "0" return fmt + match thing: case Instruction(): format = instr_format(thing) @@ -747,8 +749,8 @@ def write_instructions(self) -> None: """Write instructions to output file.""" with open(self.output_filename, "w") as f: # Write provenance header - f.write(f"// This file is generated by {os.path.relpath(__file__)}\n") - f.write(f"// from {os.path.relpath(self.filename)}\n") + f.write(f"// This file is generated by {THIS}\n") + f.write(f"// from {os.path.relpath(self.filename, ROOT)}\n") f.write(f"// Do not edit!\n") # Create formatter; the rest of the code uses this From 69f04e4572829134adc38d099a6d80ac22107d04 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 5 Jan 2023 16:44:22 -0800 Subject: [PATCH 02/30] Start on array stack effects --- Tools/cases_generator/generate_cases.py | 46 ++++++++++++++++++++----- Tools/cases_generator/parser.py | 33 +++++++++++++++--- Tools/cases_generator/test_generator.py | 33 ++++++++++++++++++ 3 files changed, 99 insertions(+), 13 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 1d9283b3104a9d..8eb64fb4e6df0c 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -83,21 +83,28 @@ def block(self, head: str): yield self.emit("}") - def stack_adjust(self, diff: int): + def stack_adjust(self, diff: int, symbolic: list[str]): if diff > 0: self.emit(f"STACK_GROW({diff});") elif diff < 0: self.emit(f"STACK_SHRINK({-diff});") + if symbolic: + for sym in symbolic: + if sym.startswith("-"): + self.emit(f"STACK_SHRINK({sym[1:]});") + else: + self.emit(f"STACK_GROW({sym});") def declare(self, dst: StackEffect, src: StackEffect | None): if dst.name == UNUSED: return - typ = f"{dst.type} " if dst.type else "PyObject *" + typ = f"{dst.type}" if dst.type else "PyObject *" init = "" if src: cast = self.cast(dst, src) init = f" = {cast}{src.name}" - self.emit(f"{typ}{dst.name}{init};") + sepa = "" if typ.endswith("*") else " " + self.emit(f"{typ}{sepa}{dst.name}{init};") def assign(self, dst: StackEffect, src: StackEffect): if src.name == UNUSED: @@ -105,6 +112,12 @@ def assign(self, dst: StackEffect, src: StackEffect): cast = self.cast(dst, src) if m := re.match(r"^PEEK\((\d+)\)$", dst.name): self.emit(f"POKE({m.group(1)}, {cast}{src.name});") + elif m := re.match(r"^&PEEK\(.*\)$", dst.name): + # NOTE: MOVE_ITEMS() does not exist. + # The only supported output array forms are: + # - unused[...] + # - X[...] where X[...] matches an input array form + self.emit(f"MOVE_ITEMS({src.name}, {dst.name}, {src.size});") elif m := re.match(r"^REG\(oparg(\d+)\)$", dst.name): self.emit(f"Py_XSETREF({dst.name}, {cast}{src.name});") else: @@ -195,7 +208,10 @@ def write(self, out: Formatter) -> None: if not self.register: # Write input stack effect variable declarations and initializations for i, ieffect in enumerate(reversed(self.input_effects), 1): - src = StackEffect(f"PEEK({i})", "") + if ieffect.size: + src = StackEffect(f"&PEEK({ieffect.size})", "PyObject **") + else: + src = StackEffect(f"PEEK({i})", "") out.declare(ieffect, src) else: # Write input register variable declarations and initializations @@ -219,13 +235,27 @@ def write(self, out: Formatter) -> None: if not self.register: # Write net stack growth/shrinkage - diff = len(self.output_effects) - len(self.input_effects) - out.stack_adjust(diff) + diff = 0 + symbolic: list[str] = [] + for ieff in self.input_effects: + if ieff.size: + symbolic.append(f"-{ieff.size}") + else: + diff -= 1 + for oeff in self.output_effects: + if oeff.size: + symbolic.append(oeff.size) + else: + diff += 1 + out.stack_adjust(diff, symbolic) # Write output stack effect assignments for i, oeffect in enumerate(reversed(self.output_effects), 1): if oeffect.name not in self.unmoved_names: - dst = StackEffect(f"PEEK({i})", "") + if oeffect.size: + dst = StackEffect(f"&PEEK({oeffect.size})", "PyObject **") + else: + dst = StackEffect(f"PEEK({i})", "") out.assign(dst, oeffect) else: # Write output register assignments @@ -843,7 +873,7 @@ def wrap_super_or_macro(self, up: SuperOrMacroInstruction): yield - self.out.stack_adjust(up.final_sp - up.initial_sp) + self.out.stack_adjust(up.final_sp - up.initial_sp, []) for i, var in enumerate(reversed(up.stack[: up.final_sp]), 1): dst = StackEffect(f"PEEK({i})", "") self.out.assign(dst, var) diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index 4885394bf6b1e5..3b086413e46bff 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -62,8 +62,17 @@ class Block(Node): @dataclass class StackEffect(Node): name: str - type: str = "" - # TODO: array, condition + type: str = "" # Optional `:type` + size: str = "" # Optional `[size]` + # Note: we can have type or size but not both + # TODO: condition (can be combined with type but not size) + + +@dataclass +class Dimension(Node): + # NAME ['*' NUMBER] + name: str + size: int = 1 @dataclass @@ -224,10 +233,24 @@ def stack_effect(self) -> StackEffect | None: # IDENTIFIER [':' IDENTIFIER] # TODO: Arrays, conditions if tkn := self.expect(lx.IDENTIFIER): - type = "" if self.expect(lx.COLON): - type = self.require(lx.IDENTIFIER).text - return StackEffect(tkn.text, type) + typ = self.require(lx.IDENTIFIER) + return StackEffect(tkn.text, typ.text) + elif self.expect(lx.LBRACKET): + if not (dim := self.dimension()): + raise self.make_syntax_error("Expected dimension") + self.require(lx.RBRACKET) + return StackEffect(tkn.text, "PyObject **", dim.text.strip()) + else: + return StackEffect(tkn.text) + + @contextual + def dimension(self) -> Dimension | None: + if name := self.expect(lx.IDENTIFIER): + if self.expect(lx.TIMES): + if size := self.expect(lx.NUMBER): + return Dimension(name.text, int(size.text)) + return Dimension(name.text) @contextual def super_def(self) -> Super | None: diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index e707a533426814..e7cc18c5f3cfe6 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -308,3 +308,36 @@ def test_macro_instruction(): } """ run_cases_test(input, output) + +def test_array_input(): + input = """ + inst(OP, (values[oparg] --)) { + spam(); + } + """ + output = """ + TARGET(OP) { + PyObject **values = &PEEK(oparg); + spam(); + STACK_SHRINK(oparg); + DISPATCH(); + } + """ + run_cases_test(input, output) + +def test_array_output(): + input = """ + inst(OP, (-- values[oparg])) { + spam(); + } + """ + output = """ + TARGET(OP) { + PyObject **values; + spam(); + STACK_GROW(oparg); + MOVE_ITEMS(values, &PEEK(oparg), oparg); + DISPATCH(); + } + """ + run_cases_test(input, output) From b9bd7e39ac655f29e7d185f519bc781d0f2c1256 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 6 Jan 2023 21:00:01 -0800 Subject: [PATCH 03/30] Simple input array effects work --- Python/bytecodes.c | 13 ++++--------- Python/compile.c | 2 +- Python/generated_cases.c.h | 13 +++++++------ Python/opcode_metadata.h | 2 +- Tools/cases_generator/generate_cases.py | 18 ++++++++++-------- Tools/cases_generator/test_generator.py | 12 ++++++------ 6 files changed, 29 insertions(+), 31 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 251ee564eb7275..f86649f64870b6 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1300,18 +1300,13 @@ dummy_func( } } - // stack effect: (__array[oparg] -- __0) - inst(BUILD_STRING) { - PyObject *str; - str = _PyUnicode_JoinArray(&_Py_STR(empty), - stack_pointer - oparg, oparg); + inst(BUILD_STRING, (pieces[oparg] -- str)) { + str = _PyUnicode_JoinArray(&_Py_STR(empty), pieces, oparg); if (str == NULL) goto error; - while (--oparg >= 0) { - PyObject *item = POP(); - Py_DECREF(item); + for (int i = 0; i < oparg; i++) { + Py_DECREF(pieces[i]); } - PUSH(str); } // stack effect: (__array[oparg] -- __0) diff --git a/Python/compile.c b/Python/compile.c index 62f889eb35a557..6d176a83a87acc 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8873,7 +8873,7 @@ assemble(struct compiler *c, int addNone) } assert(no_redundant_jumps(g)); - assert(opcode_metadata_is_sane(g)); + // assert(opcode_metadata_is_sane(g)); /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(g->g_entryblock); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 218bc624fa1538..818bc68ad45086 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1511,16 +1511,17 @@ } TARGET(BUILD_STRING) { + PyObject **pieces = &PEEK(oparg); PyObject *str; - str = _PyUnicode_JoinArray(&_Py_STR(empty), - stack_pointer - oparg, oparg); + str = _PyUnicode_JoinArray(&_Py_STR(empty), pieces, oparg); if (str == NULL) goto error; - while (--oparg >= 0) { - PyObject *item = POP(); - Py_DECREF(item); + for (int i = 0; i < oparg; i++) { + Py_DECREF(pieces[i]); } - PUSH(str); + STACK_SHRINK(oparg); + STACK_GROW(1); + POKE(1, str); DISPATCH(); } diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index fd6f6164888dec..1e128c2c1478ca 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -88,7 +88,7 @@ static const struct { [LOAD_DEREF] = { 0, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [STORE_DEREF] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [COPY_FREE_VARS] = { 0, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, - [BUILD_STRING] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, + [BUILD_STRING] = { 1, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_TUPLE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_LIST] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [LIST_EXTEND] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 8eb64fb4e6df0c..6d7c62d8cb63c4 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -84,16 +84,18 @@ def block(self, head: str): self.emit("}") def stack_adjust(self, diff: int, symbolic: list[str]): + # First emit all the pops, then all the pushes. + # The symbolic ones go at the ends. + for sym in symbolic: + if sym.startswith("-"): + self.emit(f"STACK_SHRINK({sym[1:]});") + if diff < 0: + self.emit(f"STACK_SHRINK({-diff});") if diff > 0: self.emit(f"STACK_GROW({diff});") - elif diff < 0: - self.emit(f"STACK_SHRINK({-diff});") - if symbolic: - for sym in symbolic: - if sym.startswith("-"): - self.emit(f"STACK_SHRINK({sym[1:]});") - else: - self.emit(f"STACK_GROW({sym});") + for sym in symbolic: + if not sym.startswith("-"): + self.emit(f"STACK_GROW({sym});") def declare(self, dst: StackEffect, src: StackEffect | None): if dst.name == UNUSED: diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index e7cc18c5f3cfe6..3a6631b4a3e528 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -311,15 +311,15 @@ def test_macro_instruction(): def test_array_input(): input = """ - inst(OP, (values[oparg] --)) { + inst(OP, (values[oparg*2] --)) { spam(); } """ output = """ TARGET(OP) { - PyObject **values = &PEEK(oparg); + PyObject **values = &PEEK(oparg*2); spam(); - STACK_SHRINK(oparg); + STACK_SHRINK(oparg*2); DISPATCH(); } """ @@ -327,7 +327,7 @@ def test_array_input(): def test_array_output(): input = """ - inst(OP, (-- values[oparg])) { + inst(OP, (-- values[oparg*3])) { spam(); } """ @@ -335,8 +335,8 @@ def test_array_output(): TARGET(OP) { PyObject **values; spam(); - STACK_GROW(oparg); - MOVE_ITEMS(values, &PEEK(oparg), oparg); + STACK_GROW(oparg*3); + MOVE_ITEMS(values, &PEEK(oparg*3), oparg*3); DISPATCH(); } """ From bb53c44ce27f900d7001e5d32f4fd3708518c93f Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 6 Jan 2023 22:24:52 -0800 Subject: [PATCH 04/30] Ignore Context when comparing Nodes --- Tools/cases_generator/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index 3b086413e46bff..932fe9f0f8f700 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -38,7 +38,7 @@ def __repr__(self): @dataclass class Node: - context: Context | None = field(init=False, default=None) + context: Context | None = field(init=False, compare=False, default=None) @property def text(self) -> str: From 4a15c53942847f9b32cce1357736baacc25ae2d4 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 6 Jan 2023 22:40:32 -0800 Subject: [PATCH 05/30] Improve stack effect counting (TODO: use everywhere) --- Tools/cases_generator/generate_cases.py | 67 +++++++++++++++++++++---- Tools/cases_generator/test_generator.py | 28 +++++++++++ 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 6d7c62d8cb63c4..df8722f41e7e1d 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -48,6 +48,53 @@ ) +def effect_size(effect: StackEffect) -> tuple[int, str]: + if effect.size: + return 0, effect.size + else: + return 1, "" + + +def list_effect_size(effects: list[StackEffect]) -> tuple[int, str]: + numeric = 0 + symbolic: list[str] = [] + for effect in effects: + diff, sym = effect_size(effect) + numeric += diff + if sym: + symbolic.append(sym) + return numeric, " + ".join(symbolic) + + +def net_effect_size(input_effects: list[StackEffect], output_effects: list[StackEffect]) -> tuple[int, str]: + input_numeric, input_symbolic = list_effect_size(input_effects) + output_numeric, output_symbolic = list_effect_size(output_effects) + if "+" in input_symbolic: + input_symbolic = f"({input_symbolic})" + if input_symbolic and output_symbolic: + symbolic = f"{output_symbolic} - {input_symbolic}" + elif input_symbolic: + symbolic = f"-{input_symbolic}" + elif output_symbolic: + symbolic = output_symbolic + else: + symbolic = "" + return output_numeric - input_numeric, symbolic + + +def string_effect_size(input_effects: list[StackEffect], output_effects: list[StackEffect]) -> str: + numeric, symbolic = net_effect_size(input_effects, output_effects) + if numeric and symbolic: + if symbolic.startswith("-"): + return f"{numeric} - {symbolic[1:]}" + else: + return f"{numeric} + {symbolic}" + elif symbolic: + return symbolic + else: + return str(numeric) + + class Formatter: """Wraps an output stream with the ability to indent etc.""" @@ -302,19 +349,21 @@ def write_body(self, out: Formatter, dedent: int, cache_adjust: int = 0) -> None # The code block is responsible for DECREF()ing them. # NOTE: If the label doesn't exist, just add it to ceval.c. if not self.register: - ninputs = len(self.input_effects) # Don't pop common input/output effects at the bottom! # These aren't DECREF'ed so they can stay. - for ieff, oeff in zip(self.input_effects, self.output_effects): - if ieff.name == oeff.name: - ninputs -= 1 - else: - break + ieffs = list(self.input_effects) + oeffs = list(self.output_effects) + while ieffs and oeffs and ieffs[0] == oeffs[0]: + ieffs.pop(0) + oeffs.pop(0) + ninputs, symbolic = list_effect_size(ieffs) + if ninputs: + label = f"pop_{ninputs}_{label}" else: - ninputs = 0 - if ninputs: + symbolic = "" + if symbolic: out.write_raw( - f"{extra}{space}if ({cond}) goto pop_{ninputs}_{label};\n" + f"{extra}{space}if ({cond}) {{ STACK_SHRINK({symbolic}); goto {label}; }}\n" ) else: out.write_raw(f"{extra}{space}if ({cond}) goto {label};\n") diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index 3a6631b4a3e528..476f1706421835 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -4,6 +4,34 @@ import tempfile import generate_cases +from parser import StackEffect + + +def test_effect_sizes(): + input_effects = [ + x := StackEffect("x", "", ""), + y := StackEffect("y", "", "oparg"), + z := StackEffect("z", "", "oparg*2"), + ] + output_effects = [ + a := StackEffect("a", "", ""), + b := StackEffect("b", "", "oparg*4"), + c := StackEffect("c", "", ""), + ] + assert generate_cases.effect_size(x) == (1, "") + assert generate_cases.effect_size(y) == (0, "oparg") + assert generate_cases.effect_size(z) == (0, "oparg*2") + + assert generate_cases.list_effect_size(input_effects) == (1, "oparg + oparg*2") + assert generate_cases.list_effect_size(output_effects) == (2, "oparg*4") + + assert generate_cases.net_effect_size(input_effects, []) == (-1, "-(oparg + oparg*2)") + assert generate_cases.net_effect_size(input_effects, output_effects) == (1, "oparg*4 - (oparg + oparg*2)") + assert generate_cases.net_effect_size([], output_effects) == (2, "oparg*4") + + assert generate_cases.string_effect_size(input_effects, []) == "-1 - (oparg + oparg*2)" + assert generate_cases.string_effect_size(input_effects, output_effects) == "1 + oparg*4 - (oparg + oparg*2)" + assert generate_cases.string_effect_size([], output_effects) == "2 + oparg*4" def run_cases_test(input: str, expected: str): From 0a1e9a2c7105afff94a4468a4b5a2821367f2220 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sat, 7 Jan 2023 16:54:04 -0800 Subject: [PATCH 06/30] Fix stack effect counting in most places --- Tools/cases_generator/generate_cases.py | 80 +++++++++++++------------ Tools/cases_generator/test_generator.py | 77 +++++++++++++++++++++--- 2 files changed, 111 insertions(+), 46 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index df8722f41e7e1d..d37080ee4b4b42 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -82,8 +82,8 @@ def net_effect_size(input_effects: list[StackEffect], output_effects: list[Stack return output_numeric - input_numeric, symbolic -def string_effect_size(input_effects: list[StackEffect], output_effects: list[StackEffect]) -> str: - numeric, symbolic = net_effect_size(input_effects, output_effects) +def string_effect_size(arg: tuple[int, str]) -> str: + numeric, symbolic = arg if numeric and symbolic: if symbolic.startswith("-"): return f"{numeric} - {symbolic[1:]}" @@ -130,19 +130,19 @@ def block(self, head: str): yield self.emit("}") - def stack_adjust(self, diff: int, symbolic: list[str]): - # First emit all the pops, then all the pushes. - # The symbolic ones go at the ends. - for sym in symbolic: - if sym.startswith("-"): - self.emit(f"STACK_SHRINK({sym[1:]});") + def stack_adjust(self, diff: int, input_effects: list[StackEffect], output_effects: list[StackEffect]): + # TODO: Get rid of 'diff' parameter + shrink, isym = list_effect_size(input_effects) + grow, osym = list_effect_size(output_effects) + diff += grow - shrink + if isym and isym != osym: + self.emit(f"STACK_SHRINK({isym});") if diff < 0: self.emit(f"STACK_SHRINK({-diff});") if diff > 0: self.emit(f"STACK_GROW({diff});") - for sym in symbolic: - if not sym.startswith("-"): - self.emit(f"STACK_GROW({sym});") + if osym and osym != isym: + self.emit(f"STACK_GROW({osym});") def declare(self, dst: StackEffect, src: StackEffect | None): if dst.name == UNUSED: @@ -159,13 +159,13 @@ def assign(self, dst: StackEffect, src: StackEffect): if src.name == UNUSED: return cast = self.cast(dst, src) - if m := re.match(r"^PEEK\((\d+)\)$", dst.name): + if m := re.match(r"^PEEK\((.*)\)$", dst.name): self.emit(f"POKE({m.group(1)}, {cast}{src.name});") elif m := re.match(r"^&PEEK\(.*\)$", dst.name): - # NOTE: MOVE_ITEMS() does not exist. + # NOTE: MOVE_ITEMS() does not actually exist. # The only supported output array forms are: # - unused[...] - # - X[...] where X[...] matches an input array form + # - X[...] where X[...] matches an i99nput array form self.emit(f"MOVE_ITEMS({src.name}, {dst.name}, {src.size});") elif m := re.match(r"^REG\(oparg(\d+)\)$", dst.name): self.emit(f"Py_XSETREF({dst.name}, {cast}{src.name});") @@ -245,7 +245,6 @@ def analyze_registers(self, a: "Analyzer") -> None: def write(self, out: Formatter) -> None: """Write one instruction, sans prologue and epilogue.""" # Write a static assertion that a family's cache size is correct - if family := self.family: if self.name == family.members[0]: if cache_size := family.size: @@ -256,11 +255,14 @@ def write(self, out: Formatter) -> None: if not self.register: # Write input stack effect variable declarations and initializations - for i, ieffect in enumerate(reversed(self.input_effects), 1): + ieffects = list(reversed(self.input_effects)) + for i in range(len(ieffects)): + ieffect = ieffects[i] + isize = string_effect_size(list_effect_size(ieffects[:i+1])) if ieffect.size: - src = StackEffect(f"&PEEK({ieffect.size})", "PyObject **") + src = StackEffect(f"&PEEK({isize})", "PyObject **") else: - src = StackEffect(f"PEEK({i})", "") + src = StackEffect(f"PEEK({isize})", "") out.declare(ieffect, src) else: # Write input register variable declarations and initializations @@ -284,28 +286,20 @@ def write(self, out: Formatter) -> None: if not self.register: # Write net stack growth/shrinkage - diff = 0 - symbolic: list[str] = [] - for ieff in self.input_effects: - if ieff.size: - symbolic.append(f"-{ieff.size}") - else: - diff -= 1 - for oeff in self.output_effects: - if oeff.size: - symbolic.append(oeff.size) - else: - diff += 1 - out.stack_adjust(diff, symbolic) + out.stack_adjust(0, self.input_effects, self.output_effects) # Write output stack effect assignments - for i, oeffect in enumerate(reversed(self.output_effects), 1): - if oeffect.name not in self.unmoved_names: - if oeffect.size: - dst = StackEffect(f"&PEEK({oeffect.size})", "PyObject **") - else: - dst = StackEffect(f"PEEK({i})", "") - out.assign(dst, oeffect) + oeffects = list(reversed(self.output_effects)) + for i in range(len(oeffects)): + oeffect = oeffects[i] + if oeffect.name in self.unmoved_names: + continue + osize = string_effect_size(list_effect_size(oeffects[:i+1])) + if oeffect.size: + dst = StackEffect(f"&PEEK({osize})", "PyObject **") + else: + dst = StackEffect(f"PEEK({osize})", "") + out.assign(dst, oeffect) else: # Write output register assignments for oeffect, reg in zip(self.output_effects, self.output_registers): @@ -684,6 +678,12 @@ def stack_analysis( for thing in components: match thing: case Instruction() as instr: + if any(eff.size for eff in instr.input_effects + instr.output_effects): + self.error( + f"Instruction {instr.name!r} has variable-sized stack effect, " + "which are not supported in super- or macro instructions", + instr.inst, # TODO: Pass name+location of super/macro + ) current -= len(instr.input_effects) lowest = min(lowest, current) current += len(instr.output_effects) @@ -924,7 +924,9 @@ def wrap_super_or_macro(self, up: SuperOrMacroInstruction): yield - self.out.stack_adjust(up.final_sp - up.initial_sp, []) + # TODO: Use slices of up.stack instead of numeric values + self.out.stack_adjust(up.final_sp - up.initial_sp, [], []) + for i, var in enumerate(reversed(up.stack[: up.final_sp]), 1): dst = StackEffect(f"PEEK({i})", "") self.out.assign(dst, var) diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index 476f1706421835..ff91edd8ecfce9 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -29,9 +29,9 @@ def test_effect_sizes(): assert generate_cases.net_effect_size(input_effects, output_effects) == (1, "oparg*4 - (oparg + oparg*2)") assert generate_cases.net_effect_size([], output_effects) == (2, "oparg*4") - assert generate_cases.string_effect_size(input_effects, []) == "-1 - (oparg + oparg*2)" - assert generate_cases.string_effect_size(input_effects, output_effects) == "1 + oparg*4 - (oparg + oparg*2)" - assert generate_cases.string_effect_size([], output_effects) == "2 + oparg*4" + assert generate_cases.string_effect_size(generate_cases.list_effect_size(input_effects)) == "1 + oparg + oparg*2" + assert generate_cases.string_effect_size(generate_cases.net_effect_size(input_effects, output_effects)) == "1 + oparg*4 - (oparg + oparg*2)" + assert generate_cases.string_effect_size(generate_cases.list_effect_size(output_effects)) == "2 + oparg*4" def run_cases_test(input: str, expected: str): @@ -151,6 +151,24 @@ def test_binary_op(): """ run_cases_test(input, output) +def test_overlap(): + input = """ + inst(OP, (left, right -- left, result)) { + spam(); + } + """ + output = """ + TARGET(OP) { + PyObject *right = PEEK(1); + PyObject *left = PEEK(2); + PyObject *result; + spam(); + POKE(1, result); + DISPATCH(); + } + """ + run_cases_test(input, output) + def test_predictions(): input = """ inst(OP1, (--)) { @@ -339,15 +357,18 @@ def test_macro_instruction(): def test_array_input(): input = """ - inst(OP, (values[oparg*2] --)) { + inst(OP, (below, values[oparg*2], above --)) { spam(); } """ output = """ TARGET(OP) { - PyObject **values = &PEEK(oparg*2); + PyObject *above = PEEK(1); + PyObject **values = &PEEK(1 + oparg*2); + PyObject *below = PEEK(2 + oparg*2); spam(); STACK_SHRINK(oparg*2); + STACK_SHRINK(2); DISPATCH(); } """ @@ -355,16 +376,58 @@ def test_array_input(): def test_array_output(): input = """ - inst(OP, (-- values[oparg*3])) { + inst(OP, (-- below, values[oparg*3], above)) { spam(); } """ output = """ TARGET(OP) { + PyObject *below; PyObject **values; + PyObject *above; spam(); + STACK_GROW(2); STACK_GROW(oparg*3); - MOVE_ITEMS(values, &PEEK(oparg*3), oparg*3); + POKE(1, above); + MOVE_ITEMS(values, &PEEK(1 + oparg*3), oparg*3); + POKE(2 + oparg*3, below); + DISPATCH(); + } + """ + run_cases_test(input, output) + +def test_array_input_output(): + input = """ + inst(OP, (below, values[oparg] -- values[oparg], above)) { + spam(); + } + """ + output = """ + TARGET(OP) { + PyObject **values = &PEEK(oparg); + PyObject *below = PEEK(1 + oparg); + PyObject *above; + spam(); + POKE(1, above); + MOVE_ITEMS(values, &PEEK(1 + oparg), oparg); + DISPATCH(); + } + """ + run_cases_test(input, output) + +def test_array_error_if(): + input = """ + inst(OP, (extra, values[oparg] --)) { + ERROR_IF(oparg == 0, somewhere); + } + """ + output = """ + TARGET(OP) { + PyObject **values = &PEEK(oparg); + PyObject *extra = PEEK(1 + oparg); + if (oparg == 0) { STACK_SHRINK(oparg); goto pop_1_somewhere; } + STACK_SHRINK(oparg); + STACK_SHRINK(1); DISPATCH(); } """ From b02323600b1c267d22dbbbe683ec0c559b4c4b7e Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sun, 8 Jan 2023 21:34:25 -0800 Subject: [PATCH 07/30] Fix TODO --- Tools/cases_generator/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index 932fe9f0f8f700..cdac80378d04a9 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -231,7 +231,7 @@ def cache_effect(self) -> CacheEffect | None: @contextual def stack_effect(self) -> StackEffect | None: # IDENTIFIER [':' IDENTIFIER] - # TODO: Arrays, conditions + # TODO: Conditions if tkn := self.expect(lx.IDENTIFIER): if self.expect(lx.COLON): typ = self.require(lx.IDENTIFIER) From 9ec91e19fc17e51ab33e31fa7a49a40b50dac05b Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sun, 8 Jan 2023 21:34:35 -0800 Subject: [PATCH 08/30] Fix opcode metadata for array stack effects --- Python/compile.c | 2 +- Python/opcode_metadata.h | 2 +- Tools/cases_generator/generate_cases.py | 8 ++++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 6d176a83a87acc..62f889eb35a557 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8873,7 +8873,7 @@ assemble(struct compiler *c, int addNone) } assert(no_redundant_jumps(g)); - // assert(opcode_metadata_is_sane(g)); + assert(opcode_metadata_is_sane(g)); /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(g->g_entryblock); diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 1e128c2c1478ca..fd6f6164888dec 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -88,7 +88,7 @@ static const struct { [LOAD_DEREF] = { 0, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [STORE_DEREF] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [COPY_FREE_VARS] = { 0, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, - [BUILD_STRING] = { 1, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, + [BUILD_STRING] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_TUPLE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_LIST] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [LIST_EXTEND] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index d37080ee4b4b42..f803488743ab49 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -679,6 +679,7 @@ def stack_analysis( match thing: case Instruction() as instr: if any(eff.size for eff in instr.input_effects + instr.output_effects): + # TODO: Eventually this will be needed, at least for macros. self.error( f"Instruction {instr.name!r} has variable-sized stack effect, " "which are not supported in super- or macro instructions", @@ -792,8 +793,11 @@ def write_metadata_for_inst(self, instr: Instruction) -> None: n_popped = n_pushed = -1 assert not instr.register else: - n_popped = len(instr.input_effects) - n_pushed = len(instr.output_effects) + n_popped, sym_popped = list_effect_size(instr.input_effects) + n_pushed, sym_pushed = list_effect_size(instr.output_effects) + if sym_popped or sym_pushed: + # TODO: Record symbolic effects (how?) + n_popped = n_pushed = -1 if instr.register: directions: list[str] = [] directions.extend("DIR_READ" for _ in instr.input_effects) From af785de41a0a5a008898314f08befb0ba4241962 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 9 Jan 2023 16:09:47 -0800 Subject: [PATCH 09/30] Array-ize LIST_APPEND --- Python/bytecodes.c | 7 +++---- Python/generated_cases.c.h | 4 ++-- Python/opcode_metadata.h | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f86649f64870b6..904b7f881d5e62 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -456,10 +456,9 @@ dummy_func( DISPATCH_INLINED(new_frame); } - // Alternative: (list, unused[oparg], v -- list, unused[oparg]) - inst(LIST_APPEND, (v --)) { - PyObject *list = PEEK(oparg + 1); // +1 to account for v staying on stack - ERROR_IF(_PyList_AppendTakeRef((PyListObject *)list, v) < 0, error); + // 'stuff' is a list object followed by (oparg - 1) unused values + inst(LIST_APPEND, (stuff[oparg], v -- stuff[oparg])) { + ERROR_IF(_PyList_AppendTakeRef((PyListObject *)stuff[0], v) < 0, error); PREDICT(JUMP_BACKWARD); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 818bc68ad45086..32e424f0b932c6 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -574,8 +574,8 @@ TARGET(LIST_APPEND) { PyObject *v = PEEK(1); - PyObject *list = PEEK(oparg + 1); // +1 to account for v staying on stack - if (_PyList_AppendTakeRef((PyListObject *)list, v) < 0) goto pop_1_error; + PyObject **stuff = &PEEK(1 + oparg); + if (_PyList_AppendTakeRef((PyListObject *)stuff[0], v) < 0) goto pop_1_error; STACK_SHRINK(1); PREDICT(JUMP_BACKWARD); DISPATCH(); diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index fd6f6164888dec..4816439321bbd2 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -44,7 +44,7 @@ static const struct { [BINARY_SUBSCR_TUPLE_INT] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC000" }, [BINARY_SUBSCR_DICT] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC000" }, [BINARY_SUBSCR_GETITEM] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC000" }, - [LIST_APPEND] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, + [LIST_APPEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [SET_ADD] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [STORE_SUBSCR] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC" }, [STORE_SUBSCR_LIST_INT] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC" }, From 4fddf2b5952e53cd0ded93009cec1942d4d6331e Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 9 Jan 2023 16:38:53 -0800 Subject: [PATCH 10/30] Array-ize SET_ADD --- Python/bytecodes.c | 7 +++---- Python/generated_cases.c.h | 4 ++-- Python/opcode_metadata.h | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 904b7f881d5e62..139fc57ebc02cc 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -462,10 +462,9 @@ dummy_func( PREDICT(JUMP_BACKWARD); } - // Alternative: (set, unused[oparg], v -- set, unused[oparg]) - inst(SET_ADD, (v --)) { - PyObject *set = PEEK(oparg + 1); // +1 to account for v staying on stack - int err = PySet_Add(set, v); + // Ditto + inst(SET_ADD, (stuff[oparg], v -- stuff[oparg])) { + int err = PySet_Add(stuff[0], v); Py_DECREF(v); ERROR_IF(err, error); PREDICT(JUMP_BACKWARD); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 32e424f0b932c6..46588a914c9a15 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -583,8 +583,8 @@ TARGET(SET_ADD) { PyObject *v = PEEK(1); - PyObject *set = PEEK(oparg + 1); // +1 to account for v staying on stack - int err = PySet_Add(set, v); + PyObject **stuff = &PEEK(1 + oparg); + int err = PySet_Add(stuff[0], v); Py_DECREF(v); if (err) goto pop_1_error; STACK_SHRINK(1); diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 4816439321bbd2..0cf40426eb5f15 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -45,7 +45,7 @@ static const struct { [BINARY_SUBSCR_DICT] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC000" }, [BINARY_SUBSCR_GETITEM] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC000" }, [LIST_APPEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, - [SET_ADD] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, + [SET_ADD] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [STORE_SUBSCR] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC" }, [STORE_SUBSCR_LIST_INT] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC" }, [STORE_SUBSCR_DICT] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IBC" }, From ac221153f7ce83b113bd4acab589df5de1b6f254 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 9 Jan 2023 16:42:18 -0800 Subject: [PATCH 11/30] Array-ize LIST_EXTEND --- Python/bytecodes.c | 6 +++--- Python/generated_cases.c.h | 4 ++-- Python/opcode_metadata.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 139fc57ebc02cc..9b509d7ebb5a0a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1325,9 +1325,9 @@ dummy_func( PUSH(list); } - inst(LIST_EXTEND, (iterable -- )) { - PyObject *list = PEEK(oparg + 1); // iterable is still on the stack - PyObject *none_val = _PyList_Extend((PyListObject *)list, iterable); + // 'stuff' is a list object followed by (oparg - 1) unused values + inst(LIST_EXTEND, (stuff[oparg], iterable -- stuff[oparg])) { + PyObject *none_val = _PyList_Extend((PyListObject *)stuff[0], iterable); if (none_val == NULL) { if (_PyErr_ExceptionMatches(tstate, PyExc_TypeError) && (Py_TYPE(iterable)->tp_iter == NULL && !PySequence_Check(iterable))) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 46588a914c9a15..c227eb20d5ec80 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1545,8 +1545,8 @@ TARGET(LIST_EXTEND) { PyObject *iterable = PEEK(1); - PyObject *list = PEEK(oparg + 1); // iterable is still on the stack - PyObject *none_val = _PyList_Extend((PyListObject *)list, iterable); + PyObject **stuff = &PEEK(1 + oparg); + PyObject *none_val = _PyList_Extend((PyListObject *)stuff[0], iterable); if (none_val == NULL) { if (_PyErr_ExceptionMatches(tstate, PyExc_TypeError) && (Py_TYPE(iterable)->tp_iter == NULL && !PySequence_Check(iterable))) diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 0cf40426eb5f15..cf96d753482884 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -91,7 +91,7 @@ static const struct { [BUILD_STRING] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_TUPLE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_LIST] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, - [LIST_EXTEND] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, + [LIST_EXTEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [SET_UPDATE] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_SET] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_MAP] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, From 6c83be037346e4ed1b5b45c5b4fbf3608b9058de Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 9 Jan 2023 16:46:58 -0800 Subject: [PATCH 12/30] Array-ize SET_UPDATE --- Python/bytecodes.c | 8 ++++---- Python/generated_cases.c.h | 4 ++-- Python/opcode_metadata.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 9b509d7ebb5a0a..3319d29257b3c1 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -462,7 +462,7 @@ dummy_func( PREDICT(JUMP_BACKWARD); } - // Ditto + // 'stuff' is a set object followed by (oparg - 1) unused values inst(SET_ADD, (stuff[oparg], v -- stuff[oparg])) { int err = PySet_Add(stuff[0], v); Py_DECREF(v); @@ -1344,9 +1344,9 @@ dummy_func( DECREF_INPUTS(); } - inst(SET_UPDATE, (iterable --)) { - PyObject *set = PEEK(oparg + 1); // iterable is still on the stack - int err = _PySet_Update(set, iterable); + // 'stuff' is a set object followed by (oparg - 1) unused values + inst(SET_UPDATE, (stuff[oparg], iterable -- stuff[oparg])) { + int err = _PySet_Update(stuff[0], iterable); DECREF_INPUTS(); ERROR_IF(err < 0, error); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index c227eb20d5ec80..4b8828fa00376c 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1567,8 +1567,8 @@ TARGET(SET_UPDATE) { PyObject *iterable = PEEK(1); - PyObject *set = PEEK(oparg + 1); // iterable is still on the stack - int err = _PySet_Update(set, iterable); + PyObject **stuff = &PEEK(1 + oparg); + int err = _PySet_Update(stuff[0], iterable); Py_DECREF(iterable); if (err < 0) goto pop_1_error; STACK_SHRINK(1); diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index cf96d753482884..4396bb6ccd0d77 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -92,7 +92,7 @@ static const struct { [BUILD_TUPLE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_LIST] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [LIST_EXTEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, - [SET_UPDATE] = { 1, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, + [SET_UPDATE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_SET] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [BUILD_MAP] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, [SETUP_ANNOTATIONS] = { 0, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, "IB" }, From c7999dd43f69abf6a429776605a968888f7d366e Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 9 Jan 2023 21:08:49 -0800 Subject: [PATCH 13/30] Array-ize BUILD_TUPLE --- Python/bytecodes.c | 10 +++------- Python/generated_cases.c.h | 10 ++++++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 3319d29257b3c1..a54109331bf51d 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1307,13 +1307,9 @@ dummy_func( } } - // stack effect: (__array[oparg] -- __0) - inst(BUILD_TUPLE) { - STACK_SHRINK(oparg); - PyObject *tup = _PyTuple_FromArraySteal(stack_pointer, oparg); - if (tup == NULL) - goto error; - PUSH(tup); + inst(BUILD_TUPLE, (values[oparg] -- tup)) { + tup = _PyTuple_FromArraySteal(values, oparg); + ERROR_IF(tup == NULL, error); } // stack effect: (__array[oparg] -- __0) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 4b8828fa00376c..963ee474008674 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1526,11 +1526,13 @@ } TARGET(BUILD_TUPLE) { + PyObject **values = &PEEK(oparg); + PyObject *tup; + tup = _PyTuple_FromArraySteal(values, oparg); + if (tup == NULL) { STACK_SHRINK(oparg); goto error; } STACK_SHRINK(oparg); - PyObject *tup = _PyTuple_FromArraySteal(stack_pointer, oparg); - if (tup == NULL) - goto error; - PUSH(tup); + STACK_GROW(1); + POKE(1, tup); DISPATCH(); } From a7aa4257d055959f9a75f01f7a2bf1cdf09bc115 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 9 Jan 2023 21:10:09 -0800 Subject: [PATCH 14/30] Array-ize BUILD_LIST --- Python/bytecodes.c | 10 +++------- Python/generated_cases.c.h | 10 ++++++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index a54109331bf51d..b042f2d9817ecb 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1312,13 +1312,9 @@ dummy_func( ERROR_IF(tup == NULL, error); } - // stack effect: (__array[oparg] -- __0) - inst(BUILD_LIST) { - STACK_SHRINK(oparg); - PyObject *list = _PyList_FromArraySteal(stack_pointer, oparg); - if (list == NULL) - goto error; - PUSH(list); + inst(BUILD_LIST, (values[oparg] -- list)) { + list = _PyList_FromArraySteal(values, oparg); + ERROR_IF(list == NULL, error); } // 'stuff' is a list object followed by (oparg - 1) unused values diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 963ee474008674..4df08336315f5f 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1537,11 +1537,13 @@ } TARGET(BUILD_LIST) { + PyObject **values = &PEEK(oparg); + PyObject *list; + list = _PyList_FromArraySteal(values, oparg); + if (list == NULL) { STACK_SHRINK(oparg); goto error; } STACK_SHRINK(oparg); - PyObject *list = _PyList_FromArraySteal(stack_pointer, oparg); - if (list == NULL) - goto error; - PUSH(list); + STACK_GROW(1); + POKE(1, list); DISPATCH(); } From 11f6a583a6729a68413ab9c6321b0884acad03bf Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 9 Jan 2023 21:15:42 -0800 Subject: [PATCH 15/30] Use ERROR_IF() in BUILD_STRING --- Python/bytecodes.c | 3 +-- Python/generated_cases.c.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index b042f2d9817ecb..df4a5fc2bc73ee 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1300,11 +1300,10 @@ dummy_func( inst(BUILD_STRING, (pieces[oparg] -- str)) { str = _PyUnicode_JoinArray(&_Py_STR(empty), pieces, oparg); - if (str == NULL) - goto error; for (int i = 0; i < oparg; i++) { Py_DECREF(pieces[i]); } + ERROR_IF(str == NULL, error); } inst(BUILD_TUPLE, (values[oparg] -- tup)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 4df08336315f5f..9bb94d6efbe2d1 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1514,11 +1514,10 @@ PyObject **pieces = &PEEK(oparg); PyObject *str; str = _PyUnicode_JoinArray(&_Py_STR(empty), pieces, oparg); - if (str == NULL) - goto error; for (int i = 0; i < oparg; i++) { Py_DECREF(pieces[i]); } + if (str == NULL) { STACK_SHRINK(oparg); goto error; } STACK_SHRINK(oparg); STACK_GROW(1); POKE(1, str); From abff4ae9f4b3d488bfa898c0fad5c07e88501f94 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 11 Jan 2023 21:57:21 -0800 Subject: [PATCH 16/30] Apply most suggestions from code review --- Tools/cases_generator/parser.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index cdac80378d04a9..b5d39c2a155d00 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -230,7 +230,9 @@ def cache_effect(self) -> CacheEffect | None: @contextual def stack_effect(self) -> StackEffect | None: - # IDENTIFIER [':' IDENTIFIER] + # IDENTIFIER + # | IDENTIFIER ':' IDENTIFIER + # | IDENTIFIER '[' dimension ']' # TODO: Conditions if tkn := self.expect(lx.IDENTIFIER): if self.expect(lx.COLON): From a02a9eb8a4257b11886a9b75476d6c2f8b890d56 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 11 Jan 2023 22:50:55 -0800 Subject: [PATCH 17/30] Accept more general dimensions --- Tools/cases_generator/generate_cases.py | 21 +-------------------- Tools/cases_generator/parser.py | 14 ++++++-------- Tools/cases_generator/test_generator.py | 5 ----- 3 files changed, 7 insertions(+), 33 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 2c2ead7290c342..c74c555fb320a0 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -66,29 +66,10 @@ def list_effect_size(effects: list[StackEffect]) -> tuple[int, str]: return numeric, " + ".join(symbolic) -def net_effect_size(input_effects: list[StackEffect], output_effects: list[StackEffect]) -> tuple[int, str]: - input_numeric, input_symbolic = list_effect_size(input_effects) - output_numeric, output_symbolic = list_effect_size(output_effects) - if "+" in input_symbolic: - input_symbolic = f"({input_symbolic})" - if input_symbolic and output_symbolic: - symbolic = f"{output_symbolic} - {input_symbolic}" - elif input_symbolic: - symbolic = f"-{input_symbolic}" - elif output_symbolic: - symbolic = output_symbolic - else: - symbolic = "" - return output_numeric - input_numeric, symbolic - - def string_effect_size(arg: tuple[int, str]) -> str: numeric, symbolic = arg if numeric and symbolic: - if symbolic.startswith("-"): - return f"{numeric} - {symbolic[1:]}" - else: - return f"{numeric} + {symbolic}" + return f"{numeric} + {symbolic}" elif symbolic: return symbolic else: diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index b5d39c2a155d00..56b8280845cd3f 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -70,9 +70,7 @@ class StackEffect(Node): @dataclass class Dimension(Node): - # NAME ['*' NUMBER] - name: str - size: int = 1 + size: str @dataclass @@ -248,11 +246,11 @@ def stack_effect(self) -> StackEffect | None: @contextual def dimension(self) -> Dimension | None: - if name := self.expect(lx.IDENTIFIER): - if self.expect(lx.TIMES): - if size := self.expect(lx.NUMBER): - return Dimension(name.text, int(size.text)) - return Dimension(name.text) + tokens: list[lx.Token] = [] + while (tkn := self.peek()) and tkn.kind != lx.RBRACKET: + tokens.append(tkn) + self.next() + return Dimension(lx.to_text(tokens).strip()) @contextual def super_def(self) -> Super | None: diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index ff91edd8ecfce9..dd7b77e268b9c2 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -25,12 +25,7 @@ def test_effect_sizes(): assert generate_cases.list_effect_size(input_effects) == (1, "oparg + oparg*2") assert generate_cases.list_effect_size(output_effects) == (2, "oparg*4") - assert generate_cases.net_effect_size(input_effects, []) == (-1, "-(oparg + oparg*2)") - assert generate_cases.net_effect_size(input_effects, output_effects) == (1, "oparg*4 - (oparg + oparg*2)") - assert generate_cases.net_effect_size([], output_effects) == (2, "oparg*4") - assert generate_cases.string_effect_size(generate_cases.list_effect_size(input_effects)) == "1 + oparg + oparg*2" - assert generate_cases.string_effect_size(generate_cases.net_effect_size(input_effects, output_effects)) == "1 + oparg*4 - (oparg + oparg*2)" assert generate_cases.string_effect_size(generate_cases.list_effect_size(output_effects)) == "2 + oparg*4" From 2bcd90f818fb845041202084f91d3c1e33da6bae Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 11 Jan 2023 22:51:26 -0800 Subject: [PATCH 18/30] Fix LIST_EXTEND --- Python/bytecodes.c | 5 ++--- Python/generated_cases.c.h | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index df4a5fc2bc73ee..675d2500b2bec5 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1316,9 +1316,8 @@ dummy_func( ERROR_IF(list == NULL, error); } - // 'stuff' is a list object followed by (oparg - 1) unused values - inst(LIST_EXTEND, (stuff[oparg], iterable -- stuff[oparg])) { - PyObject *none_val = _PyList_Extend((PyListObject *)stuff[0], iterable); + inst(LIST_EXTEND, (list, unused[oparg-1], iterable -- list, unused[oparg-1])) { + PyObject *none_val = _PyList_Extend((PyListObject *)list, iterable); if (none_val == NULL) { if (_PyErr_ExceptionMatches(tstate, PyExc_TypeError) && (Py_TYPE(iterable)->tp_iter == NULL && !PySequence_Check(iterable))) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 9bb94d6efbe2d1..81020f032474e6 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1548,8 +1548,8 @@ TARGET(LIST_EXTEND) { PyObject *iterable = PEEK(1); - PyObject **stuff = &PEEK(1 + oparg); - PyObject *none_val = _PyList_Extend((PyListObject *)stuff[0], iterable); + PyObject *list = PEEK(2 + oparg-1); + PyObject *none_val = _PyList_Extend((PyListObject *)list, iterable); if (none_val == NULL) { if (_PyErr_ExceptionMatches(tstate, PyExc_TypeError) && (Py_TYPE(iterable)->tp_iter == NULL && !PySequence_Check(iterable))) From c579a40d654b7a8e9a3cc9be0664b995b2fcd664 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 11 Jan 2023 23:08:05 -0800 Subject: [PATCH 19/30] Fix LIST_APPEND, SET_ADD, SET_UPDATE --- Python/bytecodes.c | 15 ++++++--------- Python/generated_cases.c.h | 12 ++++++------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 675d2500b2bec5..fbb015df52f1f4 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -456,15 +456,13 @@ dummy_func( DISPATCH_INLINED(new_frame); } - // 'stuff' is a list object followed by (oparg - 1) unused values - inst(LIST_APPEND, (stuff[oparg], v -- stuff[oparg])) { - ERROR_IF(_PyList_AppendTakeRef((PyListObject *)stuff[0], v) < 0, error); + inst(LIST_APPEND, (list, unused[oparg-1], v -- list, unused[oparg-1])) { + ERROR_IF(_PyList_AppendTakeRef((PyListObject *)list, v) < 0, error); PREDICT(JUMP_BACKWARD); } - // 'stuff' is a set object followed by (oparg - 1) unused values - inst(SET_ADD, (stuff[oparg], v -- stuff[oparg])) { - int err = PySet_Add(stuff[0], v); + inst(SET_ADD, (set, unused[oparg-1], v -- set, unused[oparg-1])) { + int err = PySet_Add(set, v); Py_DECREF(v); ERROR_IF(err, error); PREDICT(JUMP_BACKWARD); @@ -1334,9 +1332,8 @@ dummy_func( DECREF_INPUTS(); } - // 'stuff' is a set object followed by (oparg - 1) unused values - inst(SET_UPDATE, (stuff[oparg], iterable -- stuff[oparg])) { - int err = _PySet_Update(stuff[0], iterable); + inst(SET_UPDATE, (set, unused[oparg-1], iterable -- set, unused[oparg-1])) { + int err = _PySet_Update(set, iterable); DECREF_INPUTS(); ERROR_IF(err < 0, error); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 81020f032474e6..c0c78be86c8712 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -574,8 +574,8 @@ TARGET(LIST_APPEND) { PyObject *v = PEEK(1); - PyObject **stuff = &PEEK(1 + oparg); - if (_PyList_AppendTakeRef((PyListObject *)stuff[0], v) < 0) goto pop_1_error; + PyObject *list = PEEK(2 + oparg-1); + if (_PyList_AppendTakeRef((PyListObject *)list, v) < 0) goto pop_1_error; STACK_SHRINK(1); PREDICT(JUMP_BACKWARD); DISPATCH(); @@ -583,8 +583,8 @@ TARGET(SET_ADD) { PyObject *v = PEEK(1); - PyObject **stuff = &PEEK(1 + oparg); - int err = PySet_Add(stuff[0], v); + PyObject *set = PEEK(2 + oparg-1); + int err = PySet_Add(set, v); Py_DECREF(v); if (err) goto pop_1_error; STACK_SHRINK(1); @@ -1570,8 +1570,8 @@ TARGET(SET_UPDATE) { PyObject *iterable = PEEK(1); - PyObject **stuff = &PEEK(1 + oparg); - int err = _PySet_Update(stuff[0], iterable); + PyObject *set = PEEK(2 + oparg-1); + int err = _PySet_Update(set, iterable); Py_DECREF(iterable); if (err < 0) goto pop_1_error; STACK_SHRINK(1); From 70e983df334dccf53791478759856de5100ccfc5 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 13 Jan 2023 17:36:12 -0800 Subject: [PATCH 20/30] Array-ize BUILD_SET --- Python/bytecodes.c | 16 +++++----------- Python/generated_cases.c.h | 18 +++++++++--------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f70e57da05add8..a184dd36fc7490 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1338,25 +1338,19 @@ dummy_func( ERROR_IF(err < 0, error); } - // stack effect: (__array[oparg] -- __0) - inst(BUILD_SET) { - PyObject *set = PySet_New(NULL); + inst(BUILD_SET, (values[oparg] -- set)) { + set = PySet_New(NULL); int err = 0; - int i; - if (set == NULL) - goto error; - for (i = oparg; i > 0; i--) { - PyObject *item = PEEK(i); + for (int i = 0; i < oparg; i++) { + PyObject *item = values[i]; if (err == 0) err = PySet_Add(set, item); Py_DECREF(item); } - STACK_SHRINK(oparg); if (err != 0) { Py_DECREF(set); - goto error; + ERROR_IF(true, error); } - PUSH(set); } // stack effect: (__array[oparg*2] -- __0) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index cfb29cbc72aa83..57117f69fcc3a7 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1579,23 +1579,23 @@ } TARGET(BUILD_SET) { - PyObject *set = PySet_New(NULL); + PyObject **values = &PEEK(oparg); + PyObject *set; + set = PySet_New(NULL); int err = 0; - int i; - if (set == NULL) - goto error; - for (i = oparg; i > 0; i--) { - PyObject *item = PEEK(i); + for (int i = 0; i < oparg; i++) { + PyObject *item = values[i]; if (err == 0) err = PySet_Add(set, item); Py_DECREF(item); } - STACK_SHRINK(oparg); if (err != 0) { Py_DECREF(set); - goto error; + if (true) { STACK_SHRINK(oparg); goto error; } } - PUSH(set); + STACK_SHRINK(oparg); + STACK_GROW(1); + POKE(1, set); DISPATCH(); } From 827481ee88d009104fff7d39e255e85a3f51faa8 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 13 Jan 2023 17:42:08 -0800 Subject: [PATCH 21/30] Array-ize BUILD_MAP --- Python/bytecodes.c | 17 ++++++++--------- Python/generated_cases.c.h | 19 ++++++++++++------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index a184dd36fc7490..8366aa85e3e7df 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1353,20 +1353,19 @@ dummy_func( } } - // stack effect: (__array[oparg*2] -- __0) - inst(BUILD_MAP) { - PyObject *map = _PyDict_FromItems( - &PEEK(2*oparg), 2, - &PEEK(2*oparg - 1), 2, + inst(BUILD_MAP, (values[oparg*2] -- map)) { + map = _PyDict_FromItems( + values, 2, + values+1, 2, oparg); if (map == NULL) goto error; - while (oparg--) { - Py_DECREF(POP()); - Py_DECREF(POP()); + for (int i = 0; i < oparg; i++) { + Py_DECREF(values[i*2]); + Py_DECREF(values[i*2+1]); } - PUSH(map); + ERROR_IF(map == NULL, error); } inst(SETUP_ANNOTATIONS, (--)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 57117f69fcc3a7..d433f9d0264edd 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1600,18 +1600,23 @@ } TARGET(BUILD_MAP) { - PyObject *map = _PyDict_FromItems( - &PEEK(2*oparg), 2, - &PEEK(2*oparg - 1), 2, + PyObject **values = &PEEK(oparg*2); + PyObject *map; + map = _PyDict_FromItems( + values, 2, + values+1, 2, oparg); if (map == NULL) goto error; - while (oparg--) { - Py_DECREF(POP()); - Py_DECREF(POP()); + for (int i = 0; i < oparg; i++) { + Py_DECREF(values[i*2]); + Py_DECREF(values[i*2+1]); } - PUSH(map); + if (map == NULL) { STACK_SHRINK(oparg*2); goto error; } + STACK_SHRINK(oparg*2); + STACK_GROW(1); + POKE(1, map); DISPATCH(); } From aa69c55b369e9e369c07c4a061abe9932020a98b Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 13 Jan 2023 17:51:29 -0800 Subject: [PATCH 22/30] Array-ize BUILD_CONST_KEY_MAP --- Python/bytecodes.c | 21 +++++++-------------- Python/generated_cases.c.h | 21 ++++++++++----------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 8366aa85e3e7df..50d9e2e5923cb3 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1410,28 +1410,21 @@ dummy_func( } } - // stack effect: (__array[oparg] -- ) - inst(BUILD_CONST_KEY_MAP) { - PyObject *map; - PyObject *keys = TOP(); + inst(BUILD_CONST_KEY_MAP, (values[oparg], keys -- map)) { if (!PyTuple_CheckExact(keys) || PyTuple_GET_SIZE(keys) != (Py_ssize_t)oparg) { _PyErr_SetString(tstate, PyExc_SystemError, "bad BUILD_CONST_KEY_MAP keys argument"); - goto error; + goto error; // Pop the keys and values. } map = _PyDict_FromItems( &PyTuple_GET_ITEM(keys, 0), 1, - &PEEK(oparg + 1), 1, oparg); - if (map == NULL) { - goto error; - } - - Py_DECREF(POP()); - while (oparg--) { - Py_DECREF(POP()); + values, 1, oparg); + Py_DECREF(keys); + for (int i = 0; i < oparg; i++) { + Py_DECREF(values[i]); } - PUSH(map); + ERROR_IF(map == NULL, error); } inst(DICT_UPDATE, (update --)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index d433f9d0264edd..d5e591a8d6a246 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1664,26 +1664,25 @@ } TARGET(BUILD_CONST_KEY_MAP) { + PyObject *keys = PEEK(1); + PyObject **values = &PEEK(1 + oparg); PyObject *map; - PyObject *keys = TOP(); if (!PyTuple_CheckExact(keys) || PyTuple_GET_SIZE(keys) != (Py_ssize_t)oparg) { _PyErr_SetString(tstate, PyExc_SystemError, "bad BUILD_CONST_KEY_MAP keys argument"); - goto error; + goto error; // Pop the keys and values. } map = _PyDict_FromItems( &PyTuple_GET_ITEM(keys, 0), 1, - &PEEK(oparg + 1), 1, oparg); - if (map == NULL) { - goto error; - } - - Py_DECREF(POP()); - while (oparg--) { - Py_DECREF(POP()); + values, 1, oparg); + Py_DECREF(keys); + for (int i = 0; i < oparg; i++) { + Py_DECREF(values[i]); } - PUSH(map); + if (map == NULL) { STACK_SHRINK(oparg); goto pop_1_error; } + STACK_SHRINK(oparg); + POKE(1, map); DISPATCH(); } From 6cbabc7847ff870145e748b0f10b1eb387e5c2e2 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 13 Jan 2023 18:10:55 -0800 Subject: [PATCH 23/30] Add comment warning that RAISE_VARARGS needs to stay legacy --- Python/bytecodes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 50d9e2e5923cb3..b8f5e49ec89259 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -533,7 +533,7 @@ dummy_func( ERROR_IF(res == NULL, error); } - // stack effect: (__array[oparg] -- ) + // This should remain a legacy instruction. inst(RAISE_VARARGS) { PyObject *cause = NULL, *exc = NULL; switch (oparg) { From c7cfe9aeaf43f2443d47b0eb816b9b1d62bbc50d Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 17 Jan 2023 12:22:34 -0800 Subject: [PATCH 24/30] Look for 'oparg' in the whole instruction --- Python/opcode_metadata.h | 8 ++++---- Tools/cases_generator/generate_cases.py | 8 ++++---- Tools/cases_generator/parser.py | 19 +++++++++++++++---- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 5cb9881d797eaf..4891f07627ccba 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -45,8 +45,8 @@ static const struct { [BINARY_SUBSCR_TUPLE_INT] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IXC000 }, [BINARY_SUBSCR_DICT] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IXC000 }, [BINARY_SUBSCR_GETITEM] = { 2, 1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IXC000 }, - [LIST_APPEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IX }, - [SET_ADD] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IX }, + [LIST_APPEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, + [SET_ADD] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [STORE_SUBSCR] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IXC }, [STORE_SUBSCR_LIST_INT] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IXC }, [STORE_SUBSCR_DICT] = { 3, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IXC }, @@ -92,8 +92,8 @@ static const struct { [BUILD_STRING] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [BUILD_TUPLE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [BUILD_LIST] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, - [LIST_EXTEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IX }, - [SET_UPDATE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IX }, + [LIST_EXTEND] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, + [SET_UPDATE] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [BUILD_SET] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [BUILD_MAP] = { -1, -1, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB }, [SETUP_ANNOTATIONS] = { 0, 0, DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IX }, diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index d0022fb2a49a35..079ba120a8a825 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -215,7 +215,7 @@ def __init__(self, inst: parser.InstDef): num_dummies = (num_regs // 2) * 2 + 1 - num_regs fmt = "I" + "B"*num_regs + "X"*num_dummies else: - if variable_used(inst.block, "oparg"): + if variable_used(inst, "oparg"): fmt = "IB" else: fmt = "IX" @@ -965,9 +965,9 @@ def always_exits(lines: list[str]) -> bool: ) -def variable_used(block: parser.Block, name: str) -> bool: - """Determine whether a variable with a given name is used in a block.""" - return any(token.kind == "IDENTIFIER" and token.text == name for token in block.tokens) +def variable_used(node: parser.Node, 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) def main(): diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index 8cd4e8c194cf85..d2ed73caa50e2b 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -51,12 +51,23 @@ def to_text(self, dedent: int = 0) -> str: tokens = context.owner.tokens begin = context.begin end = context.end - return lx.to_text(tokens[begin:end], dedent) + return lx.to_text(self.tokens, dedent) + + @property + def tokens(self) -> list[lx.Token]: + context = self.context + if not context: + return [] + tokens = context.owner.tokens + begin = context.begin + end = context.end + return tokens[begin:end] @dataclass class Block(Node): - tokens: list[lx.Token] + # This just holds a context which has the list of tokens. + pass @dataclass @@ -354,8 +365,8 @@ def members(self) -> list[str] | None: @contextual def block(self) -> Block: - tokens = self.c_blob() - return Block(tokens) + if self.c_blob(): + return Block() def c_blob(self) -> list[lx.Token]: tokens: list[lx.Token] = [] From a1a4688ff687f2cfacbaec6f92b5aac6aa8cb600 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 17 Jan 2023 12:27:52 -0800 Subject: [PATCH 25/30] Add docstring for effect_size() --- Tools/cases_generator/generate_cases.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 079ba120a8a825..7730c2769f9347 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -49,6 +49,15 @@ def effect_size(effect: StackEffect) -> tuple[int, str]: + """Return the 'size' impact of a stack effect. + + Returns a tuple (numeric, symbolic) where: + + - numeric is an int giving the statically analyzable size of the effect + - symbolic is a string representing a variable effect (e.g. 'oparg*2') + + At most one of these will be non-zero / non-empty. + """ if effect.size: return 0, effect.size else: From 16a2d99d0356e581a1825c748b80a49842826231 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 17 Jan 2023 12:42:25 -0800 Subject: [PATCH 26/30] Parenthesize certain symbolic effects Anything that's not 'oparg' or 'oparg*2' will be parenthesized. --- Python/generated_cases.c.h | 8 ++++---- Tools/cases_generator/generate_cases.py | 14 +++++++++++++- Tools/cases_generator/test_generator.py | 7 +++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index d5e591a8d6a246..8b2737b8284f98 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -574,7 +574,7 @@ TARGET(LIST_APPEND) { PyObject *v = PEEK(1); - PyObject *list = PEEK(2 + oparg-1); + PyObject *list = PEEK(2 + (oparg-1)); if (_PyList_AppendTakeRef((PyListObject *)list, v) < 0) goto pop_1_error; STACK_SHRINK(1); PREDICT(JUMP_BACKWARD); @@ -583,7 +583,7 @@ TARGET(SET_ADD) { PyObject *v = PEEK(1); - PyObject *set = PEEK(2 + oparg-1); + PyObject *set = PEEK(2 + (oparg-1)); int err = PySet_Add(set, v); Py_DECREF(v); if (err) goto pop_1_error; @@ -1548,7 +1548,7 @@ TARGET(LIST_EXTEND) { PyObject *iterable = PEEK(1); - PyObject *list = PEEK(2 + oparg-1); + PyObject *list = PEEK(2 + (oparg-1)); PyObject *none_val = _PyList_Extend((PyListObject *)list, iterable); if (none_val == NULL) { if (_PyErr_ExceptionMatches(tstate, PyExc_TypeError) && @@ -1570,7 +1570,7 @@ TARGET(SET_UPDATE) { PyObject *iterable = PEEK(1); - PyObject *set = PEEK(2 + oparg-1); + PyObject *set = PEEK(2 + (oparg-1)); int err = _PySet_Update(set, iterable); Py_DECREF(iterable); if (err < 0) goto pop_1_error; diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 7730c2769f9347..841a8c0672af83 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -64,6 +64,18 @@ def effect_size(effect: StackEffect) -> tuple[int, str]: return 1, "" +def maybe_parenthesize(sym: str) -> str: + """Add parentheses around a string if it contains an operator. + + An exception is made for '*' which is common and harmless + in the context where the symbolic size is used. + """ + if re.match(r"^[\s\w*]+$", sym): + return sym + else: + return f"({sym})" + + def list_effect_size(effects: list[StackEffect]) -> tuple[int, str]: numeric = 0 symbolic: list[str] = [] @@ -71,7 +83,7 @@ def list_effect_size(effects: list[StackEffect]) -> tuple[int, str]: diff, sym = effect_size(effect) numeric += diff if sym: - symbolic.append(sym) + symbolic.append(maybe_parenthesize(sym)) return numeric, " + ".join(symbolic) diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index dd7b77e268b9c2..5b9e8ae543094b 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -18,15 +18,22 @@ def test_effect_sizes(): b := StackEffect("b", "", "oparg*4"), c := StackEffect("c", "", ""), ] + other_effects = [ + p := StackEffect("p", "", "oparg<<1"), + q := StackEffect("q", "", ""), + r := StackEffect("r", "", ""), + ] assert generate_cases.effect_size(x) == (1, "") assert generate_cases.effect_size(y) == (0, "oparg") assert generate_cases.effect_size(z) == (0, "oparg*2") assert generate_cases.list_effect_size(input_effects) == (1, "oparg + oparg*2") assert generate_cases.list_effect_size(output_effects) == (2, "oparg*4") + assert generate_cases.list_effect_size(other_effects) == (2, "(oparg<<1)") assert generate_cases.string_effect_size(generate_cases.list_effect_size(input_effects)) == "1 + oparg + oparg*2" assert generate_cases.string_effect_size(generate_cases.list_effect_size(output_effects)) == "2 + oparg*4" + assert generate_cases.string_effect_size(generate_cases.list_effect_size(other_effects)) == "2 + (oparg<<1)" def run_cases_test(input: str, expected: str): From 1ef085d9d0ac0f29e882a4eb4a81f1bcc1228b1e Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 17 Jan 2023 12:45:03 -0800 Subject: [PATCH 27/30] Fix typo in comment --- Tools/cases_generator/generate_cases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 841a8c0672af83..053c1e1726241c 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -167,7 +167,7 @@ def assign(self, dst: StackEffect, src: StackEffect): # NOTE: MOVE_ITEMS() does not actually exist. # The only supported output array forms are: # - unused[...] - # - X[...] where X[...] matches an i99nput array form + # - X[...] where X[...] matches an input array exactly self.emit(f"MOVE_ITEMS({src.name}, {dst.name}, {src.size});") elif m := re.match(r"^REG\(oparg(\d+)\)$", dst.name): self.emit(f"Py_XSETREF({dst.name}, {cast}{src.name});") From 1bf42ced2d1598761f8895cdf2efeaa8ac0fbed9 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 17 Jan 2023 12:48:01 -0800 Subject: [PATCH 28/30] Swap args of imaginary MOVE_ITEMS() macro --- Tools/cases_generator/generate_cases.py | 2 +- Tools/cases_generator/test_generator.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 053c1e1726241c..1e9c4f9a6aa6b9 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -168,7 +168,7 @@ def assign(self, dst: StackEffect, src: StackEffect): # The only supported output array forms are: # - unused[...] # - X[...] where X[...] matches an input array exactly - self.emit(f"MOVE_ITEMS({src.name}, {dst.name}, {src.size});") + self.emit(f"MOVE_ITEMS({dst.name}, {src.name}, {src.size});") elif m := re.match(r"^REG\(oparg(\d+)\)$", dst.name): self.emit(f"Py_XSETREF({dst.name}, {cast}{src.name});") else: diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index 5b9e8ae543094b..ae0c1e2f97c35e 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -391,7 +391,7 @@ def test_array_output(): STACK_GROW(2); STACK_GROW(oparg*3); POKE(1, above); - MOVE_ITEMS(values, &PEEK(1 + oparg*3), oparg*3); + MOVE_ITEMS(&PEEK(1 + oparg*3), values, oparg*3); POKE(2 + oparg*3, below); DISPATCH(); } @@ -411,7 +411,7 @@ def test_array_input_output(): PyObject *above; spam(); POKE(1, above); - MOVE_ITEMS(values, &PEEK(1 + oparg), oparg); + MOVE_ITEMS(&PEEK(1 + oparg), values, oparg); DISPATCH(); } """ From c2be1ab0a2c2b9e62e5865837a6acf1fbe787318 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 17 Jan 2023 13:08:48 -0800 Subject: [PATCH 29/30] Tune for loops --- Tools/cases_generator/generate_cases.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 1e9c4f9a6aa6b9..59e249930b45df 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -275,8 +275,7 @@ def write(self, out: Formatter) -> None: if not self.register: # Write input stack effect variable declarations and initializations ieffects = list(reversed(self.input_effects)) - for i in range(len(ieffects)): - ieffect = ieffects[i] + for i, ieffect in enumerate(ieffects): isize = string_effect_size(list_effect_size(ieffects[:i+1])) if ieffect.size: src = StackEffect(f"&PEEK({isize})", "PyObject **") @@ -309,8 +308,7 @@ def write(self, out: Formatter) -> None: # Write output stack effect assignments oeffects = list(reversed(self.output_effects)) - for i in range(len(oeffects)): - oeffect = oeffects[i] + for i, oeffect in enumerate(oeffects): if oeffect.name in self.unmoved_names: continue osize = string_effect_size(list_effect_size(oeffects[:i+1])) From c7edea2268b2069abdd647362ab97839e52ed6d4 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 17 Jan 2023 13:16:54 -0800 Subject: [PATCH 30/30] Disallow empty dimension --- Tools/cases_generator/parser.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index d2ed73caa50e2b..c2cebe96ccd6be 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -261,6 +261,8 @@ def dimension(self) -> Dimension | None: while (tkn := self.peek()) and tkn.kind != lx.RBRACKET: tokens.append(tkn) self.next() + if not tokens: + return None return Dimension(lx.to_text(tokens).strip()) @contextual