From b7f3135b7a85660776d5938ebc5d7b513591f3e5 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sat, 28 Jan 2023 13:21:50 -0800 Subject: [PATCH 1/3] Allow macros as family members --- Tools/cases_generator/generate_cases.py | 42 ++++++++++++++----- Tools/cases_generator/test_generator.py | 55 +++++++++++++++++-------- 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 9d894d2ff57455..e3e3d697e133f7 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -400,10 +400,12 @@ class Component: def write_body(self, out: Formatter, cache_adjust: int) -> None: with out.block(""): + input_names = {ieffect.name for _, ieffect in self.input_mapping} for var, ieffect in self.input_mapping: out.declare(ieffect, var) for _, oeffect in self.output_mapping: - out.declare(oeffect, None) + if oeffect.name not in input_names: + out.declare(oeffect, None) self.instr.write_body(out, dedent=-4, cache_adjust=cache_adjust) @@ -537,10 +539,10 @@ def analyze(self) -> None: Raises SystemExit if there is an error. """ self.find_predictions() - self.map_families() - self.check_families() self.analyze_register_instrs() self.analyze_supers_and_macros() + self.map_families() + self.check_families() def find_predictions(self) -> None: """Find the instructions that need PREDICTED() labels.""" @@ -559,11 +561,15 @@ def find_predictions(self) -> None: ) def map_families(self) -> None: - """Make instruction names back to their family, if they have one.""" + """Link instruction names back to their family, if they have one.""" for family in self.families.values(): for member in family.members: if member_instr := self.instrs.get(member): member_instr.family = family + elif member_macro := self.macro_instrs.get(member): + for part in member_macro.parts: + if isinstance(part, Component): + part.instr.family = family else: self.error( f"Unknown instruction {member!r} referenced in family {family.name!r}", @@ -580,7 +586,7 @@ def check_families(self) -> None: for family in self.families.values(): if len(family.members) < 2: self.error(f"Family {family.name!r} has insufficient members", family) - members = [member for member in family.members if member in self.instrs] + members = [member for member in family.members if member in self.instrs or member in self.macro_instrs] if members != family.members: unknown = set(family.members) - set(members) self.error( @@ -588,15 +594,31 @@ def check_families(self) -> None: ) if len(members) < 2: continue - head = self.instrs[members[0]] + head = self.instrs[members[0]] # Head must be a regular instruction cache = head.cache_offset input = len(head.input_effects) output = len(head.output_effects) for member in members[1:]: - instr = self.instrs[member] - c = instr.cache_offset - i = len(instr.input_effects) - o = len(instr.output_effects) + if instr := self.instrs.get(member): + c = instr.cache_offset + i = len(instr.input_effects) + o = len(instr.output_effects) + else: + macro = self.macro_instrs[member] + c, i, o = 0, 0, 0 + for part in macro.parts: + if isinstance(part, Component): + c += part.instr.cache_offset + # A component may pop what the previous component pushed, + # so we offset the input/output counts by that. + delta_i = len(part.instr.input_effects) + delta_o = len(part.instr.output_effects) + offset = min(delta_i, o) + i += delta_i - offset + o += delta_o - offset + else: + assert isinstance(part, parser.CacheEffect), part + c += part.size if (c, i, o) != (cache, input, output): self.error( f"Family {family.name!r} has inconsistent " diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index bd1b974399abdd..8f8ba24620ede8 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -333,21 +333,24 @@ def test_super_instruction(): def test_macro_instruction(): input = """ - inst(OP1, (counter/1, arg1 -- interim)) { - interim = op1(arg1); + inst(OP1, (counter/1, left, right -- left, right)) { + op1(left, right); } - op(OP2, (extra/2, arg2, interim -- res)) { - res = op2(arg2, interim); + op(OP2, (extra/2, arg2, left, right -- res)) { + res = op2(arg2, left, right); } macro(OP) = OP1 + cache/2 + OP2; + inst(OP3, (unused/5, arg2, left, right -- res)) { + res = op3(arg2, left, right); + } + family(op3, INLINE_CACHE_ENTRIES_OP) = { OP3, OP }; """ output = """ TARGET(OP1) { - PyObject *arg1 = PEEK(1); - PyObject *interim; + PyObject *right = PEEK(1); + PyObject *left = PEEK(2); uint16_t counter = read_u16(&next_instr[0].cache); - interim = op1(arg1); - POKE(1, interim); + op1(left, right); JUMPBY(1); DISPATCH(); } @@ -355,24 +358,40 @@ def test_macro_instruction(): TARGET(OP) { PyObject *_tmp_1 = PEEK(1); PyObject *_tmp_2 = PEEK(2); + PyObject *_tmp_3 = PEEK(3); { - PyObject *arg1 = _tmp_1; - PyObject *interim; + PyObject *right = _tmp_1; + PyObject *left = _tmp_2; uint16_t counter = read_u16(&next_instr[0].cache); - interim = op1(arg1); - _tmp_1 = interim; + op1(left, right); + _tmp_2 = left; + _tmp_1 = right; } { - PyObject *interim = _tmp_1; - PyObject *arg2 = _tmp_2; + PyObject *right = _tmp_1; + PyObject *left = _tmp_2; + PyObject *arg2 = _tmp_3; PyObject *res; uint32_t extra = read_u32(&next_instr[3].cache); - res = op2(arg2, interim); - _tmp_2 = res; + res = op2(arg2, left, right); + _tmp_3 = res; } JUMPBY(5); - STACK_SHRINK(1); - POKE(1, _tmp_2); + STACK_SHRINK(2); + POKE(1, _tmp_3); + DISPATCH(); + } + + TARGET(OP3) { + static_assert(INLINE_CACHE_ENTRIES_OP == 5, "incorrect cache size"); + PyObject *right = PEEK(1); + PyObject *left = PEEK(2); + PyObject *arg2 = PEEK(3); + PyObject *res; + res = op3(arg2, left, right); + STACK_SHRINK(2); + POKE(1, res); + JUMPBY(5); DISPATCH(); } """ From f10e4edf02b3c21021df1c7dfad6172a9cbce126 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sun, 29 Jan 2023 08:33:21 -0800 Subject: [PATCH 2/3] Check for instructions straddling families This includes macropmarts. --- Tools/cases_generator/generate_cases.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index e3e3d697e133f7..0387f75db554f2 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -565,11 +565,26 @@ def map_families(self) -> None: for family in self.families.values(): for member in family.members: if member_instr := self.instrs.get(member): - member_instr.family = family + if member_instr.family not in (family, None): + self.error( + f"Instruction {member} is a member of multiple families " + f"({member_instr.family.name}, {family.name}).", + family + ) + else: + member_instr.family = family elif member_macro := self.macro_instrs.get(member): for part in member_macro.parts: if isinstance(part, Component): - part.instr.family = family + if part.instr.family not in (family, None): + self.error( + f"Component {part.instr.name} of macro {member} " + f"is a member of multiple families " + f"({part.instr.family.name}, {family.name}).", + family + ) + else: + part.instr.family = family else: self.error( f"Unknown instruction {member!r} referenced in family {family.name!r}", From 9989acaf7f408c275e3a7440b5a28cf45dfc767e Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sun, 29 Jan 2023 17:52:51 -0800 Subject: [PATCH 3/3] Refactor effect counts to allow macro family head --- Tools/cases_generator/generate_cases.py | 58 +++++++++++++------------ Tools/cases_generator/test_generator.py | 3 +- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 0387f75db554f2..15a2af4441053a 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -609,40 +609,42 @@ def check_families(self) -> None: ) if len(members) < 2: continue - head = self.instrs[members[0]] # Head must be a regular instruction - cache = head.cache_offset - input = len(head.input_effects) - output = len(head.output_effects) + expected_effects = self.effect_counts(members[0]) for member in members[1:]: - if instr := self.instrs.get(member): - c = instr.cache_offset - i = len(instr.input_effects) - o = len(instr.output_effects) - else: - macro = self.macro_instrs[member] - c, i, o = 0, 0, 0 - for part in macro.parts: - if isinstance(part, Component): - c += part.instr.cache_offset - # A component may pop what the previous component pushed, - # so we offset the input/output counts by that. - delta_i = len(part.instr.input_effects) - delta_o = len(part.instr.output_effects) - offset = min(delta_i, o) - i += delta_i - offset - o += delta_o - offset - else: - assert isinstance(part, parser.CacheEffect), part - c += part.size - if (c, i, o) != (cache, input, output): + member_effects = self.effect_counts(member) + if member_effects != expected_effects: self.error( f"Family {family.name!r} has inconsistent " - f"(cache, inputs, outputs) effects:\n" - f" {family.members[0]} = {(cache, input, output)}; " - f"{member} = {(c, i, o)}", + f"(cache, input, output) effects:\n" + f" {family.members[0]} = {expected_effects}; " + f"{member} = {member_effects}", family, ) + def effect_counts(self, name: str) -> tuple[int, int, int]: + if instr := self.instrs.get(name): + cache = instr.cache_offset + input = len(instr.input_effects) + output = len(instr.output_effects) + elif macro := self.macro_instrs.get(name): + cache, input, output = 0, 0, 0 + for part in macro.parts: + if isinstance(part, Component): + cache += part.instr.cache_offset + # A component may pop what the previous component pushed, + # so we offset the input/output counts by that. + delta_i = len(part.instr.input_effects) + delta_o = len(part.instr.output_effects) + offset = min(delta_i, output) + input += delta_i - offset + output += delta_o - offset + else: + assert isinstance(part, parser.CacheEffect), part + cache += part.size + else: + assert False, f"Unknown instruction {name!r}" + return cache, input, output + def analyze_register_instrs(self) -> None: for instr in self.instrs.values(): if instr.register: diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index 8f8ba24620ede8..d6880079f08392 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -343,7 +343,7 @@ def test_macro_instruction(): inst(OP3, (unused/5, arg2, left, right -- res)) { res = op3(arg2, left, right); } - family(op3, INLINE_CACHE_ENTRIES_OP) = { OP3, OP }; + family(op, INLINE_CACHE_ENTRIES_OP) = { OP, OP3 }; """ output = """ TARGET(OP1) { @@ -383,7 +383,6 @@ def test_macro_instruction(): } TARGET(OP3) { - static_assert(INLINE_CACHE_ENTRIES_OP == 5, "incorrect cache size"); PyObject *right = PEEK(1); PyObject *left = PEEK(2); PyObject *arg2 = PEEK(3);