From edb68e19eb2ad5f96d6637e3813578ce740850c4 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 8 Apr 2025 14:16:39 -0700 Subject: [PATCH 1/5] Add more comprehensive peepholing to the JIT --- Lib/test/test_capi/test_opt.py | 6 ++-- Python/optimizer_analysis.c | 53 ++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 00aebd68f9ef43..af5460f412fe39 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -1283,7 +1283,7 @@ class Bar: load_attr_top = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", 0, call) load_attr_bottom = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", call) self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1) - self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 1) + self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 2) def test_guard_type_version_removed_escaping(self): @@ -1306,7 +1306,7 @@ class Foo: load_attr_top = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", 0, call) load_attr_bottom = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", call) self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1) - self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 1) + self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 2) def test_guard_type_version_executor_invalidated(self): """ @@ -1601,7 +1601,7 @@ def testfunc(n): self.assertIsNotNone(ex) uops = get_opnames(ex) self.assertNotIn("_COMPARE_OP_INT", uops) - self.assertIn("_POP_TWO_LOAD_CONST_INLINE_BORROW", uops) + self.assertNotIn("_POP_TWO_LOAD_CONST_INLINE_BORROW", uops) def test_to_bool_bool_contains_op_set(self): """ diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index ab28fae94a96a9..b2605e704026b7 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -555,28 +555,47 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size) } break; case _POP_TOP: + case _POP_TOP_LOAD_CONST_INLINE: + case _POP_TOP_LOAD_CONST_INLINE_BORROW: + case _POP_TWO_LOAD_CONST_INLINE_BORROW: + optimize_pop_top_again: { _PyUOpInstruction *last = &buffer[pc-1]; while (last->opcode == _NOP) { last--; } - if (last->opcode == _LOAD_CONST_INLINE || - last->opcode == _LOAD_CONST_INLINE_BORROW || - last->opcode == _LOAD_FAST || - last->opcode == _LOAD_FAST_BORROW || - last->opcode == _COPY - ) { - last->opcode = _NOP; - buffer[pc].opcode = _NOP; - } - if (last->opcode == _REPLACE_WITH_TRUE) { - last->opcode = _NOP; + switch (last->opcode) { + case _POP_TWO_LOAD_CONST_INLINE_BORROW: + last->opcode = _POP_TOP; + goto optimize_pop_top_again; + case _POP_TOP_LOAD_CONST_INLINE: + case _POP_TOP_LOAD_CONST_INLINE_BORROW: + last->opcode = _NOP; + goto optimize_pop_top_again; + case _COPY: + case _LOAD_CONST_INLINE: + case _LOAD_CONST_INLINE_BORROW: + case _LOAD_FAST: + case _LOAD_FAST_BORROW: + case _LOAD_SMALL_INT: + last->opcode = _NOP; + if (opcode == _POP_TOP) { + opcode = buffer[pc].opcode = _NOP; + } + else if (opcode == _POP_TOP_LOAD_CONST_INLINE) { + opcode = buffer[pc].opcode = _LOAD_CONST_INLINE; + } + else if (opcode == _POP_TOP_LOAD_CONST_INLINE_BORROW) { + opcode = buffer[pc].opcode = _LOAD_CONST_INLINE_BORROW; + } + else { + assert(opcode == _POP_TWO_LOAD_CONST_INLINE_BORROW); + opcode = buffer[pc].opcode = _POP_TOP_LOAD_CONST_INLINE_BORROW; + goto optimize_pop_top_again; + } } - break; + _Py_FALLTHROUGH; } - case _JUMP_TO_TOP: - case _EXIT_TRACE: - return pc + 1; default: { /* _PUSH_FRAME doesn't escape or error, but it @@ -591,7 +610,11 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size) buffer[last_set_ip].opcode = _SET_IP; last_set_ip = -1; } + break; } + case _JUMP_TO_TOP: + case _EXIT_TRACE: + return pc + 1; } } Py_UNREACHABLE(); From aada255ecceabd68b981c38c9c39b9fd518f264e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 9 Apr 2025 14:06:02 -0700 Subject: [PATCH 2/5] blurb add --- .../2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst new file mode 100644 index 00000000000000..03523de79b69ab --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-09-14-05-54.gh-issue-130415.llQtUq.rst @@ -0,0 +1,3 @@ +Improve the JIT's ability to remove unused constant and local variable +loads, and fix an issue where deallocating unused values could cause JIT +code to crash or behave incorrectly. From e0ad73962603c8e57a980a3d33e1b3678508b65a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 18 Apr 2025 14:06:26 -0700 Subject: [PATCH 3/5] Remove unnecessary loop. --- Python/optimizer_analysis.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index b2605e704026b7..387ebc813abd66 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -567,7 +567,7 @@ remove_unneeded_uops(_PyUOpInstruction *buffer, int buffer_size) switch (last->opcode) { case _POP_TWO_LOAD_CONST_INLINE_BORROW: last->opcode = _POP_TOP; - goto optimize_pop_top_again; + break; case _POP_TOP_LOAD_CONST_INLINE: case _POP_TOP_LOAD_CONST_INLINE_BORROW: last->opcode = _NOP; From 97dbcce6e67b42a4b5627cafde5fbe8afc7d429d Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 18 Apr 2025 14:06:54 -0700 Subject: [PATCH 4/5] Turn _REPLACE_WITH_TRUE into _POP_TOP_LOAD_CONST_INLINE_BORROW --- Python/optimizer_bytecodes.c | 1 + Python/optimizer_cases.c.h | 1 + 2 files changed, 2 insertions(+) diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 0f7e183608e3e9..4777f3ad38fd0b 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -913,6 +913,7 @@ dummy_func(void) { } op(_REPLACE_WITH_TRUE, (value -- res)) { + REPLACE_OP(this_instr, _POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)&Py_True); res = sym_new_const(ctx, Py_True); } diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 0df52e654432b6..dc1694d646e489 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -255,6 +255,7 @@ case _REPLACE_WITH_TRUE: { JitOptSymbol *res; + REPLACE_OP(this_instr, _POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)Py_True); res = sym_new_const(ctx, Py_True); stack_pointer[-1] = res; break; From 9b64c178e07535cc10c0ea04583193ab23467494 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 18 Apr 2025 15:40:18 -0700 Subject: [PATCH 5/5] fixup --- Python/optimizer_bytecodes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 4777f3ad38fd0b..5e05c9a5648914 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -913,7 +913,7 @@ dummy_func(void) { } op(_REPLACE_WITH_TRUE, (value -- res)) { - REPLACE_OP(this_instr, _POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)&Py_True); + REPLACE_OP(this_instr, _POP_TOP_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)Py_True); res = sym_new_const(ctx, Py_True); }