From ec8c701fb9bfa2c7b9d10dd3a975d575ffd187a4 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 17:11:15 +0200 Subject: [PATCH 01/33] Add INSTRUCTION_SIZE macro --- Tools/cases_generator/generators_common.py | 7 +++++++ Tools/cases_generator/lexer.py | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 7e032c21d2485c..e337661feec259 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -1,3 +1,4 @@ +import dataclasses from pathlib import Path from typing import TextIO @@ -490,12 +491,18 @@ def _emit_block( if reachable: reachable = if_reachable self.out.emit(rbrace) + elif tkn.kind == "INSTRUCTION_SIZE": + self._emit_instruction_size(tkn, inst) else: self.out.emit(tkn) except StackError as ex: raise analysis_error(ex.args[0], tkn) from None raise analysis_error("Expecting closing brace. Reached end of file", tkn) + def _emit_instruction_size(self, tkn: Token, inst: Instruction) -> None: + """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" + params = dataclasses.asdict(tkn) | {"text": str(inst.size)} + self.out.emit(Token(**params)) def emit_tokens( self, diff --git a/Tools/cases_generator/lexer.py b/Tools/cases_generator/lexer.py index 37f96398ff175f..819fd8ab1616d1 100644 --- a/Tools/cases_generator/lexer.py +++ b/Tools/cases_generator/lexer.py @@ -228,6 +228,9 @@ def choice(*opts: str) -> str: "tier2", } +# An instruction size in the DSL +INSTRUCTION_SIZE = "INSTRUCTION_SIZE" + __all__ = [] __all__.extend(kwds) @@ -292,6 +295,8 @@ def tokenize(src: str, line: int = 1, filename: str = "") -> Iterator[Token]: kind = keywords[text] elif text in annotations: kind = ANNOTATION + elif text == INSTRUCTION_SIZE: + kind = INSTRUCTION_SIZE elif letter.match(text): kind = IDENTIFIER elif text == "...": From 09c2f59382f8a1f716ef45278a7aa65922cbb03c Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 17:11:45 +0200 Subject: [PATCH 02/33] Use INSTRUCTION_SIZE in bytecodes.c --- Python/bytecodes.c | 24 ++++++++++++------------ Python/generated_cases.c.h | 38 +++++++++++++++++++------------------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index b22916aeaa248b..374f12689eda1d 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -841,7 +841,7 @@ dummy_func( new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; INPUTS_DEAD(); - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_BINARY_SUBSCR); + frame->return_offset = INSTRUCTION_SIZE; } macro(BINARY_SUBSCR_GETITEM) = @@ -1101,8 +1101,8 @@ dummy_func( gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(next_instr - this_instr + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(next_instr - this_instr + oparg); + assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); assert(gen_frame->previous == NULL); gen_frame->previous = frame; DISPATCH_INLINED(gen_frame); @@ -1147,8 +1147,8 @@ dummy_func( gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(1 + INLINE_CACHE_ENTRIES_SEND + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_SEND + oparg); + assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); gen_frame->previous = frame; } @@ -2255,7 +2255,7 @@ dummy_func( new_frame->localsplus[0] = owner; DEAD(owner); new_frame->localsplus[1] = PyStackRef_FromPyObjectNew(name); - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = INSTRUCTION_SIZE; DISPATCH_INLINED(new_frame); } @@ -3053,7 +3053,7 @@ dummy_func( tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; // oparg is the return offset from the next instruction. - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_FOR_ITER + oparg); + frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); } macro(FOR_ITER_GEN) = @@ -3332,7 +3332,7 @@ dummy_func( if (new_frame == NULL) { ERROR_NO_POP(); } - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = INSTRUCTION_SIZE; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -4194,8 +4194,8 @@ dummy_func( if (new_frame == NULL) { ERROR_NO_POP(); } - assert(next_instr - this_instr == 1 + INLINE_CACHE_ENTRIES_CALL_KW); - frame->return_offset = 1 + INLINE_CACHE_ENTRIES_CALL_KW; + assert(INSTRUCTION_SIZE == 1 + INLINE_CACHE_ENTRIES_CALL_KW); + frame->return_offset = INSTRUCTION_SIZE; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -4461,7 +4461,7 @@ dummy_func( if (new_frame == NULL) { ERROR_NO_POP(); } - assert(next_instr - this_instr == 1); + assert(INSTRUCTION_SIZE == 1); frame->return_offset = 1; DISPATCH_INLINED(new_frame); } @@ -4794,7 +4794,7 @@ dummy_func( op(_SAVE_RETURN_OFFSET, (--)) { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = INSTRUCTION_SIZE; #endif #if TIER_TWO frame->return_offset = oparg; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 7bd1b7dd5aba27..99e618191b4e8e 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -537,7 +537,7 @@ new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_BINARY_SUBSCR); + frame->return_offset = 2; } // _PUSH_FRAME { @@ -933,7 +933,7 @@ if (new_frame == NULL) { goto error; } - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -1183,7 +1183,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; #endif #if TIER_TWO frame->return_offset = oparg; @@ -1284,7 +1284,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; #endif #if TIER_TWO frame->return_offset = oparg; @@ -1735,7 +1735,7 @@ if (new_frame == NULL) { goto error; } - assert(next_instr - this_instr == 1); + assert(1 == 1); frame->return_offset = 1; DISPATCH_INLINED(new_frame); } @@ -1956,8 +1956,8 @@ if (new_frame == NULL) { goto error; } - assert(next_instr - this_instr == 1 + INLINE_CACHE_ENTRIES_CALL_KW); - frame->return_offset = 1 + INLINE_CACHE_ENTRIES_CALL_KW; + assert(4 == 1 + INLINE_CACHE_ENTRIES_CALL_KW); + frame->return_offset = 4; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -2104,7 +2104,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; #endif #if TIER_TWO frame->return_offset = oparg; @@ -2282,7 +2282,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; #endif #if TIER_TWO frame->return_offset = oparg; @@ -2849,7 +2849,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; #endif #if TIER_TWO frame->return_offset = oparg; @@ -2929,7 +2929,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; #endif #if TIER_TWO frame->return_offset = oparg; @@ -3982,7 +3982,7 @@ tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; // oparg is the return offset from the next instruction. - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_FOR_ITER + oparg); + frame->return_offset = (uint16_t)(2 + oparg); } // _PUSH_FRAME { @@ -4444,7 +4444,7 @@ if (new_frame == NULL) { goto error; } - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 4; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -5401,7 +5401,7 @@ STACK_SHRINK(1); new_frame->localsplus[0] = owner; new_frame->localsplus[1] = PyStackRef_FromPyObjectNew(name); - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 10; DISPATCH_INLINED(new_frame); } @@ -5732,7 +5732,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 10; #endif #if TIER_TWO frame->return_offset = oparg; @@ -7090,8 +7090,8 @@ gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(next_instr - this_instr + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(next_instr - this_instr + oparg); + assert(2 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)(2 + oparg); assert(gen_frame->previous == NULL); gen_frame->previous = frame; DISPATCH_INLINED(gen_frame); @@ -7163,8 +7163,8 @@ gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(1 + INLINE_CACHE_ENTRIES_SEND + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_SEND + oparg); + assert(2 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)(2 + oparg); gen_frame->previous = frame; } // _PUSH_FRAME From 287eb9279f0faed602ca652c6701d4a575fc4da0 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 17:11:56 +0200 Subject: [PATCH 03/33] Add tests --- Lib/test/test_generated_cases.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index cd3718b80612bd..42b89b5e10742e 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -1371,6 +1371,24 @@ def test_stack_save_only(self): with self.assertRaises(SyntaxError): self.run_cases_test(input, output) + def test_instruction_size_macro(self): + input = """ + inst(OP, (--)) { + frame->return_offset = INSTRUCTION_SIZE; + } + """ + + output = """ + TARGET(OP) { + frame->instr_ptr = next_instr; + next_instr += 1; + INSTRUCTION_STATS(OP); + frame->return_offset = 1; + DISPATCH(); + } + """ + self.run_cases_test(input, output) + class TestGeneratedAbstractCases(unittest.TestCase): def setUp(self) -> None: From c76fce40386bc5d6902bf8cfab8eaf15dbaf2463 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 17:12:08 +0200 Subject: [PATCH 04/33] Update docs --- Tools/cases_generator/interpreter_definition.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Tools/cases_generator/interpreter_definition.md b/Tools/cases_generator/interpreter_definition.md index ba09931c541646..b8174052a299fb 100644 --- a/Tools/cases_generator/interpreter_definition.md +++ b/Tools/cases_generator/interpreter_definition.md @@ -178,15 +178,17 @@ list of annotations and their meanings are as follows: ### Special functions/macros -The C code may include special functions that are understood by the tools as +The C code may include special functions and macros that are understood by the tools as part of the DSL. -Those functions include: +Those include: * `DEOPT_IF(cond, instruction)`. Deoptimize if `cond` is met. * `ERROR_IF(cond, label)`. Jump to error handler at `label` if `cond` is true. * `DECREF_INPUTS()`. Generate `Py_DECREF()` calls for the input stack effects. * `SYNC_SP()`. Synchronizes the physical stack pointer with the stack effects. +* `INSTRUCTION_SIZE`. Replaced with the size of the instruction which is equal +to `1 + INLINE_CACHE_ENTRIES`. Note that the use of `DECREF_INPUTS()` is optional -- manual calls to `Py_DECREF()` or other approaches are also acceptable From e3f3b07d9bcf92de1fff222705e340aaa72a2141 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 17:13:18 +0200 Subject: [PATCH 05/33] Add news entry --- .../2024-10-14-17-13-12.gh-issue-118423.SkBoda.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-17-13-12.gh-issue-118423.SkBoda.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-17-13-12.gh-issue-118423.SkBoda.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-17-13-12.gh-issue-118423.SkBoda.rst new file mode 100644 index 00000000000000..8511a8de5530d6 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-17-13-12.gh-issue-118423.SkBoda.rst @@ -0,0 +1,2 @@ +Add a new ``INSTRUCTION_SIZE`` macro to the cases generator which returns +the current instruction size. From ed5b3753e4b64a47d62229240d86f27274e55283 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 17:21:12 +0200 Subject: [PATCH 06/33] Fix mypy error --- Tools/cases_generator/generators_common.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index e337661feec259..23939ce70edfd4 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -499,8 +499,9 @@ def _emit_block( raise analysis_error(ex.args[0], tkn) from None raise analysis_error("Expecting closing brace. Reached end of file", tkn) - def _emit_instruction_size(self, tkn: Token, inst: Instruction) -> None: + def _emit_instruction_size(self, tkn: Token, inst: Instruction | None) -> None: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" + assert inst is not None, "INSTRUCTION_SIZE requires instruction to be passed" params = dataclasses.asdict(tkn) | {"text": str(inst.size)} self.out.emit(Token(**params)) From 870341d2a155a482245321b386e124495bcc5e48 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 20:54:37 +0200 Subject: [PATCH 07/33] Treat INSTRUCTION_SIZE as an identifier --- Tools/cases_generator/generators_common.py | 24 ++++++++++++++-------- Tools/cases_generator/lexer.py | 5 ----- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 23939ce70edfd4..3057ade1c5da5c 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -119,7 +119,8 @@ def __init__(self, out: CWriter): "PyStackRef_CLOSE": self.stackref_close, "PyStackRef_CLOSE_SPECIALIZED": self.stackref_close, "PyStackRef_AsPyObjectSteal": self.stackref_steal, - "DISPATCH": self.dispatch + "DISPATCH": self.dispatch, + "INSTRUCTION_SIZE": self.instruction_size, } self.out = out @@ -358,6 +359,19 @@ def reload_stack( self.emit_reload(storage) return True + def instruction_size(self, + tkn: Token, + tkn_iter: TokenIterator, + uop: Uop, + storage: Storage, + inst: Instruction | None + ) -> bool: + """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" + assert inst is not None, "INSTRUCTION_SIZE requires instruction to be passed" + params = dataclasses.asdict(tkn) | {"text": str(inst.size)} + self.out.emit(Token(**params)) + return True + def _print_storage(self, storage: Storage) -> None: if PRINT_STACKS: self.out.start_line() @@ -491,20 +505,12 @@ def _emit_block( if reachable: reachable = if_reachable self.out.emit(rbrace) - elif tkn.kind == "INSTRUCTION_SIZE": - self._emit_instruction_size(tkn, inst) else: self.out.emit(tkn) except StackError as ex: raise analysis_error(ex.args[0], tkn) from None raise analysis_error("Expecting closing brace. Reached end of file", tkn) - def _emit_instruction_size(self, tkn: Token, inst: Instruction | None) -> None: - """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" - assert inst is not None, "INSTRUCTION_SIZE requires instruction to be passed" - params = dataclasses.asdict(tkn) | {"text": str(inst.size)} - self.out.emit(Token(**params)) - def emit_tokens( self, uop: Uop, diff --git a/Tools/cases_generator/lexer.py b/Tools/cases_generator/lexer.py index 819fd8ab1616d1..37f96398ff175f 100644 --- a/Tools/cases_generator/lexer.py +++ b/Tools/cases_generator/lexer.py @@ -228,9 +228,6 @@ def choice(*opts: str) -> str: "tier2", } -# An instruction size in the DSL -INSTRUCTION_SIZE = "INSTRUCTION_SIZE" - __all__ = [] __all__.extend(kwds) @@ -295,8 +292,6 @@ def tokenize(src: str, line: int = 1, filename: str = "") -> Iterator[Token]: kind = keywords[text] elif text in annotations: kind = ANNOTATION - elif text == INSTRUCTION_SIZE: - kind = INSTRUCTION_SIZE elif letter.match(text): kind = IDENTIFIER elif text == "...": From 421ebda6b7f726013abb5b0d560d72db84d5b643 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 21:11:40 +0200 Subject: [PATCH 08/33] Regenerate executor cases --- Python/executor_cases.c.h | 12 ++++++------ Tools/cases_generator/generators_common.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 0ed361a2ee7fb0..53c46524a978da 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1069,7 +1069,7 @@ new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_BINARY_SUBSCR); + frame->return_offset = 1; stack_pointer[-2].bits = (uintptr_t)new_frame; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); @@ -1376,8 +1376,8 @@ gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(1 + INLINE_CACHE_ENTRIES_SEND + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_SEND + oparg); + assert(1 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)(1 + oparg); gen_frame->previous = frame; stack_pointer[-1].bits = (uintptr_t)gen_frame; break; @@ -2748,7 +2748,7 @@ break; } - /* _LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN is not a viable micro-op for tier 2 because it uses the 'this_instr' variable */ + /* _LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN is not a viable micro-op for tier 2 because it has unused cache entries */ case _GUARD_DORV_NO_DICT: { _PyStackRef owner; @@ -3564,7 +3564,7 @@ tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; // oparg is the return offset from the next instruction. - frame->return_offset = (uint16_t)(1 + INLINE_CACHE_ENTRIES_FOR_ITER + oparg); + frame->return_offset = (uint16_t)(1 + oparg); stack_pointer[0].bits = (uintptr_t)gen_frame; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -5589,7 +5589,7 @@ case _SAVE_RETURN_OFFSET: { oparg = CURRENT_OPARG(); #if TIER_ONE - frame->return_offset = (uint16_t)(next_instr - this_instr); + frame->return_offset = 1; #endif #if TIER_TWO frame->return_offset = oparg; diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 3057ade1c5da5c..536bb694eeb008 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -367,8 +367,8 @@ def instruction_size(self, inst: Instruction | None ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" - assert inst is not None, "INSTRUCTION_SIZE requires instruction to be passed" - params = dataclasses.asdict(tkn) | {"text": str(inst.size)} + size = inst.size if inst else 1 + uop.size + params = dataclasses.asdict(tkn) | {"text": str(size)} self.out.emit(Token(**params)) return True From 5154f7f41f23940e64d940437cea3f109084d54b Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 21:19:19 +0200 Subject: [PATCH 09/33] Re-add newline --- Tools/cases_generator/generators_common.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 536bb694eeb008..5758bf3dbf4dd6 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -511,6 +511,7 @@ def _emit_block( raise analysis_error(ex.args[0], tkn) from None raise analysis_error("Expecting closing brace. Reached end of file", tkn) + def emit_tokens( self, uop: Uop, From e7e83dce3b0e021358e01d07ea34399c3962c8ed Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 14 Oct 2024 21:38:30 +0200 Subject: [PATCH 10/33] Use replaceText() --- Tools/cases_generator/generators_common.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 5758bf3dbf4dd6..0088993db19253 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -368,8 +368,7 @@ def instruction_size(self, ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" size = inst.size if inst else 1 + uop.size - params = dataclasses.asdict(tkn) | {"text": str(size)} - self.out.emit(Token(**params)) + self.out.emit(tkn.replaceText(str(size))) return True def _print_storage(self, storage: Storage) -> None: From 101a5fdde7ef082cb24a80dd71103192d220d2ad Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 15 Oct 2024 22:16:27 +0200 Subject: [PATCH 11/33] Emit size directly --- Python/executor_cases.c.h | 10 +++--- Python/generated_cases.c.h | 38 +++++++++++----------- Tools/cases_generator/generators_common.py | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 53c46524a978da..282952b7e4bdbc 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1069,7 +1069,7 @@ new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; - frame->return_offset = 1; + frame->return_offset = 1 ; stack_pointer[-2].bits = (uintptr_t)new_frame; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); @@ -1376,8 +1376,8 @@ gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(1 + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(1 + oparg); + assert( 1 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)( 1 + oparg); gen_frame->previous = frame; stack_pointer[-1].bits = (uintptr_t)gen_frame; break; @@ -3564,7 +3564,7 @@ tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; // oparg is the return offset from the next instruction. - frame->return_offset = (uint16_t)(1 + oparg); + frame->return_offset = (uint16_t)( 1 + oparg); stack_pointer[0].bits = (uintptr_t)gen_frame; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -5589,7 +5589,7 @@ case _SAVE_RETURN_OFFSET: { oparg = CURRENT_OPARG(); #if TIER_ONE - frame->return_offset = 1; + frame->return_offset = 1 ; #endif #if TIER_TWO frame->return_offset = oparg; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 99e618191b4e8e..55e87697d3e4e6 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -537,7 +537,7 @@ new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; - frame->return_offset = 2; + frame->return_offset = 2 ; } // _PUSH_FRAME { @@ -933,7 +933,7 @@ if (new_frame == NULL) { goto error; } - frame->return_offset = 4; + frame->return_offset = 4 ; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -1183,7 +1183,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4; + frame->return_offset = 4 ; #endif #if TIER_TWO frame->return_offset = oparg; @@ -1284,7 +1284,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4; + frame->return_offset = 4 ; #endif #if TIER_TWO frame->return_offset = oparg; @@ -1735,7 +1735,7 @@ if (new_frame == NULL) { goto error; } - assert(1 == 1); + assert( 1 == 1); frame->return_offset = 1; DISPATCH_INLINED(new_frame); } @@ -1956,8 +1956,8 @@ if (new_frame == NULL) { goto error; } - assert(4 == 1 + INLINE_CACHE_ENTRIES_CALL_KW); - frame->return_offset = 4; + assert( 4 == 1 + INLINE_CACHE_ENTRIES_CALL_KW); + frame->return_offset = 4 ; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -2104,7 +2104,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4; + frame->return_offset = 4 ; #endif #if TIER_TWO frame->return_offset = oparg; @@ -2282,7 +2282,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4; + frame->return_offset = 4 ; #endif #if TIER_TWO frame->return_offset = oparg; @@ -2849,7 +2849,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4; + frame->return_offset = 4 ; #endif #if TIER_TWO frame->return_offset = oparg; @@ -2929,7 +2929,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4; + frame->return_offset = 4 ; #endif #if TIER_TWO frame->return_offset = oparg; @@ -3982,7 +3982,7 @@ tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; // oparg is the return offset from the next instruction. - frame->return_offset = (uint16_t)(2 + oparg); + frame->return_offset = (uint16_t)( 2 + oparg); } // _PUSH_FRAME { @@ -4444,7 +4444,7 @@ if (new_frame == NULL) { goto error; } - frame->return_offset = 4; + frame->return_offset = 4 ; DISPATCH_INLINED(new_frame); } /* Callable is not a normal Python function */ @@ -5401,7 +5401,7 @@ STACK_SHRINK(1); new_frame->localsplus[0] = owner; new_frame->localsplus[1] = PyStackRef_FromPyObjectNew(name); - frame->return_offset = 10; + frame->return_offset = 10 ; DISPATCH_INLINED(new_frame); } @@ -5732,7 +5732,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 10; + frame->return_offset = 10 ; #endif #if TIER_TWO frame->return_offset = oparg; @@ -7090,8 +7090,8 @@ gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(2 + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(2 + oparg); + assert( 2 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)( 2 + oparg); assert(gen_frame->previous == NULL); gen_frame->previous = frame; DISPATCH_INLINED(gen_frame); @@ -7163,8 +7163,8 @@ gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert(2 + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(2 + oparg); + assert( 2 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)( 2 + oparg); gen_frame->previous = frame; } // _PUSH_FRAME diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 0088993db19253..e51f1be593a6b0 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -368,7 +368,7 @@ def instruction_size(self, ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" size = inst.size if inst else 1 + uop.size - self.out.emit(tkn.replaceText(str(size))) + self.out.emit(f" {size} ") return True def _print_storage(self, storage: Storage) -> None: From ee461d5b23f3d22bce200cdf5898af96d5b8f930 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Wed, 16 Oct 2024 21:11:13 +0200 Subject: [PATCH 12/33] Pass instruction in tier2 --- Tools/cases_generator/tier2_generator.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 634848c10309d5..9bc8a4f33d42e4 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -167,7 +167,7 @@ def oparg( return True -def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> Stack: +def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst: Instruction) -> Stack: locals: dict[str, Local] = {} try: emitter.out.start_line() @@ -188,11 +188,18 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> Stack: type = f"uint{cache.size*16}_t " cast = f"uint{cache.size*16}_t" emitter.emit(f"{type}{cache.name} = ({cast})CURRENT_OPERAND();\n") - storage = emitter.emit_tokens(uop, storage, None) + storage = emitter.emit_tokens(uop, storage, inst) except StackError as ex: raise analysis_error(ex.args[0], uop.body[0]) from None return storage.stack + +def get_instruction_for_uop(instructions: dict[str, Instruction], uop: Uop) -> Instruction | None: + for inst in instructions.values(): + if uop in inst.parts: + return inst + + SKIPS = ("_EXTENDED_ARG",) @@ -227,8 +234,9 @@ def generate_tier2( continue out.emit(f"case {uop.name}: {{\n") declare_variables(uop, out) + inst = get_instruction_for_uop(analysis.instructions, uop) stack = Stack() - stack = write_uop(uop, emitter, stack) + stack = write_uop(uop, emitter, stack, inst) out.start_line() if not uop.properties.always_exits: stack.flush(out) From 7135103360147b08de5b609b064f156f282033ee Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Wed, 16 Oct 2024 21:12:41 +0200 Subject: [PATCH 13/33] Fix INSTRUCTION_SIZE in tier2 --- Python/executor_cases.c.h | 10 +++++----- Tools/cases_generator/generators_common.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 775f78dd67f6b3..42d7a580543e14 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1071,7 +1071,7 @@ new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; - frame->return_offset = 1 ; + frame->return_offset = 2 ; stack_pointer[-2].bits = (uintptr_t)new_frame; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); @@ -1378,8 +1378,8 @@ gen->gi_frame_state = FRAME_EXECUTING; gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; - assert( 1 + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)( 1 + oparg); + assert( 2 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)( 2 + oparg); gen_frame->previous = frame; stack_pointer[-1].bits = (uintptr_t)gen_frame; break; @@ -3566,7 +3566,7 @@ tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; // oparg is the return offset from the next instruction. - frame->return_offset = (uint16_t)( 1 + oparg); + frame->return_offset = (uint16_t)( 2 + oparg); stack_pointer[0].bits = (uintptr_t)gen_frame; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); @@ -5591,7 +5591,7 @@ case _SAVE_RETURN_OFFSET: { oparg = CURRENT_OPARG(); #if TIER_ONE - frame->return_offset = 1 ; + frame->return_offset = 10 ; #endif #if TIER_TWO frame->return_offset = oparg; diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index e51f1be593a6b0..b77fabbe7accfe 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -367,8 +367,8 @@ def instruction_size(self, inst: Instruction | None ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" - size = inst.size if inst else 1 + uop.size - self.out.emit(f" {size} ") + assert inst is not None, "INSTRUCTION_SIZE macro requires instruction" + self.out.emit(f" {inst.size} ") return True def _print_storage(self, storage: Storage) -> None: From 715ce31689d295b71afff901e748299e942fce23 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Wed, 16 Oct 2024 21:19:47 +0200 Subject: [PATCH 14/33] Make mypy happy --- Tools/cases_generator/tier2_generator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 9bc8a4f33d42e4..77cf189a10268f 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -167,7 +167,7 @@ def oparg( return True -def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst: Instruction) -> Stack: +def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst: Instruction | None) -> Stack: locals: dict[str, Local] = {} try: emitter.out.start_line() @@ -198,6 +198,7 @@ def get_instruction_for_uop(instructions: dict[str, Instruction], uop: Uop) -> I for inst in instructions.values(): if uop in inst.parts: return inst + return None SKIPS = ("_EXTENDED_ARG",) From 823532180e47dd6446c5cd416aa668ad5dbba3ea Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Thu, 17 Oct 2024 00:25:02 +0200 Subject: [PATCH 15/33] Fix tests --- Lib/test/test_generated_cases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index 42b89b5e10742e..1084e1bd3080f3 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -1383,7 +1383,7 @@ def test_instruction_size_macro(self): frame->instr_ptr = next_instr; next_instr += 1; INSTRUCTION_STATS(OP); - frame->return_offset = 1; + frame->return_offset = 1 ; DISPATCH(); } """ From 148070e75af34256d7a1b0b1d00a2a00550276dc Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Thu, 17 Oct 2024 21:14:20 +0200 Subject: [PATCH 16/33] Remove unused imports --- Tools/cases_generator/generators_common.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index b77fabbe7accfe..461766dfd31bd9 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -1,4 +1,3 @@ -import dataclasses from pathlib import Path from typing import TextIO @@ -10,9 +9,9 @@ analysis_error, ) from cwriter import CWriter -from typing import Callable, Mapping, TextIO, Iterator, Iterable +from typing import Callable, TextIO, Iterator, Iterable from lexer import Token -from stack import Stack, Local, Storage, StackError +from stack import Storage, StackError # Set this to true for voluminous output showing state of stack and locals PRINT_STACKS = False From 3e6b5926f683e82ffa5faae4c84b71ff04eb00ec Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Thu, 17 Oct 2024 23:15:21 +0200 Subject: [PATCH 17/33] Ensure all instructions have the same size --- Python/bytecodes.c | 2 +- Python/executor_cases.c.h | 2 +- Tools/cases_generator/generators_common.py | 34 +++++++++++++++++----- Tools/cases_generator/tier2_generator.py | 33 ++++++++++++++++----- 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 6301bfb613f970..48168315580bb4 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -4796,7 +4796,7 @@ dummy_func( op(_SAVE_RETURN_OFFSET, (--)) { #if TIER_ONE - frame->return_offset = INSTRUCTION_SIZE; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 42d7a580543e14..6b2d9d79f5528d 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -5591,7 +5591,7 @@ case _SAVE_RETURN_OFFSET: { oparg = CURRENT_OPARG(); #if TIER_ONE - frame->return_offset = 10 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 461766dfd31bd9..dc95746e65b691 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -90,7 +90,7 @@ def emit_to(out: CWriter, tkn_iter: TokenIterator, end: str) -> Token: ReplacementFunctionType = Callable[ - [Token, TokenIterator, Uop, Storage, Instruction | None], bool + [Token, TokenIterator, Uop, Storage, Instruction | None, int | None], bool ] def always_true(tkn: Token | None) -> bool: @@ -130,6 +130,7 @@ def dispatch( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: self.emit(tkn) return False @@ -141,6 +142,7 @@ def deopt_if( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: self.out.emit_at("DEOPT_IF", tkn) lparen = next(tkn_iter) @@ -165,6 +167,7 @@ def error_if( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -206,6 +209,7 @@ def error_no_pop( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) # LPAREN next(tkn_iter) # RPAREN @@ -220,6 +224,7 @@ def decref_inputs( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -253,6 +258,7 @@ def kill_inputs( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -268,6 +274,7 @@ def kill( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) name_tkn = next(tkn_iter) @@ -289,6 +296,7 @@ def stackref_close( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: self.out.emit(tkn) tkn = next(tkn_iter) @@ -313,6 +321,7 @@ def sync_sp( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -333,6 +342,7 @@ def save_stack( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -351,6 +361,7 @@ def reload_stack( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -363,11 +374,15 @@ def instruction_size(self, tkn_iter: TokenIterator, uop: Uop, storage: Storage, - inst: Instruction | None + inst: Instruction | None, + inst_size: int | None = None, ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" - assert inst is not None, "INSTRUCTION_SIZE macro requires instruction" - self.out.emit(f" {inst.size} ") + assert inst or inst_size is not None, "INSTRUCTION_SIZE macro requires instruction or size" + if inst: + self.out.emit(f" {inst.size} ") + elif inst_size is not None: + self.out.emit(f" {inst_size} ") return True def _print_storage(self, storage: Storage) -> None: @@ -382,6 +397,7 @@ def _emit_if( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> tuple[bool, Token, Storage]: """Returns (reachable?, closing '}', stack).""" tkn = next(tkn_iter) @@ -390,7 +406,7 @@ def _emit_if( rparen = emit_to(self.out, tkn_iter, "RPAREN") self.emit(rparen) if_storage = storage.copy() - reachable, rbrace, if_storage = self._emit_block(tkn_iter, uop, if_storage, inst, True) + reachable, rbrace, if_storage = self._emit_block(tkn_iter, uop, if_storage, inst, inst_size, True) try: maybe_else = tkn_iter.peek() if maybe_else and maybe_else.kind == "ELSE": @@ -406,7 +422,7 @@ def _emit_if( self.out.start_line() self.emit("}\n") else: - else_reachable, rbrace, else_storage = self._emit_block(tkn_iter, uop, storage, inst, True) + else_reachable, rbrace, else_storage = self._emit_block(tkn_iter, uop, storage, inst, inst_size, True) if not reachable: # Discard the if storage reachable = else_reachable @@ -442,6 +458,7 @@ def _emit_block( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None, emit_first_brace: bool ) -> tuple[bool, Token, Storage]: """ Returns (reachable?, closing '}', stack).""" @@ -484,7 +501,7 @@ def _emit_block( self.out.emit(tkn) elif tkn.kind == "IDENTIFIER": if tkn.text in self._replacers: - if not self._replacers[tkn.text](tkn, tkn_iter, uop, storage, inst): + if not self._replacers[tkn.text](tkn, tkn_iter, uop, storage, inst, inst_size): reachable = False else: if tkn in out_stores: @@ -515,10 +532,11 @@ def emit_tokens( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None ) -> Storage: tkn_iter = TokenIterator(uop.body) self.out.start_line() - _, rbrace, storage = self._emit_block(tkn_iter, uop, storage, inst, False) + _, rbrace, storage = self._emit_block(tkn_iter, uop, storage, inst, inst_size, False) try: self._print_storage(storage) storage.push_outputs() diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 77cf189a10268f..1cd529dfebc5ba 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -75,6 +75,7 @@ def error_if( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -97,6 +98,7 @@ def error_no_pop( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: next(tkn_iter) # LPAREN next(tkn_iter) # RPAREN @@ -111,6 +113,7 @@ def deopt_if( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -132,6 +135,7 @@ def exit_if( # type: ignore[override] uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -152,6 +156,7 @@ def oparg( uop: Uop, storage: Storage, inst: Instruction | None, + inst_size: int | None = None, ) -> bool: if not uop.name.endswith("_0") and not uop.name.endswith("_1"): self.emit(tkn) @@ -167,7 +172,7 @@ def oparg( return True -def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst: Instruction | None) -> Stack: +def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst_size: int | None = None) -> Stack: locals: dict[str, Local] = {} try: emitter.out.start_line() @@ -188,17 +193,29 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst: Instruction | None type = f"uint{cache.size*16}_t " cast = f"uint{cache.size*16}_t" emitter.emit(f"{type}{cache.name} = ({cast})CURRENT_OPERAND();\n") - storage = emitter.emit_tokens(uop, storage, inst) + storage = emitter.emit_tokens(uop, storage, None, inst_size) except StackError as ex: raise analysis_error(ex.args[0], uop.body[0]) from None return storage.stack -def get_instruction_for_uop(instructions: dict[str, Instruction], uop: Uop) -> Instruction | None: +def contains_instruction_size_macro(uop: Uop) -> bool: + for token in uop.body: + if token.kind == "IDENTIFIER" and token.text in 'INSTRUCTION_SIZE': + return True + return False + + +def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int: + size = None for inst in instructions.values(): if uop in inst.parts: - return inst - return None + if size is None: + size = inst.size + elif size != inst.size: + assert size == inst.size, f"All instructions must have the same size: {size} != {inst.size}" + assert size is not None + return size SKIPS = ("_EXTENDED_ARG",) @@ -235,9 +252,11 @@ def generate_tier2( continue out.emit(f"case {uop.name}: {{\n") declare_variables(uop, out) - inst = get_instruction_for_uop(analysis.instructions, uop) + inst_size = None + if contains_instruction_size_macro(uop): + inst_size = get_instruction_size_for_uop(analysis.instructions, uop) stack = Stack() - stack = write_uop(uop, emitter, stack, inst) + stack = write_uop(uop, emitter, stack, inst_size) out.start_line() if not uop.properties.always_exits: stack.flush(out) From 6e78ec0377dc7717440bbc43656b5536f1315b48 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Thu, 17 Oct 2024 23:15:49 +0200 Subject: [PATCH 18/33] Add tests --- Lib/test/test_generated_cases.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index 1084e1bd3080f3..252c7a714ae084 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -1390,6 +1390,34 @@ def test_instruction_size_macro(self): self.run_cases_test(input, output) + input = """ + inst(OP, (--)) { + frame->return_offset = INSTRUCTION_SIZE; + } + macro(OP2) = unused/1 + OP; + """ + + output = """ + TARGET(OP) { + frame->instr_ptr = next_instr; + next_instr += 1; + INSTRUCTION_STATS(OP); + frame->return_offset = 1 ; + DISPATCH(); + } + + TARGET(OP2) { + frame->instr_ptr = next_instr; + next_instr += 2; + INSTRUCTION_STATS(OP2); + /* Skip 1 cache entry */ + frame->return_offset = 2 ; + DISPATCH(); + } + """ + self.run_cases_test(input, output) + + class TestGeneratedAbstractCases(unittest.TestCase): def setUp(self) -> None: super().setUp() From 75e6f6c82db31e26626ad1f271321c5109b5dd6e Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Thu, 17 Oct 2024 23:24:52 +0200 Subject: [PATCH 19/33] Regen files --- Python/generated_cases.c.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 404cf02c83d028..56867e99f48661 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1185,7 +1185,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; @@ -1286,7 +1286,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; @@ -2106,7 +2106,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; @@ -2284,7 +2284,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; @@ -2851,7 +2851,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; @@ -2931,7 +2931,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 4 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; @@ -5734,7 +5734,7 @@ // _SAVE_RETURN_OFFSET { #if TIER_ONE - frame->return_offset = 10 ; + frame->return_offset = (uint16_t)(next_instr - this_instr); #endif #if TIER_TWO frame->return_offset = oparg; From a3c0d10a1c7b504818df8517177a81bd57423061 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Thu, 17 Oct 2024 23:25:09 +0200 Subject: [PATCH 20/33] Make mypy happy --- Tools/cases_generator/tier2_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 1cd529dfebc5ba..81574c13876ac7 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -212,7 +212,7 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) if uop in inst.parts: if size is None: size = inst.size - elif size != inst.size: + if size != inst.size: assert size == inst.size, f"All instructions must have the same size: {size} != {inst.size}" assert size is not None return size From 7235d2df47e9afd18f1e5933c0099524cd5a0ddc Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 20 Oct 2024 17:15:10 +0200 Subject: [PATCH 21/33] Fix tests --- Lib/test/test_generated_cases.py | 6 ++-- Tools/cases_generator/generators_common.py | 32 ++++++++++++++++++++++ Tools/cases_generator/tier1_generator.py | 5 +++- Tools/cases_generator/tier2_generator.py | 21 ++------------ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index 252c7a714ae084..dbf83f9031f5f1 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -1389,7 +1389,8 @@ def test_instruction_size_macro(self): """ self.run_cases_test(input, output) - + # Two instructions of different sizes referencing the same + # uop containing the `INSTRUCTION_SIZE` macro is not allowed. input = """ inst(OP, (--)) { frame->return_offset = INSTRUCTION_SIZE; @@ -1415,7 +1416,8 @@ def test_instruction_size_macro(self): DISPATCH(); } """ - self.run_cases_test(input, output) + with self.assertRaisesRegex(AssertionError, "All instructions containing a uop"): + self.run_cases_test(input, output) class TestGeneratedAbstractCases(unittest.TestCase): diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index dc95746e65b691..253a7788fbeb6f 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -583,3 +583,35 @@ def cflags(p: Properties) -> str: return " | ".join(flags) else: return "0" + + +def contains_instruction_size_macro(uop: Uop) -> bool: + """Return True if the uop contains the INSTRUCTION_SIZE macro.""" + for token in uop.body: + if token.kind == "IDENTIFIER" and token.text in 'INSTRUCTION_SIZE': + return True + return False + + +def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int: + """Return the size of the instruction that contains the given uop. + + If there is more than one instruction that contains the uop, + ensure that they all have the same size. + """ + size = None + for inst in instructions.values(): + if uop in inst.parts: + if size is None: + size = inst.size + if size != inst.size: + assert size == inst.size, ( + "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " + f"must have the same size: {size} != {inst.size}" + ) + assert size is not None + return size + + +def assert_same_instruction_size(instructions: dict[str, Instruction], uop: Uop) -> None: + get_instruction_size_for_uop(instructions, uop) diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py index 8dadc5736c8889..f25237bfd09381 100644 --- a/Tools/cases_generator/tier1_generator.py +++ b/Tools/cases_generator/tier1_generator.py @@ -22,7 +22,8 @@ write_header, type_and_null, Emitter, - TokenIterator, + contains_instruction_size_macro, + assert_same_instruction_size, ) from cwriter import CWriter from typing import TextIO @@ -172,6 +173,8 @@ def generate_tier1( offset = 1 # The instruction itself stack = Stack() for part in inst.parts: + if isinstance(part, Uop) and contains_instruction_size_macro(part): + assert_same_instruction_size(analysis.instructions, part) # Only emit braces if more than one uop insert_braces = len([p for p in inst.parts if isinstance(p, Uop)]) > 1 offset, stack = write_uop(part, emitter, offset, stack, inst, insert_braces) diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 81574c13876ac7..84c77e521bfe6a 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -22,6 +22,8 @@ Emitter, TokenIterator, always_true, + contains_instruction_size_macro, + get_instruction_size_for_uop, ) from cwriter import CWriter from typing import TextIO, Iterator @@ -199,25 +201,6 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst_size: int | None = return storage.stack -def contains_instruction_size_macro(uop: Uop) -> bool: - for token in uop.body: - if token.kind == "IDENTIFIER" and token.text in 'INSTRUCTION_SIZE': - return True - return False - - -def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int: - size = None - for inst in instructions.values(): - if uop in inst.parts: - if size is None: - size = inst.size - if size != inst.size: - assert size == inst.size, f"All instructions must have the same size: {size} != {inst.size}" - assert size is not None - return size - - SKIPS = ("_EXTENDED_ARG",) From 5cfb4236a826a6172e54763fb6f93ae2107f75ef Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 21 Oct 2024 22:32:51 +0200 Subject: [PATCH 22/33] Raise instead of assert when size is None --- Tools/cases_generator/generators_common.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 253a7788fbeb6f..be5bd09c67cfa2 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -609,7 +609,9 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " f"must have the same size: {size} != {inst.size}" ) - assert size is not None + if size is None: + msg = f"No instruction containing the uop '{uop.name}' was found" + raise ValueError(msg) return size From 21907848b693c6bf7104c8378ffa08099a4b2ef8 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 21 Oct 2024 23:17:40 +0200 Subject: [PATCH 23/33] Add an `instruction_size` field to the Uop class --- Tools/cases_generator/analyzer.py | 35 ++++++++++++ Tools/cases_generator/generators_common.py | 66 +++------------------- Tools/cases_generator/tier1_generator.py | 4 -- Tools/cases_generator/tier2_generator.py | 16 +----- 4 files changed, 45 insertions(+), 76 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 381ad3a4e2082c..8db4eb41cc197f 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -173,6 +173,8 @@ class Uop: implicitly_created: bool = False replicated = 0 replicates: "Uop | None" = None + # Size of the instruction(s), only set for uops containing the INSTRUCTION_SIZE macro + instruction_size: int | None = None def dump(self, indent: str) -> None: print( @@ -1079,6 +1081,35 @@ def add_instruction(name: str) -> None: return instmap, len(no_arg), min_instrumented +def contains_instruction_size_macro(uop: Uop) -> bool: + """Return True if the uop contains the INSTRUCTION_SIZE macro.""" + for token in uop.body: + if token.kind == "IDENTIFIER" and token.text in 'INSTRUCTION_SIZE': + return True + return False + + +def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int: + """Return the size of the instruction that contains the given uop. + + If there is more than one instruction that contains the uop, + ensure that they all have the same size. + """ + size = None + for inst in instructions.values(): + if uop in inst.parts: + if size is None: + size = inst.size + assert size == inst.size, ( + "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " + f"must have the same size: {size} != {inst.size}" + ) + if size is None: + msg = f"No instruction containing the uop '{uop.name}' was found" + raise ValueError(msg) + return size + + def analyze_forest(forest: list[parser.AstNode]) -> Analysis: instructions: dict[str, Instruction] = {} uops: dict[str, Uop] = {} @@ -1122,6 +1153,10 @@ def analyze_forest(forest: list[parser.AstNode]) -> Analysis: continue if target.text in instructions: instructions[target.text].is_target = True + for uop in uops.values(): + if not contains_instruction_size_macro(uop): + continue + uop.instruction_size = get_instruction_size_for_uop(instructions, uop) # Special case BINARY_OP_INPLACE_ADD_UNICODE # BINARY_OP_INPLACE_ADD_UNICODE is not a normal family member, # as it is the wrong size, but we need it to maintain an diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index be5bd09c67cfa2..659794c1eeb799 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -90,7 +90,7 @@ def emit_to(out: CWriter, tkn_iter: TokenIterator, end: str) -> Token: ReplacementFunctionType = Callable[ - [Token, TokenIterator, Uop, Storage, Instruction | None, int | None], bool + [Token, TokenIterator, Uop, Storage, Instruction | None], bool ] def always_true(tkn: Token | None) -> bool: @@ -130,7 +130,6 @@ def dispatch( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: self.emit(tkn) return False @@ -142,7 +141,6 @@ def deopt_if( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: self.out.emit_at("DEOPT_IF", tkn) lparen = next(tkn_iter) @@ -167,7 +165,6 @@ def error_if( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -209,7 +206,6 @@ def error_no_pop( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) # LPAREN next(tkn_iter) # RPAREN @@ -224,7 +220,6 @@ def decref_inputs( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -258,7 +253,6 @@ def kill_inputs( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -274,7 +268,6 @@ def kill( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) name_tkn = next(tkn_iter) @@ -296,7 +289,6 @@ def stackref_close( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: self.out.emit(tkn) tkn = next(tkn_iter) @@ -321,7 +313,6 @@ def sync_sp( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -342,7 +333,6 @@ def save_stack( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -361,7 +351,6 @@ def reload_stack( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) next(tkn_iter) @@ -375,14 +364,10 @@ def instruction_size(self, uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" - assert inst or inst_size is not None, "INSTRUCTION_SIZE macro requires instruction or size" - if inst: - self.out.emit(f" {inst.size} ") - elif inst_size is not None: - self.out.emit(f" {inst_size} ") + assert uop.instruction_size is not None, "INSTRUCTION_SIZE macro requires uop.instruction_size to be set" + self.out.emit(f" {uop.instruction_size} ") return True def _print_storage(self, storage: Storage) -> None: @@ -397,7 +382,6 @@ def _emit_if( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> tuple[bool, Token, Storage]: """Returns (reachable?, closing '}', stack).""" tkn = next(tkn_iter) @@ -406,7 +390,7 @@ def _emit_if( rparen = emit_to(self.out, tkn_iter, "RPAREN") self.emit(rparen) if_storage = storage.copy() - reachable, rbrace, if_storage = self._emit_block(tkn_iter, uop, if_storage, inst, inst_size, True) + reachable, rbrace, if_storage = self._emit_block(tkn_iter, uop, if_storage, inst, True) try: maybe_else = tkn_iter.peek() if maybe_else and maybe_else.kind == "ELSE": @@ -422,7 +406,7 @@ def _emit_if( self.out.start_line() self.emit("}\n") else: - else_reachable, rbrace, else_storage = self._emit_block(tkn_iter, uop, storage, inst, inst_size, True) + else_reachable, rbrace, else_storage = self._emit_block(tkn_iter, uop, storage, inst, True) if not reachable: # Discard the if storage reachable = else_reachable @@ -458,7 +442,6 @@ def _emit_block( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None, emit_first_brace: bool ) -> tuple[bool, Token, Storage]: """ Returns (reachable?, closing '}', stack).""" @@ -501,7 +484,7 @@ def _emit_block( self.out.emit(tkn) elif tkn.kind == "IDENTIFIER": if tkn.text in self._replacers: - if not self._replacers[tkn.text](tkn, tkn_iter, uop, storage, inst, inst_size): + if not self._replacers[tkn.text](tkn, tkn_iter, uop, storage, inst): reachable = False else: if tkn in out_stores: @@ -532,11 +515,10 @@ def emit_tokens( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None ) -> Storage: tkn_iter = TokenIterator(uop.body) self.out.start_line() - _, rbrace, storage = self._emit_block(tkn_iter, uop, storage, inst, inst_size, False) + _, rbrace, storage = self._emit_block(tkn_iter, uop, storage, inst, False) try: self._print_storage(storage) storage.push_outputs() @@ -583,37 +565,3 @@ def cflags(p: Properties) -> str: return " | ".join(flags) else: return "0" - - -def contains_instruction_size_macro(uop: Uop) -> bool: - """Return True if the uop contains the INSTRUCTION_SIZE macro.""" - for token in uop.body: - if token.kind == "IDENTIFIER" and token.text in 'INSTRUCTION_SIZE': - return True - return False - - -def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int: - """Return the size of the instruction that contains the given uop. - - If there is more than one instruction that contains the uop, - ensure that they all have the same size. - """ - size = None - for inst in instructions.values(): - if uop in inst.parts: - if size is None: - size = inst.size - if size != inst.size: - assert size == inst.size, ( - "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " - f"must have the same size: {size} != {inst.size}" - ) - if size is None: - msg = f"No instruction containing the uop '{uop.name}' was found" - raise ValueError(msg) - return size - - -def assert_same_instruction_size(instructions: dict[str, Instruction], uop: Uop) -> None: - get_instruction_size_for_uop(instructions, uop) diff --git a/Tools/cases_generator/tier1_generator.py b/Tools/cases_generator/tier1_generator.py index f25237bfd09381..fcdd3bdacd0e7a 100644 --- a/Tools/cases_generator/tier1_generator.py +++ b/Tools/cases_generator/tier1_generator.py @@ -22,8 +22,6 @@ write_header, type_and_null, Emitter, - contains_instruction_size_macro, - assert_same_instruction_size, ) from cwriter import CWriter from typing import TextIO @@ -173,8 +171,6 @@ def generate_tier1( offset = 1 # The instruction itself stack = Stack() for part in inst.parts: - if isinstance(part, Uop) and contains_instruction_size_macro(part): - assert_same_instruction_size(analysis.instructions, part) # Only emit braces if more than one uop insert_braces = len([p for p in inst.parts if isinstance(p, Uop)]) > 1 offset, stack = write_uop(part, emitter, offset, stack, inst, insert_braces) diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 84c77e521bfe6a..654270c7d28e09 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -22,8 +22,6 @@ Emitter, TokenIterator, always_true, - contains_instruction_size_macro, - get_instruction_size_for_uop, ) from cwriter import CWriter from typing import TextIO, Iterator @@ -77,7 +75,6 @@ def error_if( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -100,7 +97,6 @@ def error_no_pop( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: next(tkn_iter) # LPAREN next(tkn_iter) # RPAREN @@ -115,7 +111,6 @@ def deopt_if( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -137,7 +132,6 @@ def exit_if( # type: ignore[override] uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: self.out.emit_at("if ", tkn) lparen = next(tkn_iter) @@ -158,7 +152,6 @@ def oparg( uop: Uop, storage: Storage, inst: Instruction | None, - inst_size: int | None = None, ) -> bool: if not uop.name.endswith("_0") and not uop.name.endswith("_1"): self.emit(tkn) @@ -174,7 +167,7 @@ def oparg( return True -def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst_size: int | None = None) -> Stack: +def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> Stack: locals: dict[str, Local] = {} try: emitter.out.start_line() @@ -195,7 +188,7 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack, inst_size: int | None = type = f"uint{cache.size*16}_t " cast = f"uint{cache.size*16}_t" emitter.emit(f"{type}{cache.name} = ({cast})CURRENT_OPERAND();\n") - storage = emitter.emit_tokens(uop, storage, None, inst_size) + storage = emitter.emit_tokens(uop, storage, None) except StackError as ex: raise analysis_error(ex.args[0], uop.body[0]) from None return storage.stack @@ -235,11 +228,8 @@ def generate_tier2( continue out.emit(f"case {uop.name}: {{\n") declare_variables(uop, out) - inst_size = None - if contains_instruction_size_macro(uop): - inst_size = get_instruction_size_for_uop(analysis.instructions, uop) stack = Stack() - stack = write_uop(uop, emitter, stack, inst_size) + stack = write_uop(uop, emitter, stack) out.start_line() if not uop.properties.always_exits: stack.flush(out) From 33343dd6010d568890a328261eea11f9e8c52e2c Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 21 Oct 2024 23:20:40 +0200 Subject: [PATCH 24/33] Remove extra white space --- Tools/cases_generator/tier2_generator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/cases_generator/tier2_generator.py b/Tools/cases_generator/tier2_generator.py index 654270c7d28e09..634848c10309d5 100644 --- a/Tools/cases_generator/tier2_generator.py +++ b/Tools/cases_generator/tier2_generator.py @@ -193,7 +193,6 @@ def write_uop(uop: Uop, emitter: Emitter, stack: Stack) -> Stack: raise analysis_error(ex.args[0], uop.body[0]) from None return storage.stack - SKIPS = ("_EXTENDED_ARG",) From f9ecb1639f2d403eb83909d4a164d2d331d781f4 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 21 Oct 2024 23:24:20 +0200 Subject: [PATCH 25/33] Raise analysis_error instead of ValueError --- Tools/cases_generator/analyzer.py | 3 +-- Tools/cases_generator/generators_common.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 8db4eb41cc197f..cab93ad3f796d7 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -1105,8 +1105,7 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) f"must have the same size: {size} != {inst.size}" ) if size is None: - msg = f"No instruction containing the uop '{uop.name}' was found" - raise ValueError(msg) + raise analysis_error(f"No instruction containing the uop '{uop.name}' was found") return size diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 659794c1eeb799..229e52b2328594 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -366,7 +366,8 @@ def instruction_size(self, inst: Instruction | None, ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" - assert uop.instruction_size is not None, "INSTRUCTION_SIZE macro requires uop.instruction_size to be set" + if uop.instruction_size is None: + raise analysis_error("INSTRUCTION_SIZE macro requires uop.instruction_size to be set", tkn) self.out.emit(f" {uop.instruction_size} ") return True From abfaaff79a7d27005d8c3f7d0277ddc4138d27dd Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 21 Oct 2024 23:25:39 +0200 Subject: [PATCH 26/33] Fix wording --- Tools/cases_generator/generators_common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/generators_common.py b/Tools/cases_generator/generators_common.py index 229e52b2328594..f0065ca7abda0b 100644 --- a/Tools/cases_generator/generators_common.py +++ b/Tools/cases_generator/generators_common.py @@ -367,7 +367,7 @@ def instruction_size(self, ) -> bool: """Replace the INSTRUCTION_SIZE macro with the size of the current instruction.""" if uop.instruction_size is None: - raise analysis_error("INSTRUCTION_SIZE macro requires uop.instruction_size to be set", tkn) + raise analysis_error("The INSTRUCTION_SIZE macro requires uop.instruction_size to be set", tkn) self.out.emit(f" {uop.instruction_size} ") return True From 3f3ad1065ddcfdb10017beef33cae0d3482f1bdd Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 21 Oct 2024 23:38:58 +0200 Subject: [PATCH 27/33] Make mypy happy --- Tools/cases_generator/analyzer.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index cab93ad3f796d7..64ad5a80fc4d5a 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -1081,12 +1081,12 @@ def add_instruction(name: str) -> None: return instmap, len(no_arg), min_instrumented +def is_instruction_size_macro(token: lexer.Token) -> bool: + return token.kind == "IDENTIFIER" and token.text == "INSTRUCTION_SIZE" + + def contains_instruction_size_macro(uop: Uop) -> bool: - """Return True if the uop contains the INSTRUCTION_SIZE macro.""" - for token in uop.body: - if token.kind == "IDENTIFIER" and token.text in 'INSTRUCTION_SIZE': - return True - return False + return any(is_instruction_size_macro(token) for token in uop.body) def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int: @@ -1105,7 +1105,8 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) f"must have the same size: {size} != {inst.size}" ) if size is None: - raise analysis_error(f"No instruction containing the uop '{uop.name}' was found") + token = next(t for t in uop.body if is_instruction_size_macro(t)) + raise analysis_error(f"No instruction containing the uop '{uop.name}' was found", token) return size From a8f72ef78ea53a34b66ffa1f50c1a78ded293740 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 28 Oct 2024 19:29:59 +0100 Subject: [PATCH 28/33] Simplify test --- Lib/test/test_generated_cases.py | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index dbf83f9031f5f1..f13eb434a6873e 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -1398,25 +1398,8 @@ def test_instruction_size_macro(self): macro(OP2) = unused/1 + OP; """ - output = """ - TARGET(OP) { - frame->instr_ptr = next_instr; - next_instr += 1; - INSTRUCTION_STATS(OP); - frame->return_offset = 1 ; - DISPATCH(); - } - - TARGET(OP2) { - frame->instr_ptr = next_instr; - next_instr += 2; - INSTRUCTION_STATS(OP2); - /* Skip 1 cache entry */ - frame->return_offset = 2 ; - DISPATCH(); - } - """ - with self.assertRaisesRegex(AssertionError, "All instructions containing a uop"): + output = "" # No output needed as this should raise an error. + with self.assertRaisesRegex(SyntaxError, "All instructions containing a uop"): self.run_cases_test(input, output) From 8138a06c269bdcb63ba47d8b40455e7e417d9581 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 28 Oct 2024 19:36:30 +0100 Subject: [PATCH 29/33] Replace assert with raise --- Tools/cases_generator/analyzer.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 64ad5a80fc4d5a..f8b79a916acbb1 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -1100,10 +1100,13 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) if uop in inst.parts: if size is None: size = inst.size - assert size == inst.size, ( - "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " - f"must have the same size: {size} != {inst.size}" - ) + if size != inst.size: + token = next(t for t in uop.body if is_instruction_size_macro(t)) + raise analysis_error( + "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " + f"must have the same size: {size} != {inst.size}", + token + ) if size is None: token = next(t for t in uop.body if is_instruction_size_macro(t)) raise analysis_error(f"No instruction containing the uop '{uop.name}' was found", token) From 772f80371421df432ec596eb19106f42bdbe3837 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 28 Oct 2024 19:41:40 +0100 Subject: [PATCH 30/33] Simplify code --- Tools/cases_generator/analyzer.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index f8b79a916acbb1..8b0ddbf254535a 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -1085,16 +1085,16 @@ def is_instruction_size_macro(token: lexer.Token) -> bool: return token.kind == "IDENTIFIER" and token.text == "INSTRUCTION_SIZE" -def contains_instruction_size_macro(uop: Uop) -> bool: - return any(is_instruction_size_macro(token) for token in uop.body) - - -def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int: - """Return the size of the instruction that contains the given uop. +def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int | None: + """Return the size of the instruction that contains the given uop or + `None` if the uop does not contains the `INSTRUCTION_SIZE` macro. If there is more than one instruction that contains the uop, ensure that they all have the same size. """ + if not any(is_instruction_size_macro(token) for token in uop.body): + return None + size = None for inst in instructions.values(): if uop in inst.parts: @@ -1157,8 +1157,6 @@ def analyze_forest(forest: list[parser.AstNode]) -> Analysis: if target.text in instructions: instructions[target.text].is_target = True for uop in uops.values(): - if not contains_instruction_size_macro(uop): - continue uop.instruction_size = get_instruction_size_for_uop(instructions, uop) # Special case BINARY_OP_INPLACE_ADD_UNICODE # BINARY_OP_INPLACE_ADD_UNICODE is not a normal family member, From a718cfb1f23d72dd9641aa470cd5f20da6d1439c Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 28 Oct 2024 19:45:28 +0100 Subject: [PATCH 31/33] Remove some code repetition --- Tools/cases_generator/analyzer.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 8b0ddbf254535a..ef63771f0524e6 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -1092,7 +1092,7 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) If there is more than one instruction that contains the uop, ensure that they all have the same size. """ - if not any(is_instruction_size_macro(token) for token in uop.body): + if not (token := next((t for t in uop.body if is_instruction_size_macro(t)), None)): return None size = None @@ -1101,14 +1101,12 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) if size is None: size = inst.size if size != inst.size: - token = next(t for t in uop.body if is_instruction_size_macro(t)) raise analysis_error( "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " f"must have the same size: {size} != {inst.size}", token ) if size is None: - token = next(t for t in uop.body if is_instruction_size_macro(t)) raise analysis_error(f"No instruction containing the uop '{uop.name}' was found", token) return size From a10d4ee3fdcd9a2f9b778b3d4d077b0cf691d5ef Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 29 Oct 2024 17:49:05 +0100 Subject: [PATCH 32/33] Simplify INSTRUCTION_SIZE check --- Tools/cases_generator/analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index ef63771f0524e6..aa6064c4eebd01 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -1082,7 +1082,7 @@ def add_instruction(name: str) -> None: def is_instruction_size_macro(token: lexer.Token) -> bool: - return token.kind == "IDENTIFIER" and token.text == "INSTRUCTION_SIZE" + return token.text == "INSTRUCTION_SIZE" def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int | None: From 3ae7a5d6e9e8865ba6529c91a1110ea597c7c9cc Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 29 Oct 2024 17:53:02 +0100 Subject: [PATCH 33/33] Simplify code --- Tools/cases_generator/analyzer.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index aa6064c4eebd01..94112143532d58 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -1081,10 +1081,6 @@ def add_instruction(name: str) -> None: return instmap, len(no_arg), min_instrumented -def is_instruction_size_macro(token: lexer.Token) -> bool: - return token.text == "INSTRUCTION_SIZE" - - def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) -> int | None: """Return the size of the instruction that contains the given uop or `None` if the uop does not contains the `INSTRUCTION_SIZE` macro. @@ -1092,7 +1088,10 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) If there is more than one instruction that contains the uop, ensure that they all have the same size. """ - if not (token := next((t for t in uop.body if is_instruction_size_macro(t)), None)): + for tkn in uop.body: + if tkn.text == "INSTRUCTION_SIZE": + break + else: return None size = None @@ -1104,10 +1103,10 @@ def get_instruction_size_for_uop(instructions: dict[str, Instruction], uop: Uop) raise analysis_error( "All instructions containing a uop with the `INSTRUCTION_SIZE` macro " f"must have the same size: {size} != {inst.size}", - token + tkn ) if size is None: - raise analysis_error(f"No instruction containing the uop '{uop.name}' was found", token) + raise analysis_error(f"No instruction containing the uop '{uop.name}' was found", tkn) return size