From 7fb598a23dbb34209208eb6f8d94cc7d19477b33 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 14 Nov 2022 14:33:02 -0800 Subject: [PATCH 01/19] Add macro() and op() DSL features --- Python/bytecodes.c | 7 +++--- Python/generated_cases.c.h | 23 +++++++++++-------- Tools/cases_generator/generate_cases.py | 8 ++++--- Tools/cases_generator/parser.py | 30 ++++++++++++++++--------- 4 files changed, 42 insertions(+), 26 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 78f7d4ac061674..faee0a28efe0a5 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -70,6 +70,8 @@ do { \ #define DISPATCH_SAME_OPARG() ((void)0) #define inst(name, ...) case name: +#define op(name, ...) /* NAME is ignored */ +#define macro(name) static int MACRO_##name #define super(name) static int SUPER_##name #define family(name, ...) static int family_##name @@ -156,10 +158,7 @@ dummy_func( res = NULL; } - inst(END_FOR, (value1, value2 --)) { - Py_DECREF(value1); - Py_DECREF(value2); - } + macro(END_FOR) = POP_TOP + POP_TOP; inst(UNARY_POSITIVE, (value -- res)) { res = PyNumber_Positive(value); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 2c6333f8e61537..04df761d784e3e 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -78,15 +78,6 @@ DISPATCH(); } - TARGET(END_FOR) { - PyObject *value2 = PEEK(1); - PyObject *value1 = PEEK(2); - Py_DECREF(value1); - Py_DECREF(value2); - STACK_SHRINK(2); - DISPATCH(); - } - TARGET(UNARY_POSITIVE) { PyObject *value = PEEK(1); PyObject *res; @@ -3808,3 +3799,17 @@ } DISPATCH(); } + + TARGET(END_FOR) { + { + PyObject *value = PEEK(1); + Py_DECREF(value); + STACK_SHRINK(1); + } + { + PyObject *value = PEEK(1); + Py_DECREF(value); + STACK_SHRINK(1); + } + DISPATCH(); + } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index e11d0c77e99d27..186708c85e15ba 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -167,8 +167,8 @@ def __init__(self, filename: str): with open(filename) as f: self.src = f.read() - instrs: dict[str, Instruction] - supers: dict[str, parser.Super] + instrs: dict[str, Instruction] # Includes ops + supers: dict[str, parser.Super] # Includes macros families: dict[str, parser.Family] def parse(self) -> None: @@ -290,6 +290,8 @@ def write_instructions(self, filename: str) -> None: # Write regular instructions for name, instr in self.instrs.items(): + if instr.kind != "inst": + continue # ops are not real instructions f.write(f"\n{indent}TARGET({name}) {{\n") if instr.predicted: f.write(f"{indent} PREDICTED({name});\n") @@ -303,7 +305,7 @@ def write_instructions(self, filename: str) -> None: components = [self.instrs[name] for name in sup.ops] f.write(f"\n{indent}TARGET({sup.name}) {{\n") for i, instr in enumerate(components): - if i > 0: + if i > 0 and sup.kind == "super": f.write(f"{indent} NEXTOPARG();\n") f.write(f"{indent} next_instr++;\n") f.write(f"{indent} {{\n") diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index c511607fdf70ec..9727439afdf58c 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -1,7 +1,7 @@ """Parser for bytecodes.inst.""" from dataclasses import dataclass, field -from typing import NamedTuple, Callable, TypeVar +from typing import NamedTuple, Callable, TypeVar, Literal import lexer as lx from plexer import PLexer @@ -74,6 +74,7 @@ class CacheEffect(Node): @dataclass class InstHeader(Node): + kind: Literal["inst", "op"] name: str inputs: list[InputEffect] outputs: list[OutputEffect] @@ -81,9 +82,14 @@ class InstHeader(Node): @dataclass class InstDef(Node): + # TODO: Merge InstHeader and InstDef header: InstHeader block: Block + @property + def kind(self) -> str: + return self.header.kind + @property def name(self) -> str: return self.header.name @@ -99,6 +105,7 @@ def outputs(self) -> list[StackEffect]: @dataclass class Super(Node): + kind: Literal["macro", "super"] name: str ops: list[str] @@ -122,10 +129,12 @@ def inst_def(self) -> InstDef | None: @contextual def inst_header(self) -> InstHeader | None: - # inst(NAME) | inst(NAME, (inputs -- outputs)) + # inst(NAME) + # | inst(NAME, (inputs -- outputs)) + # | op(NAME, (inputs -- outputs)) # TODO: Error out when there is something unexpected. # TODO: Make INST a keyword in the lexer. - if (tkn := self.expect(lx.IDENTIFIER)) and tkn.text == "inst": + if (tkn := self.expect(lx.IDENTIFIER)) and (kind := tkn.text) in ("inst", "op"): if (self.expect(lx.LPAREN) and (tkn := self.expect(lx.IDENTIFIER))): name = tkn.text @@ -134,9 +143,10 @@ def inst_header(self) -> InstHeader | None: if self.expect(lx.RPAREN): if ((tkn := self.peek()) and tkn.kind == lx.LBRACE): - return InstHeader(name, inp, outp) - elif self.expect(lx.RPAREN): - return InstHeader(name, [], []) + return InstHeader(kind, name, inp, outp) + elif self.expect(lx.RPAREN) and kind == "inst": + # No legacy stack effect if kind is "op". + return InstHeader(kind, name, [], []) return None def stack_effect(self) -> tuple[list[InputEffect], list[OutputEffect]]: @@ -200,13 +210,13 @@ def output(self) -> OutputEffect | None: @contextual def super_def(self) -> Super | None: - if (tkn := self.expect(lx.IDENTIFIER)) and tkn.text == "super": + if (tkn := self.expect(lx.IDENTIFIER)) and (kind := tkn.text) in ("super", "macro"): if self.expect(lx.LPAREN): if (tkn := self.expect(lx.IDENTIFIER)): if self.expect(lx.RPAREN): if self.expect(lx.EQUALS): if ops := self.ops(): - res = Super(tkn.text, ops) + res = Super(kind, tkn.text, ops) return res def ops(self) -> list[str] | None: @@ -278,7 +288,7 @@ def c_blob(self) -> list[lx.Token]: filename = sys.argv[1] if filename == "-c" and sys.argv[2:]: src = sys.argv[2] - filename = None + filename = "" else: with open(filename) as f: src = f.read() @@ -287,7 +297,7 @@ def c_blob(self) -> list[lx.Token]: end = srclines.index("// END BYTECODES //") src = "\n".join(srclines[begin+1 : end]) else: - filename = None + filename = "" src = "if (x) { x.foo; // comment\n}" parser = Parser(src, filename) x = parser.inst_def() or parser.super_def() or parser.family_def() From edae5af11b1de5c952eae321b52d4da38819598d Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 14 Nov 2022 22:03:19 -0800 Subject: [PATCH 02/19] Improve code generation for super/macro instructions (But this needs to be cleaned up for maintainability.) --- Python/generated_cases.c.h | 59 +++++++++++++++---------- Tools/cases_generator/generate_cases.py | 49 ++++++++++++++++++-- 2 files changed, 81 insertions(+), 27 deletions(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 04df761d784e3e..16558aa01f831f 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3702,13 +3702,14 @@ } TARGET(LOAD_FAST__LOAD_FAST) { + PyObject *_tmp_1; + PyObject *_tmp_2; { PyObject *value; value = GETLOCAL(oparg); assert(value != NULL); Py_INCREF(value); - STACK_GROW(1); - POKE(1, value); + _tmp_1 = value; } NEXTOPARG(); next_instr++; @@ -3717,20 +3718,23 @@ value = GETLOCAL(oparg); assert(value != NULL); Py_INCREF(value); - STACK_GROW(1); - POKE(1, value); + _tmp_2 = value; } + STACK_GROW(2); + POKE(1, _tmp_2); // lowest=0 current=2 highest=2 i=0 + POKE(2, _tmp_1); // lowest=0 current=2 highest=2 i=1 DISPATCH(); } TARGET(LOAD_FAST__LOAD_CONST) { + PyObject *_tmp_1; + PyObject *_tmp_2; { PyObject *value; value = GETLOCAL(oparg); assert(value != NULL); Py_INCREF(value); - STACK_GROW(1); - POKE(1, value); + _tmp_1 = value; } NEXTOPARG(); next_instr++; @@ -3738,17 +3742,19 @@ PyObject *value; value = GETITEM(consts, oparg); Py_INCREF(value); - STACK_GROW(1); - POKE(1, value); + _tmp_2 = value; } + STACK_GROW(2); + POKE(1, _tmp_2); // lowest=0 current=2 highest=2 i=0 + POKE(2, _tmp_1); // lowest=0 current=2 highest=2 i=1 DISPATCH(); } TARGET(STORE_FAST__LOAD_FAST) { + PyObject *_tmp_1 = PEEK(1); { - PyObject *value = PEEK(1); + PyObject *value = _tmp_1; SETLOCAL(oparg, value); - STACK_SHRINK(1); } NEXTOPARG(); next_instr++; @@ -3757,35 +3763,37 @@ value = GETLOCAL(oparg); assert(value != NULL); Py_INCREF(value); - STACK_GROW(1); - POKE(1, value); + _tmp_1 = value; } + POKE(1, _tmp_1); // lowest=-1 current=0 highest=0 i=-1 DISPATCH(); } TARGET(STORE_FAST__STORE_FAST) { + PyObject *_tmp_1 = PEEK(2); + PyObject *_tmp_2 = PEEK(1); { - PyObject *value = PEEK(1); + PyObject *value = _tmp_2; SETLOCAL(oparg, value); - STACK_SHRINK(1); } NEXTOPARG(); next_instr++; { - PyObject *value = PEEK(1); + PyObject *value = _tmp_1; SETLOCAL(oparg, value); - STACK_SHRINK(1); } + STACK_SHRINK(2); DISPATCH(); } TARGET(LOAD_CONST__LOAD_FAST) { + PyObject *_tmp_1; + PyObject *_tmp_2; { PyObject *value; value = GETITEM(consts, oparg); Py_INCREF(value); - STACK_GROW(1); - POKE(1, value); + _tmp_1 = value; } NEXTOPARG(); next_instr++; @@ -3794,22 +3802,25 @@ value = GETLOCAL(oparg); assert(value != NULL); Py_INCREF(value); - STACK_GROW(1); - POKE(1, value); + _tmp_2 = value; } + STACK_GROW(2); + POKE(1, _tmp_2); // lowest=0 current=2 highest=2 i=0 + POKE(2, _tmp_1); // lowest=0 current=2 highest=2 i=1 DISPATCH(); } TARGET(END_FOR) { + PyObject *_tmp_1 = PEEK(2); + PyObject *_tmp_2 = PEEK(1); { - PyObject *value = PEEK(1); + PyObject *value = _tmp_2; Py_DECREF(value); - STACK_SHRINK(1); } { - PyObject *value = PEEK(1); + PyObject *value = _tmp_1; Py_DECREF(value); - STACK_SHRINK(1); } + STACK_SHRINK(2); DISPATCH(); } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 186708c85e15ba..ebfd19792c428f 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -300,17 +300,41 @@ def write_instructions(self, filename: str) -> None: f.write(f"{indent} DISPATCH();\n") f.write(f"{indent}}}\n") - # Write super-instructions + # Write super-instructions and macros + # TODO: Cleanup the hacks for name, sup in self.supers.items(): - components = [self.instrs[name] for name in sup.ops] f.write(f"\n{indent}TARGET({sup.name}) {{\n") + components = [self.instrs[name] for name in sup.ops] + lowest, highest = self.super_macro_analysis(name, components) + current = 0 + for i in range(lowest, current): + f.write(f"{indent} PyObject *_tmp_{i - lowest + 1} = PEEK({-i});\n") + for i in range(current, highest): + f.write(f"{indent} PyObject *_tmp_{i - lowest + 1};\n") for i, instr in enumerate(components): if i > 0 and sup.kind == "super": f.write(f"{indent} NEXTOPARG();\n") f.write(f"{indent} next_instr++;\n") f.write(f"{indent} {{\n") - instr.write(f, indent, dedent=-4) + for seffect in reversed(instr.input_effects): + if seffect.name != "unused": + f.write(f"{indent} PyObject *{seffect.name} = _tmp_{current - lowest};\n") + current -= 1 + for oeffect in instr.output_effects: + if oeffect.name != "unused": + f.write(f"{indent} PyObject *{oeffect.name};\n") + instr.write_body(f, indent, dedent=-4) + for oeffect in instr.output_effects: + if oeffect.name != "unused": + f.write(f"{indent} _tmp_{current - lowest + 1} = {oeffect.name};\n") + current += 1 f.write(f" {indent}}}\n") + if current > 0: + f.write(f"{indent} STACK_GROW({current});\n") + elif current < 0: + f.write(f"{indent} STACK_SHRINK({-current});\n") + for i in range(lowest, current): + f.write(f"{indent} POKE({i - lowest + 1}, _tmp_{current - i});\n") f.write(f"{indent} DISPATCH();\n") f.write(f"{indent}}}\n") @@ -320,6 +344,25 @@ def write_instructions(self, filename: str) -> None: file=sys.stderr, ) + # TODO: Move this into analysis phase + def super_macro_analysis(self, name: str, components: list[Instruction]) -> tuple[int, int]: + """Analyze a super-instruction or macro.""" + lowest = current = highest = 0 + for instr in components: + if instr.cache_effects: + print( + f"Super-instruction {name!r} has cache effects in {instr.name!r}", + file=sys.stderr, + ) + self.errors += 1 + current -= len(instr.input_effects) + lowest = min(lowest, current) + current += len(instr.output_effects) + highest = max(highest, current) + # At this point, 'current' is the net stack effect, + # and 'lowest' and 'highest' are the extremes. + return lowest, highest + def always_exits(block: parser.Block) -> bool: """Determine whether a block always ends in a return/goto/etc.""" From 881357edd8edcd4b5dfcde66c0c6109bd0f323ed Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 15 Nov 2022 16:17:50 -0800 Subject: [PATCH 03/19] Move super code generation into a helper --- Python/generated_cases.c.h | 14 ++-- Tools/cases_generator/generate_cases.py | 91 ++++++++++++++----------- 2 files changed, 59 insertions(+), 46 deletions(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 16558aa01f831f..f5639c4e13d11f 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3721,8 +3721,8 @@ _tmp_2 = value; } STACK_GROW(2); - POKE(1, _tmp_2); // lowest=0 current=2 highest=2 i=0 - POKE(2, _tmp_1); // lowest=0 current=2 highest=2 i=1 + POKE(1, _tmp_2); + POKE(2, _tmp_1); DISPATCH(); } @@ -3745,8 +3745,8 @@ _tmp_2 = value; } STACK_GROW(2); - POKE(1, _tmp_2); // lowest=0 current=2 highest=2 i=0 - POKE(2, _tmp_1); // lowest=0 current=2 highest=2 i=1 + POKE(1, _tmp_2); + POKE(2, _tmp_1); DISPATCH(); } @@ -3765,7 +3765,7 @@ Py_INCREF(value); _tmp_1 = value; } - POKE(1, _tmp_1); // lowest=-1 current=0 highest=0 i=-1 + POKE(1, _tmp_1); DISPATCH(); } @@ -3805,8 +3805,8 @@ _tmp_2 = value; } STACK_GROW(2); - POKE(1, _tmp_2); // lowest=0 current=2 highest=2 i=0 - POKE(2, _tmp_1); // lowest=0 current=2 highest=2 i=1 + POKE(1, _tmp_2); + POKE(2, _tmp_1); DISPATCH(); } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index ebfd19792c428f..d606ebe6e9149b 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -198,7 +198,7 @@ def parse(self) -> None: print( f"Read {len(self.instrs)} instructions, " - f"{len(self.supers)} supers, " + f"{len(self.supers)} supers/macros, " f"and {len(self.families)} families from {self.filename}", file=sys.stderr, ) @@ -289,9 +289,11 @@ def write_instructions(self, filename: str) -> None: f.write(f"// Do not edit!\n") # Write regular instructions + n_instrs = 0 for name, instr in self.instrs.items(): if instr.kind != "inst": continue # ops are not real instructions + n_instrs += 1 f.write(f"\n{indent}TARGET({name}) {{\n") if instr.predicted: f.write(f"{indent} PREDICTED({name});\n") @@ -301,49 +303,60 @@ def write_instructions(self, filename: str) -> None: f.write(f"{indent}}}\n") # Write super-instructions and macros - # TODO: Cleanup the hacks - for name, sup in self.supers.items(): - f.write(f"\n{indent}TARGET({sup.name}) {{\n") - components = [self.instrs[name] for name in sup.ops] - lowest, highest = self.super_macro_analysis(name, components) - current = 0 - for i in range(lowest, current): - f.write(f"{indent} PyObject *_tmp_{i - lowest + 1} = PEEK({-i});\n") - for i in range(current, highest): - f.write(f"{indent} PyObject *_tmp_{i - lowest + 1};\n") - for i, instr in enumerate(components): - if i > 0 and sup.kind == "super": - f.write(f"{indent} NEXTOPARG();\n") - f.write(f"{indent} next_instr++;\n") - f.write(f"{indent} {{\n") - for seffect in reversed(instr.input_effects): - if seffect.name != "unused": - f.write(f"{indent} PyObject *{seffect.name} = _tmp_{current - lowest};\n") - current -= 1 - for oeffect in instr.output_effects: - if oeffect.name != "unused": - f.write(f"{indent} PyObject *{oeffect.name};\n") - instr.write_body(f, indent, dedent=-4) - for oeffect in instr.output_effects: - if oeffect.name != "unused": - f.write(f"{indent} _tmp_{current - lowest + 1} = {oeffect.name};\n") - current += 1 - f.write(f" {indent}}}\n") - if current > 0: - f.write(f"{indent} STACK_GROW({current});\n") - elif current < 0: - f.write(f"{indent} STACK_SHRINK({-current});\n") - for i in range(lowest, current): - f.write(f"{indent} POKE({i - lowest + 1}, _tmp_{current - i});\n") - f.write(f"{indent} DISPATCH();\n") - f.write(f"{indent}}}\n") + n_supers = 0 + n_macros = 0 + for sup in self.supers.values(): + if sup.kind == "super": + n_supers += 1 + elif sup.kind == "macro": + n_macros += 1 + self.write_super_macro(f, sup, indent) print( - f"Wrote {len(self.instrs)} instructions and " - f"{len(self.supers)} super-instructions to {filename}", + f"Wrote {n_instrs} instructions, {n_supers} supers, " + f"and {n_macros} macros to {filename}", file=sys.stderr, ) + def write_super_macro( + self, f: typing.TextIO, sup: parser.Super, indent: str = "" + ) -> None: + f.write(f"\n{indent}TARGET({sup.name}) {{\n") + components = [self.instrs[name] for name in sup.ops] + lowest, highest = self.super_macro_analysis(sup.name, components) + # TODO: Rename tmp variables _tmp_A, _tmp_B, etc. + current = 0 + for i in range(lowest, current): + f.write(f"{indent} PyObject *_tmp_{i - lowest + 1} = PEEK({-i});\n") + for i in range(current, highest): + f.write(f"{indent} PyObject *_tmp_{i - lowest + 1};\n") + for i, instr in enumerate(components): + if i > 0 and sup.kind == "super": + f.write(f"{indent} NEXTOPARG();\n") + f.write(f"{indent} next_instr++;\n") + f.write(f"{indent} {{\n") + for seffect in reversed(instr.input_effects): + if seffect.name != "unused": + f.write(f"{indent} PyObject *{seffect.name} = _tmp_{current - lowest};\n") + current -= 1 + for oeffect in instr.output_effects: + if oeffect.name != "unused": + f.write(f"{indent} PyObject *{oeffect.name};\n") + instr.write_body(f, indent, dedent=-4) + for oeffect in instr.output_effects: + if oeffect.name != "unused": + f.write(f"{indent} _tmp_{current - lowest + 1} = {oeffect.name};\n") + current += 1 + f.write(f" {indent}}}\n") + if current > 0: + f.write(f"{indent} STACK_GROW({current});\n") + elif current < 0: + f.write(f"{indent} STACK_SHRINK({-current});\n") + for i in range(lowest, current): + f.write(f"{indent} POKE({i - lowest + 1}, _tmp_{current - i});\n") + f.write(f"{indent} DISPATCH();\n") + f.write(f"{indent}}}\n") + # TODO: Move this into analysis phase def super_macro_analysis(self, name: str, components: list[Instruction]) -> tuple[int, int]: """Analyze a super-instruction or macro.""" From 0f12c4034b9ad3c203557e224a05547dece39999 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 15 Nov 2022 17:34:29 -0800 Subject: [PATCH 04/19] Reduce the fiddling with integers in super analysis Also: - Add helpers for indent management; - Reformat with black. --- Tools/cases_generator/generate_cases.py | 128 +++++++++++++++--------- 1 file changed, 82 insertions(+), 46 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index d606ebe6e9149b..bc2b5ec1cb31cd 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -5,6 +5,7 @@ """ import argparse +import contextlib import os import re import sys @@ -51,9 +52,7 @@ def __init__(self, inst: parser.InstDef): ] self.output_effects = self.outputs # For consistency/completeness - def write( - self, f: typing.TextIO, indent: str, dedent: int = 0 - ) -> None: + def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: """Write one instruction, sans prologue and epilogue.""" if dedent < 0: indent += " " * -dedent # DO WE NEED THIS? @@ -114,9 +113,7 @@ def write( if self.cache_offset: f.write(f"{indent} next_instr += {self.cache_offset};\n") - def write_body( - self, f: typing.TextIO, ndent: str, dedent: int - ) -> None: + def write_body(self, f: typing.TextIO, ndent: str, dedent: int) -> None: """Write the instruction body.""" # Get lines of text with proper dedelt @@ -180,7 +177,9 @@ def parse(self) -> None: if tkn.text == BEGIN_MARKER: break else: - raise psr.make_syntax_error(f"Couldn't find {BEGIN_MARKER!r} in {psr.filename}") + raise psr.make_syntax_error( + f"Couldn't find {BEGIN_MARKER!r} in {psr.filename}" + ) # Parse until end marker self.instrs = {} @@ -292,7 +291,7 @@ def write_instructions(self, filename: str) -> None: n_instrs = 0 for name, instr in self.instrs.items(): if instr.kind != "inst": - continue # ops are not real instructions + continue # ops are not real instructions n_instrs += 1 f.write(f"\n{indent}TARGET({name}) {{\n") if instr.predicted: @@ -321,45 +320,80 @@ def write_instructions(self, filename: str) -> None: def write_super_macro( self, f: typing.TextIO, sup: parser.Super, indent: str = "" ) -> None: - f.write(f"\n{indent}TARGET({sup.name}) {{\n") - components = [self.instrs[name] for name in sup.ops] - lowest, highest = self.super_macro_analysis(sup.name, components) - # TODO: Rename tmp variables _tmp_A, _tmp_B, etc. - current = 0 - for i in range(lowest, current): - f.write(f"{indent} PyObject *_tmp_{i - lowest + 1} = PEEK({-i});\n") - for i in range(current, highest): - f.write(f"{indent} PyObject *_tmp_{i - lowest + 1};\n") - for i, instr in enumerate(components): - if i > 0 and sup.kind == "super": - f.write(f"{indent} NEXTOPARG();\n") - f.write(f"{indent} next_instr++;\n") - f.write(f"{indent} {{\n") - for seffect in reversed(instr.input_effects): - if seffect.name != "unused": - f.write(f"{indent} PyObject *{seffect.name} = _tmp_{current - lowest};\n") - current -= 1 - for oeffect in instr.output_effects: - if oeffect.name != "unused": - f.write(f"{indent} PyObject *{oeffect.name};\n") - instr.write_body(f, indent, dedent=-4) - for oeffect in instr.output_effects: - if oeffect.name != "unused": - f.write(f"{indent} _tmp_{current - lowest + 1} = {oeffect.name};\n") - current += 1 - f.write(f" {indent}}}\n") - if current > 0: - f.write(f"{indent} STACK_GROW({current});\n") - elif current < 0: - f.write(f"{indent} STACK_SHRINK({-current});\n") - for i in range(lowest, current): - f.write(f"{indent} POKE({i - lowest + 1}, _tmp_{current - i});\n") - f.write(f"{indent} DISPATCH();\n") - f.write(f"{indent}}}\n") + + # TODO: Make write() and block() methods of some Formatter class + def write(arg: str) -> None: + if arg: + f.write(f"{indent}{arg}\n") + else: + f.write("\n") + + @contextlib.contextmanager + def block(head: str): + if head: + write(head + " {") + else: + write("{") + nonlocal indent + indent += " " + yield + indent = indent[:-4] + write("}") + + write("") + with block(f"TARGET({sup.name})"): + components = [self.instrs[name] for name in sup.ops] + stack, nbelow = self.super_macro_analysis(sup.name, components) + sp = nbelow + + for i, var in enumerate(stack): + if i < sp: + write(f"PyObject *{var} = PEEK({sp - i});") + else: + write(f"PyObject *{var};") + + for i, instr in enumerate(components): + if i > 0 and sup.kind == "super": + write(f"NEXTOPARG();") + write(f"next_instr++;") + + with block(""): + instack = stack[sp - len(instr.input_effects) : sp] + for var, ineffect in zip(instack, instr.input_effects): + if ineffect.name != "unused": + write(f"PyObject *{ineffect.name} = {var};") + for outeffect in instr.output_effects: + if outeffect.name != "unused": + write(f"PyObject *{outeffect.name};") + + instr.write_body(f, indent, dedent=-4) + + sp -= len(instack) + nout = len(instr.output_effects) + sp += nout + outstack = stack[sp - nout : sp] + for var, outeffect in zip(outstack, instr.output_effects): + if outeffect.name != "unused": + write(f"{var} = {outeffect.name};") + + if sp > nbelow: + write(f"STACK_GROW({sp - nbelow});") + elif sp < nbelow: + write(f"STACK_SHRINK({nbelow - sp});") + for i, var in enumerate(reversed(stack[:sp]), 1): + write(f"POKE({i}, {var});") + write(f"DISPATCH();") # TODO: Move this into analysis phase - def super_macro_analysis(self, name: str, components: list[Instruction]) -> tuple[int, int]: - """Analyze a super-instruction or macro.""" + def super_macro_analysis( + self, name: str, components: list[Instruction] + ) -> tuple[list[str], int]: + """Analyze a super-instruction or macro. + + Print an error if there's a cache effect (which we don't support yet). + + Return the list of variable names and the initial stack pointer. + """ lowest = current = highest = 0 for instr in components: if instr.cache_effects: @@ -374,7 +408,9 @@ def super_macro_analysis(self, name: str, components: list[Instruction]) -> tupl highest = max(highest, current) # At this point, 'current' is the net stack effect, # and 'lowest' and 'highest' are the extremes. - return lowest, highest + # Note that 'lowest' may be negative. + stack = [f"_tmp_{i+1}" for i in range(highest - lowest)] + return stack, -lowest def always_exits(block: parser.Block) -> bool: From babdbf9a9a069ecc801c565757af041cbb58a818 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 15 Nov 2022 19:41:05 -0800 Subject: [PATCH 05/19] Do the super/macro analysis at analysis time --- Tools/cases_generator/generate_cases.py | 156 +++++++++++++++--------- 1 file changed, 95 insertions(+), 61 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index bc2b5ec1cb31cd..2aefeb42ce39c8 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -6,6 +6,7 @@ import argparse import contextlib +import dataclasses import os import re import sys @@ -151,6 +152,71 @@ def write_body(self, f: typing.TextIO, ndent: str, dedent: int) -> None: f.write(line) +@dataclasses.dataclass +class SuperComponent: + instr: Instruction + input_mapping: typing.Dict[str, parser.StackEffect] + output_mapping: typing.Dict[str, parser.StackEffect] + + +class SuperInstruction(parser.Super): + + stack: list[str] + initial_sp: int + final_sp: int + parts: list[SuperComponent] + + def __init__(self, sup: parser.Super): + super().__init__(sup.kind, sup.name, sup.ops) + self.context = sup.context + + def analyze(self, a: "Analyzer") -> None: + components = [a.instrs[name] for name in self.ops] + self.stack, self.initial_sp = self.super_macro_analysis(a, components) + sp = self.initial_sp + self.parts = [] + for instr in components: + input_mapping = {} + for ieffect in reversed(instr.input_effects): + sp -= 1 + if ieffect.name != "unused": + input_mapping[self.stack[sp]] = ieffect + output_mapping = {} + for oeffect in instr.output_effects: + if oeffect.name != "unused": + output_mapping[self.stack[sp]] = oeffect + sp += 1 + self.parts.append(SuperComponent(instr, input_mapping, output_mapping)) + self.final_sp = sp + + def super_macro_analysis( + self, a: "Analyzer", components: list[Instruction] + ) -> tuple[list[str], int]: + """Analyze a super-instruction or macro. + + Print an error if there's a cache effect (which we don't support yet). + + Return the list of variable names and the initial stack pointer. + """ + lowest = current = highest = 0 + for instr in components: + if instr.cache_effects: + print( + f"Super-instruction {self.name!r} has cache effects in {instr.name!r}", + file=sys.stderr, + ) + a.errors += 1 + current -= len(instr.input_effects) + lowest = min(lowest, current) + current += len(instr.output_effects) + highest = max(highest, current) + # At this point, 'current' is the net stack effect, + # and 'lowest' and 'highest' are the extremes. + # Note that 'lowest' may be negative. + stack = [f"_tmp_{i+1}" for i in range(highest - lowest)] + return stack, -lowest + + class Analyzer: """Parse input, analyze it, and write to output.""" @@ -166,6 +232,7 @@ def __init__(self, filename: str): instrs: dict[str, Instruction] # Includes ops supers: dict[str, parser.Super] # Includes macros + super_instrs: dict[str, SuperInstruction] families: dict[str, parser.Family] def parse(self) -> None: @@ -210,6 +277,7 @@ def analyze(self) -> None: self.find_predictions() self.map_families() self.check_families() + self.analyze_supers() def find_predictions(self) -> None: """Find the instructions that need PREDICTED() labels.""" @@ -278,6 +346,14 @@ def check_families(self) -> None: ) self.errors += 1 + def analyze_supers(self) -> None: + """Analyze each super instruction.""" + self.super_instrs = {} + for name, sup in self.supers.items(): + dup = SuperInstruction(sup) + dup.analyze(self) + self.super_instrs[name] = dup + def write_instructions(self, filename: str) -> None: """Write instructions to output file.""" indent = " " * 8 @@ -304,7 +380,7 @@ def write_instructions(self, filename: str) -> None: # Write super-instructions and macros n_supers = 0 n_macros = 0 - for sup in self.supers.values(): + for sup in self.super_instrs.values(): if sup.kind == "super": n_supers += 1 elif sup.kind == "macro": @@ -318,7 +394,7 @@ def write_instructions(self, filename: str) -> None: ) def write_super_macro( - self, f: typing.TextIO, sup: parser.Super, indent: str = "" + self, f: typing.TextIO, sup: SuperInstruction, indent: str = "" ) -> None: # TODO: Make write() and block() methods of some Formatter class @@ -342,76 +418,34 @@ def block(head: str): write("") with block(f"TARGET({sup.name})"): - components = [self.instrs[name] for name in sup.ops] - stack, nbelow = self.super_macro_analysis(sup.name, components) - sp = nbelow - - for i, var in enumerate(stack): - if i < sp: - write(f"PyObject *{var} = PEEK({sp - i});") + for i, var in enumerate(sup.stack): + if i < sup.initial_sp: + write(f"PyObject *{var} = PEEK({sup.initial_sp - i});") else: write(f"PyObject *{var};") - for i, instr in enumerate(components): + for i, comp in enumerate(sup.parts): if i > 0 and sup.kind == "super": write(f"NEXTOPARG();") write(f"next_instr++;") with block(""): - instack = stack[sp - len(instr.input_effects) : sp] - for var, ineffect in zip(instack, instr.input_effects): - if ineffect.name != "unused": - write(f"PyObject *{ineffect.name} = {var};") - for outeffect in instr.output_effects: - if outeffect.name != "unused": - write(f"PyObject *{outeffect.name};") - - instr.write_body(f, indent, dedent=-4) - - sp -= len(instack) - nout = len(instr.output_effects) - sp += nout - outstack = stack[sp - nout : sp] - for var, outeffect in zip(outstack, instr.output_effects): - if outeffect.name != "unused": - write(f"{var} = {outeffect.name};") - - if sp > nbelow: - write(f"STACK_GROW({sp - nbelow});") - elif sp < nbelow: - write(f"STACK_SHRINK({nbelow - sp});") - for i, var in enumerate(reversed(stack[:sp]), 1): + for var, ieffect in comp.input_mapping.items(): + write(f"PyObject *{ieffect.name} = {var};") + for oeffect in comp.output_mapping.values(): + write(f"PyObject *{oeffect.name};") + comp.instr.write_body(f, indent, dedent=-4) + for var, oeffect in comp.output_mapping.items(): + write(f"{var} = {oeffect.name};") + + if sup.final_sp > sup.initial_sp: + write(f"STACK_GROW({sup.final_sp - sup.initial_sp});") + elif sup.final_sp < sup.initial_sp: + write(f"STACK_SHRINK({sup.initial_sp - sup.final_sp});") + for i, var in enumerate(reversed(sup.stack[:sup.final_sp]), 1): write(f"POKE({i}, {var});") write(f"DISPATCH();") - # TODO: Move this into analysis phase - def super_macro_analysis( - self, name: str, components: list[Instruction] - ) -> tuple[list[str], int]: - """Analyze a super-instruction or macro. - - Print an error if there's a cache effect (which we don't support yet). - - Return the list of variable names and the initial stack pointer. - """ - lowest = current = highest = 0 - for instr in components: - if instr.cache_effects: - print( - f"Super-instruction {name!r} has cache effects in {instr.name!r}", - file=sys.stderr, - ) - self.errors += 1 - current -= len(instr.input_effects) - lowest = min(lowest, current) - current += len(instr.output_effects) - highest = max(highest, current) - # At this point, 'current' is the net stack effect, - # and 'lowest' and 'highest' are the extremes. - # Note that 'lowest' may be negative. - stack = [f"_tmp_{i+1}" for i in range(highest - lowest)] - return stack, -lowest - def always_exits(block: parser.Block) -> bool: """Determine whether a block always ends in a return/goto/etc.""" From 2fa0a048549498e8e47eafff2ef7ec72056cea89 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 16 Nov 2022 11:01:26 -0800 Subject: [PATCH 06/19] Convert WITH_EXCEPT_START, fix generator to make it work --- Python/bytecodes.c | 15 ++++----------- Python/generated_cases.c.h | 18 +++++++++--------- Tools/cases_generator/generate_cases.py | 5 +++-- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index faee0a28efe0a5..9fcbebde1447f0 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2724,8 +2724,7 @@ dummy_func( PUSH(res); } - // stack effect: ( -- __0) - inst(WITH_EXCEPT_START) { + inst(WITH_EXCEPT_START, (exit_func, lasti, unused, val -- exit_func, lasti, unused, val, res)) { /* At the top of the stack are 4 values: - TOP = exc_info() - SECOND = previous exception @@ -2734,23 +2733,17 @@ dummy_func( We call FOURTH(type(TOP), TOP, GetTraceback(TOP)). Then we push the __exit__ return value. */ - PyObject *exit_func; - PyObject *exc, *val, *tb, *res; + PyObject *exc, *tb; - val = TOP(); assert(val && PyExceptionInstance_Check(val)); exc = PyExceptionInstance_Class(val); tb = PyException_GetTraceback(val); Py_XDECREF(tb); - assert(PyLong_Check(PEEK(3))); - exit_func = PEEK(4); + assert(PyLong_Check(lasti)); PyObject *stack[4] = {NULL, exc, val, tb}; res = PyObject_Vectorcall(exit_func, stack + 1, 3 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL); - if (res == NULL) - goto error; - - PUSH(res); + ERROR_IF(res == NULL, error); } // stack effect: ( -- __0) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index f5639c4e13d11f..58e4d16091dadb 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2745,6 +2745,10 @@ } TARGET(WITH_EXCEPT_START) { + PyObject *val = PEEK(1); + PyObject *lasti = PEEK(3); + PyObject *exit_func = PEEK(4); + PyObject *res; /* At the top of the stack are 4 values: - TOP = exc_info() - SECOND = previous exception @@ -2753,23 +2757,19 @@ We call FOURTH(type(TOP), TOP, GetTraceback(TOP)). Then we push the __exit__ return value. */ - PyObject *exit_func; - PyObject *exc, *val, *tb, *res; + PyObject *exc, *tb; - val = TOP(); assert(val && PyExceptionInstance_Check(val)); exc = PyExceptionInstance_Class(val); tb = PyException_GetTraceback(val); Py_XDECREF(tb); - assert(PyLong_Check(PEEK(3))); - exit_func = PEEK(4); + assert(PyLong_Check(lasti)); PyObject *stack[4] = {NULL, exc, val, tb}; res = PyObject_Vectorcall(exit_func, stack + 1, 3 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL); - if (res == NULL) - goto error; - - PUSH(res); + if (res == NULL) goto pop_4_error; + STACK_GROW(1); + POKE(1, res); DISPATCH(); } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 2aefeb42ce39c8..907ad6720d00a8 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -82,13 +82,14 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: assert cache_offset == self.cache_offset # Write input stack effect variable declarations and initializations + input_names = [seffect.name for seffect in self.input_effects] for i, seffect in enumerate(reversed(self.input_effects), 1): if seffect.name != "unused": f.write(f"{indent} PyObject *{seffect.name} = PEEK({i});\n") # Write output stack effect variable declarations for seffect in self.output_effects: - if seffect.name != "unused": + if seffect.name not in input_names and seffect.name != "unused": f.write(f"{indent} PyObject *{seffect.name};\n") self.write_body(f, indent, dedent) @@ -105,8 +106,8 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: f.write(f"{indent} STACK_SHRINK({-diff});\n") # Write output stack effect assignments - input_names = [seffect.name for seffect in self.input_effects] for i, output in enumerate(reversed(self.output_effects), 1): + # TODO: Only skip if output occurs at same position as input if output.name not in input_names and output.name != "unused": f.write(f"{indent} POKE({i}, {output.name});\n") From cb0c874062c68de8fb64abdf37e242e65f253cc3 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 16 Nov 2022 15:39:40 -0800 Subject: [PATCH 07/19] Fix lexer to balk at unrecognized characters, e.g. '@' --- Tools/cases_generator/lexer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/cases_generator/lexer.py b/Tools/cases_generator/lexer.py index 493a32e38166d7..980c920bf357f4 100644 --- a/Tools/cases_generator/lexer.py +++ b/Tools/cases_generator/lexer.py @@ -112,7 +112,8 @@ def choice(*opts): COMMENT = 'COMMENT' newline = r"\n" -matcher = re.compile(choice(id_re, number_re, str_re, char, newline, macro, comment_re, *operators.values())) +invalid = r"\S" # A single non-space character that's not caught by any of the other patterns +matcher = re.compile(choice(id_re, number_re, str_re, char, newline, macro, comment_re, *operators.values(), invalid)) letter = re.compile(r'[a-zA-Z_]') kwds = ( @@ -177,7 +178,6 @@ def __repr__(self): def tokenize(src, line=1, filename=None): linestart = -1 - # TODO: finditer() skips over unrecognized characters, e.g. '@' for m in matcher.finditer(src): start, end = m.span() text = m.group(0) From fe0d336697bd22c64aa996e7cb69ab0bd0face70 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 17 Nov 2022 14:57:05 -0800 Subject: [PATCH 08/19] 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 907ad6720d00a8..a3a61260e2d20c 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -118,7 +118,7 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: def write_body(self, f: typing.TextIO, ndent: str, dedent: int) -> None: """Write the instruction body.""" - # Get lines of text with proper dedelt + # Get lines of text with proper dedent blocklines = self.block.to_text(dedent=dedent).splitlines(True) # Remove blank lines from both ends From d84a5c364f977a1d1aa8412d4256a63b30c0b7ab Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 17 Nov 2022 16:28:52 -0800 Subject: [PATCH 09/19] Code review from GH-99526 --- Tools/cases_generator/generate_cases.py | 14 ++++++++------ Tools/cases_generator/parser.py | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index a3a61260e2d20c..4662b5b9ffe56b 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -19,6 +19,8 @@ BEGIN_MARKER = "// BEGIN BYTECODES //" END_MARKER = "// END BYTECODES //" RE_PREDICTED = r"(?s)(?:PREDICT\(|GO_TO_INSTRUCTION\(|DEOPT_IF\(.*?,\s*)(\w+)\);" +UNUSED = "unused" +BITS_PER_CODE_UNIT = 16 arg_parser = argparse.ArgumentParser() arg_parser.add_argument("-i", "--input", type=str, default=DEFAULT_INPUT) @@ -70,9 +72,9 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: # Write cache effect variable declarations cache_offset = 0 for ceffect in self.cache_effects: - if ceffect.name != "unused": + if ceffect.name != UNUSED: # TODO: if name is 'descr' use PyObject *descr = read_obj(...) - bits = ceffect.size * 16 + bits = ceffect.size * BITS_PER_CODE_UNIT f.write(f"{indent} uint{bits}_t {ceffect.name} = ") if ceffect.size == 1: f.write(f"*(next_instr + {cache_offset});\n") @@ -84,12 +86,12 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: # Write input stack effect variable declarations and initializations input_names = [seffect.name for seffect in self.input_effects] for i, seffect in enumerate(reversed(self.input_effects), 1): - if seffect.name != "unused": + if seffect.name != UNUSED: f.write(f"{indent} PyObject *{seffect.name} = PEEK({i});\n") # Write output stack effect variable declarations for seffect in self.output_effects: - if seffect.name not in input_names and seffect.name != "unused": + if seffect.name not in input_names and seffect.name != UNUSED: f.write(f"{indent} PyObject *{seffect.name};\n") self.write_body(f, indent, dedent) @@ -108,7 +110,7 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: # Write output stack effect assignments for i, output in enumerate(reversed(self.output_effects), 1): # TODO: Only skip if output occurs at same position as input - if output.name not in input_names and output.name != "unused": + if output.name not in input_names and output.name != UNUSED: f.write(f"{indent} POKE({i}, {output.name});\n") # Write cache effect @@ -223,7 +225,7 @@ class Analyzer: filename: str src: str - errors: int = 0 + errors: int = 0 # TODO: add a method to print an error message def __init__(self, filename: str): """Read the input file.""" diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index 9727439afdf58c..ae5ef1e26ea1c2 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -99,7 +99,7 @@ def inputs(self) -> list[InputEffect]: return self.header.inputs @property - def outputs(self) -> list[StackEffect]: + def outputs(self) -> list[OutputEffect]: return self.header.outputs From d7ad95004375a146c050e2dfbb28ea1452d7f0f9 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 17 Nov 2022 17:26:12 -0800 Subject: [PATCH 10/19] Fix moved output names; support object pointers in cache --- Tools/cases_generator/generate_cases.py | 36 +++++++++++++++++-------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 4662b5b9ffe56b..72941ec2dd037d 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -73,25 +73,36 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: cache_offset = 0 for ceffect in self.cache_effects: if ceffect.name != UNUSED: - # TODO: if name is 'descr' use PyObject *descr = read_obj(...) bits = ceffect.size * BITS_PER_CODE_UNIT - f.write(f"{indent} uint{bits}_t {ceffect.name} = ") - if ceffect.size == 1: - f.write(f"*(next_instr + {cache_offset});\n") + if bits == 64: + # NOTE: We assume that 64-bit data in the cache + # is always an object pointer. + # If this becomes false, we need a way to specify + # syntactically what type the cache data is. + f.write( + f"{indent} PyObject *{ceffect.name} = " + f"read_obj(next_instr + {cache_offset});\n" + ) else: - f.write(f"read_u{bits}(next_instr + {cache_offset});\n") + f.write(f"{indent} uint{bits}_t {ceffect.name} = ") + if ceffect.size == 1: + # There is no read_u16() helper function. + f.write(f"*(next_instr + {cache_offset});\n") + else: + f.write(f"read_u{bits}(next_instr + {cache_offset});\n") cache_offset += ceffect.size assert cache_offset == self.cache_offset # Write input stack effect variable declarations and initializations - input_names = [seffect.name for seffect in self.input_effects] for i, seffect in enumerate(reversed(self.input_effects), 1): if seffect.name != UNUSED: f.write(f"{indent} PyObject *{seffect.name} = PEEK({i});\n") # Write output stack effect variable declarations + input_names = {seffect.name for seffect in self.input_effects} + input_names.add(UNUSED) for seffect in self.output_effects: - if seffect.name not in input_names and seffect.name != UNUSED: + if seffect.name not in input_names: f.write(f"{indent} PyObject *{seffect.name};\n") self.write_body(f, indent, dedent) @@ -108,10 +119,13 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: f.write(f"{indent} STACK_SHRINK({-diff});\n") # Write output stack effect assignments - for i, output in enumerate(reversed(self.output_effects), 1): - # TODO: Only skip if output occurs at same position as input - if output.name not in input_names and output.name != UNUSED: - f.write(f"{indent} POKE({i}, {output.name});\n") + unmoved_names = {UNUSED} + for ieffect, oeffect in zip(self.input_effects, self.output_effects): + if ieffect.name == oeffect.name: + unmoved_names.add(ieffect.name) + for i, seffect in enumerate(reversed(self.output_effects)): + if seffect.name not in unmoved_names: + f.write(f"{indent} POKE({i+1}, {seffect.name});\n") # Write cache effect if self.cache_offset: From a0346757f42695ae8c6715fe3d8290a95d26c2f6 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 17 Nov 2022 17:47:34 -0800 Subject: [PATCH 11/19] Tune README --- Tools/cases_generator/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/cases_generator/README.md b/Tools/cases_generator/README.md index abcafe257720eb..dc055ead1941cd 100644 --- a/Tools/cases_generator/README.md +++ b/Tools/cases_generator/README.md @@ -2,9 +2,9 @@ What's currently here: -- lexer.py: lexer for C, originally written by Mark Shannon -- plexer.py: OO interface on top of lexer.py; main class: `PLexer` -- parser.py: Parser for instruction definition DSL; main class `Parser` +- `lexer.py`: lexer for C, originally written by Mark Shannon +- `plexer.py`: OO interface on top of lexer.py; main class: `PLexer` +- `parser.py`: Parser for instruction definition DSL; main class `Parser` - `generate_cases.py`: driver script to read `Python/bytecodes.c` and write `Python/generated_cases.c.h` From 1aafac8c3bd72a051aa77ece1c544388a8525591 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 17 Nov 2022 18:41:23 -0800 Subject: [PATCH 12/19] Introduce error() method to print errors --- Tools/cases_generator/generate_cases.py | 45 +++++++++++++------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 72941ec2dd037d..38cff314b7a75f 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -218,11 +218,10 @@ def super_macro_analysis( lowest = current = highest = 0 for instr in components: if instr.cache_effects: - print( + a.error( f"Super-instruction {self.name!r} has cache effects in {instr.name!r}", - file=sys.stderr, + instr, ) - a.errors += 1 current -= len(instr.input_effects) lowest = min(lowest, current) current += len(instr.output_effects) @@ -239,7 +238,18 @@ class Analyzer: filename: str src: str - errors: int = 0 # TODO: add a method to print an error message + errors: int = 0 + + def error(self, msg: str, node: parser.Node) -> None: + lineno = 0 + if context := node.context: + # Use line number of first non-comment in the node + for token in context.owner.tokens[context.begin : context.end]: + lineno = token.line + if token.kind != "COMMENT": + break + print(f"{self.filename}:{lineno}: {msg}", file=sys.stderr) + self.errors += 1 def __init__(self, filename: str): """Read the input file.""" @@ -303,11 +313,10 @@ def find_predictions(self) -> None: if target_instr := self.instrs.get(target): target_instr.predicted = True else: - print( + self.error( f"Unknown instruction {target!r} predicted in {instr.name!r}", - file=sys.stderr, + instr, # TODO: Use better location ) - self.errors += 1 def map_families(self) -> None: """Make instruction names back to their family, if they have one.""" @@ -316,11 +325,10 @@ def map_families(self) -> None: if member_instr := self.instrs.get(member): member_instr.family = family else: - print( + self.error( f"Unknown instruction {member!r} referenced in family {family.name!r}", - file=sys.stderr, + family, ) - self.errors += 1 def check_families(self) -> None: """Check each family: @@ -331,13 +339,11 @@ def check_families(self) -> None: """ for family in self.families.values(): if len(family.members) < 2: - print(f"Family {family.name!r} has insufficient members") - self.errors += 1 + self.error(f"Family {family.name!r} has insufficient members", family) members = [member for member in family.members if member in self.instrs] if members != family.members: unknown = set(family.members) - set(members) - print(f"Family {family.name!r} has unknown members: {unknown}") - self.errors += 1 + self.error(f"Family {family.name!r} has unknown members: {unknown}", family) if len(members) < 2: continue head = self.instrs[members[0]] @@ -350,18 +356,13 @@ def check_families(self) -> None: i = len(instr.input_effects) o = len(instr.output_effects) if (c, i, o) != (cache, input, output): - self.errors += 1 - print( + self.error( f"Family {family.name!r} has inconsistent " - f"(cache, inputs, outputs) effects:", - file=sys.stderr, - ) - print( + f"(cache, inputs, outputs) effects:\n" f" {family.members[0]} = {(cache, input, output)}; " f"{member} = {(c, i, o)}", - file=sys.stderr, + family, ) - self.errors += 1 def analyze_supers(self) -> None: """Analyze each super instruction.""" From 20062f431af22d91927087209188eaaf36f12927 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 17 Nov 2022 19:01:01 -0800 Subject: [PATCH 13/19] Check components of super/macro ops --- Tools/cases_generator/generate_cases.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 38cff314b7a75f..8549a79db3ec99 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -188,7 +188,7 @@ def __init__(self, sup: parser.Super): self.context = sup.context def analyze(self, a: "Analyzer") -> None: - components = [a.instrs[name] for name in self.ops] + components = self.check_components(a) self.stack, self.initial_sp = self.super_macro_analysis(a, components) sp = self.initial_sp self.parts = [] @@ -206,6 +206,20 @@ def analyze(self, a: "Analyzer") -> None: self.parts.append(SuperComponent(instr, input_mapping, output_mapping)) self.final_sp = sp + def check_components(self, a: "Analyzer") -> list[Instruction]: + components: list[Instruction] = [] + if not self.ops: + a.error(f"{self.kind.capitalize()}-instruction has no operands", self) + for name in self.ops: + if name not in a.instrs: + a.error(f"Unknown instruction {name!r}", self) + else: + instr = a.instrs[name] + if self.kind == "super" and instr.kind != "inst": + a.error(f"Super-instruction operand {instr.name} must be inst, not op", instr) + components.append(instr) + return components + def super_macro_analysis( self, a: "Analyzer", components: list[Instruction] ) -> tuple[list[str], int]: From 7194723b96beb05f966a6ba6a0eadea0a095bf9f Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 17 Nov 2022 19:22:48 -0800 Subject: [PATCH 14/19] Fix crash when WITH_EXCEPT_START errors out This was popping the common items off the stack. --- Python/bytecodes.c | 9 +++++---- Python/generated_cases.c.h | 10 +++++----- Tools/cases_generator/generate_cases.py | 7 +++++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 9fcbebde1447f0..de8bbf5d93a160 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -82,6 +82,7 @@ do { \ static PyObject *value, *value1, *value2, *left, *right, *res, *sum, *prod, *sub; static PyObject *container, *start, *stop, *v, *lhs, *rhs; static PyObject *list, *tuple, *dict; +static PyObject *exit_func, *lasti, *val; static PyObject * dummy_func( @@ -2726,10 +2727,10 @@ dummy_func( inst(WITH_EXCEPT_START, (exit_func, lasti, unused, val -- exit_func, lasti, unused, val, res)) { /* At the top of the stack are 4 values: - - TOP = exc_info() - - SECOND = previous exception - - THIRD: lasti of exception in exc_info() - - FOURTH: the context.__exit__ bound method + - val: TOP = exc_info() + - unused: SECOND = previous exception + - lasti: THIRD = lasti of exception in exc_info() + - exit_func: FOURTH = the context.__exit__ bound method We call FOURTH(type(TOP), TOP, GetTraceback(TOP)). Then we push the __exit__ return value. */ diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 58e4d16091dadb..37f18df56dffc4 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2750,10 +2750,10 @@ PyObject *exit_func = PEEK(4); PyObject *res; /* At the top of the stack are 4 values: - - TOP = exc_info() - - SECOND = previous exception - - THIRD: lasti of exception in exc_info() - - FOURTH: the context.__exit__ bound method + - val: TOP = exc_info() + - unused: SECOND = previous exception + - lasti: THIRD = lasti of exception in exc_info() + - exit_func: FOURTH = the context.__exit__ bound method We call FOURTH(type(TOP), TOP, GetTraceback(TOP)). Then we push the __exit__ return value. */ @@ -2767,7 +2767,7 @@ PyObject *stack[4] = {NULL, exc, val, tb}; res = PyObject_Vectorcall(exit_func, stack + 1, 3 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL); - if (res == NULL) goto pop_4_error; + if (res == NULL) goto error; STACK_GROW(1); POKE(1, res); DISPATCH(); diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 8549a79db3ec99..1b8370bd7ad081 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -161,6 +161,13 @@ def write_body(self, f: typing.TextIO, ndent: str, dedent: int) -> None: # The code block is responsible for DECREF()ing them. # NOTE: If the label doesn't exist, just add it to ceval.c. 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 if ninputs: f.write(f"{space}if ({cond}) goto pop_{ninputs}_{label};\n") else: From b74aa6aaac304d256ca66ca14de896d2b4aecbae Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 22 Nov 2022 13:27:23 -0800 Subject: [PATCH 15/19] Don't use typing.Dict I blame Copilot. :-) Co-authored-by: Brandt Bucher --- Tools/cases_generator/generate_cases.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 1b8370bd7ad081..891790a63825dc 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -179,8 +179,8 @@ def write_body(self, f: typing.TextIO, ndent: str, dedent: int) -> None: @dataclasses.dataclass class SuperComponent: instr: Instruction - input_mapping: typing.Dict[str, parser.StackEffect] - output_mapping: typing.Dict[str, parser.StackEffect] + input_mapping: dict[str, parser.StackEffect] + output_mapping: dict[str, parser.StackEffect] class SuperInstruction(parser.Super): From a0feff9402678b7b8c1203ea95f2613f0262747b Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 22 Nov 2022 13:28:32 -0800 Subject: [PATCH 16/19] Kill more "unused" literals I thought I got them all... Co-authored-by: Brandt Bucher --- Tools/cases_generator/generate_cases.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 891790a63825dc..efd4d95ec8de6b 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -203,11 +203,11 @@ def analyze(self, a: "Analyzer") -> None: input_mapping = {} for ieffect in reversed(instr.input_effects): sp -= 1 - if ieffect.name != "unused": + if ieffect.name != UNUSED: input_mapping[self.stack[sp]] = ieffect output_mapping = {} for oeffect in instr.output_effects: - if oeffect.name != "unused": + if oeffect.name != UNUSED: output_mapping[self.stack[sp]] = oeffect sp += 1 self.parts.append(SuperComponent(instr, input_mapping, output_mapping)) From a2e999143554941a779c64e9568016cfd1c973ce Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 22 Nov 2022 13:29:58 -0800 Subject: [PATCH 17/19] Don't over-use f-strings Co-authored-by: Brandt Bucher --- Tools/cases_generator/generate_cases.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index efd4d95ec8de6b..7f0539bc8b063c 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -465,8 +465,8 @@ def block(head: str): for i, comp in enumerate(sup.parts): if i > 0 and sup.kind == "super": - write(f"NEXTOPARG();") - write(f"next_instr++;") + write("NEXTOPARG();") + write("next_instr++;") with block(""): for var, ieffect in comp.input_mapping.items(): @@ -483,7 +483,7 @@ def block(head: str): write(f"STACK_SHRINK({sup.initial_sp - sup.final_sp});") for i, var in enumerate(reversed(sup.stack[:sup.final_sp]), 1): write(f"POKE({i}, {var});") - write(f"DISPATCH();") + write("DISPATCH();") def always_exits(block: parser.Block) -> bool: From 240126b0985ab75b4ada76be7dde61c4d620dbeb Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 22 Nov 2022 13:32:12 -0800 Subject: [PATCH 18/19] Avoid compiler warning on unused variable --- Python/bytecodes.c | 1 + Python/generated_cases.c.h | 1 + 2 files changed, 2 insertions(+) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index de8bbf5d93a160..a1f910da8ed54a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2741,6 +2741,7 @@ dummy_func( tb = PyException_GetTraceback(val); Py_XDECREF(tb); assert(PyLong_Check(lasti)); + (void)lasti; // Shut up compiler warning if asserts are off PyObject *stack[4] = {NULL, exc, val, tb}; res = PyObject_Vectorcall(exit_func, stack + 1, 3 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 37f18df56dffc4..f285b3df90a64e 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2764,6 +2764,7 @@ tb = PyException_GetTraceback(val); Py_XDECREF(tb); assert(PyLong_Check(lasti)); + (void)lasti; // Shut up compiler warning if asserts are off PyObject *stack[4] = {NULL, exc, val, tb}; res = PyObject_Vectorcall(exit_func, stack + 1, 3 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL); From 7afa58c3b343d34ba4ad64c1357f106ec21b78e3 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 22 Nov 2022 13:43:28 -0800 Subject: [PATCH 19/19] Introduce read_uint16(p) as equivalent to *p This simplifies a few lines in the code generator. --- Include/internal/pycore_code.h | 6 ++++++ Python/generated_cases.c.h | 2 +- Tools/cases_generator/generate_cases.py | 8 ++------ 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index ba36ee38d2b0ba..80c1bfb6c9afa2 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -293,6 +293,12 @@ write_obj(uint16_t *p, PyObject *val) memcpy(p, &val, sizeof(val)); } +static inline uint16_t +read_u16(uint16_t *p) +{ + return *p; +} + static inline uint32_t read_u32(uint16_t *p) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index f285b3df90a64e..ae8fdd5e99c3dc 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -437,7 +437,7 @@ TARGET(BINARY_SUBSCR_GETITEM) { uint32_t type_version = read_u32(next_instr + 1); - uint16_t func_version = *(next_instr + 3); + uint16_t func_version = read_u16(next_instr + 3); PyObject *sub = PEEK(1); PyObject *container = PEEK(2); PyTypeObject *tp = Py_TYPE(container); diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 7f0539bc8b063c..424b15ede2aadf 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -84,12 +84,8 @@ def write(self, f: typing.TextIO, indent: str, dedent: int = 0) -> None: f"read_obj(next_instr + {cache_offset});\n" ) else: - f.write(f"{indent} uint{bits}_t {ceffect.name} = ") - if ceffect.size == 1: - # There is no read_u16() helper function. - f.write(f"*(next_instr + {cache_offset});\n") - else: - f.write(f"read_u{bits}(next_instr + {cache_offset});\n") + f.write(f"{indent} uint{bits}_t {ceffect.name} = " + f"read_u{bits}(next_instr + {cache_offset});\n") cache_offset += ceffect.size assert cache_offset == self.cache_offset