From 0c48c459c2eccb7009ff7510f7a600c29c7e47c2 Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Tue, 23 Dec 2025 19:58:19 +0000 Subject: [PATCH 1/3] Strength reduce _LOAD_FAST{_BORROW} to _LOAD_CONST_INLINE_BORROW --- Lib/test/test_capi/test_opt.py | 34 ++++++++++++++++++++++++++++++++++ Python/optimizer_bytecodes.c | 12 ++++++++++++ Python/optimizer_cases.c.h | 8 ++++++++ 3 files changed, 54 insertions(+) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 16288a447e20fe..67106f613ca393 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -3114,6 +3114,40 @@ def testfunc(n): self.assertNotIn("_POP_TOP_INT", uops) self.assertIn("_POP_TOP_NOP", uops) + def test_strength_reduce_constant_load_fast(self): + # If we detect a _LOAD_FAST is actually loading an immortal constant, + # reduce that to a _LOAD_CONST_INLINE_BORROW which saves + # the read from locals. + def testfunc(n): + for _ in range(n): + x = 0 + y = x + + _, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD) + self.assertIsNotNone(ex) + uops = get_opnames(ex) + + self.assertIn("_LOAD_CONST_INLINE_BORROW", uops) + self.assertNotIn("_LOAD_FAST_2", uops) + + def test_strength_reduce_constant_load_fast_borrow(self): + # If we detect a _LOAD_FAST_BORROW is actually loading a constant, + # reduce that to a _LOAD_CONST_INLINE_BORROW which saves + # the read from locals. + def testfunc(n): + class A: pass + a = A() + for _ in range(n): + x = 0 + a.x = x + + _, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD) + self.assertIsNotNone(ex) + uops = get_opnames(ex) + + self.assertIn("_LOAD_CONST_INLINE_BORROW", uops) + self.assertNotIn("_LOAD_FAST_BORROW_4", uops) + def test_143026(self): # https://github.com/python/cpython/issues/143026 diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index b40b597643dc94..721b6b486e8f73 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -87,10 +87,22 @@ dummy_func(void) { op(_LOAD_FAST, (-- value)) { value = GETLOCAL(oparg); + PyObject *const_val = sym_get_const(ctx, value); + if (const_val != NULL && _Py_IsImmortal(const_val)) { + // Note: non-immortal is not safe to replace + // to _LOAD_CONST_INLINE, as it might not be held in co_const. + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } } op(_LOAD_FAST_BORROW, (-- value)) { value = PyJitRef_Borrow(GETLOCAL(oparg)); + PyObject *const_val = sym_get_const(ctx, value); + if (const_val != NULL) { + // It's safe to always borrow here, because + // _LOAD_FAST_BORROW guarantees it. + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } } op(_LOAD_FAST_AND_CLEAR, (-- value)) { diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index a17a5688847e07..77988ca37d645f 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -43,6 +43,10 @@ case _LOAD_FAST: { JitOptRef value; value = GETLOCAL(oparg); + PyObject *const_val = sym_get_const(ctx, value); + if (const_val != NULL && _Py_IsImmortal(const_val)) { + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } CHECK_STACK_BOUNDS(1); stack_pointer[0] = value; stack_pointer += 1; @@ -53,6 +57,10 @@ case _LOAD_FAST_BORROW: { JitOptRef value; value = PyJitRef_Borrow(GETLOCAL(oparg)); + PyObject *const_val = sym_get_const(ctx, value); + if (const_val != NULL) { + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } CHECK_STACK_BOUNDS(1); stack_pointer[0] = value; stack_pointer += 1; From b343a84cf517bfe2521a310ac5991cfcaf3d96f4 Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Tue, 23 Dec 2025 20:57:41 +0000 Subject: [PATCH 2/3] block anything with free variables --- Lib/test/test_capi/test_opt.py | 2 +- Python/optimizer_bytecodes.c | 14 ++++++++++---- Python/optimizer_cases.c.h | 14 ++++++++------ 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 67106f613ca393..ea72f9fdc00915 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -3115,7 +3115,7 @@ def testfunc(n): self.assertIn("_POP_TOP_NOP", uops) def test_strength_reduce_constant_load_fast(self): - # If we detect a _LOAD_FAST is actually loading an immortal constant, + # If we detect a _LOAD_FAST is actually loading a constant, # reduce that to a _LOAD_CONST_INLINE_BORROW which saves # the read from locals. def testfunc(n): diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 721b6b486e8f73..4c696b7ecae4a3 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -88,9 +88,12 @@ dummy_func(void) { op(_LOAD_FAST, (-- value)) { value = GETLOCAL(oparg); PyObject *const_val = sym_get_const(ctx, value); - if (const_val != NULL && _Py_IsImmortal(const_val)) { - // Note: non-immortal is not safe to replace - // to _LOAD_CONST_INLINE, as it might not be held in co_const. + PyCodeObject *co = get_current_code_object(ctx); + // We don't reason about free variables yet, so we need to forbid + // anything with those. + if (const_val != NULL && co->co_nfreevars == 0) { + // It's safe to always borrow here, for + // the same reason as _LOAD_CONST. REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); } } @@ -98,7 +101,10 @@ dummy_func(void) { op(_LOAD_FAST_BORROW, (-- value)) { value = PyJitRef_Borrow(GETLOCAL(oparg)); PyObject *const_val = sym_get_const(ctx, value); - if (const_val != NULL) { + PyCodeObject *co = get_current_code_object(ctx); + // We don't reason about free variables yet, so we need to forbid + // anything with those. + if (const_val != NULL && co->co_nfreevars == 0) { // It's safe to always borrow here, because // _LOAD_FAST_BORROW guarantees it. REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 77988ca37d645f..347cfc5f1c4ca7 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -44,13 +44,14 @@ JitOptRef value; value = GETLOCAL(oparg); PyObject *const_val = sym_get_const(ctx, value); - if (const_val != NULL && _Py_IsImmortal(const_val)) { - REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); - } CHECK_STACK_BOUNDS(1); stack_pointer[0] = value; stack_pointer += 1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); + PyCodeObject *co = get_current_code_object(ctx); + if (const_val != NULL && co->co_nfreevars == 0) { + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } break; } @@ -58,13 +59,14 @@ JitOptRef value; value = PyJitRef_Borrow(GETLOCAL(oparg)); PyObject *const_val = sym_get_const(ctx, value); - if (const_val != NULL) { - REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); - } CHECK_STACK_BOUNDS(1); stack_pointer[0] = value; stack_pointer += 1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); + PyCodeObject *co = get_current_code_object(ctx); + if (const_val != NULL && co->co_nfreevars == 0) { + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } break; } From 8b0342d2409d04daf78b7eb36bad49419cada13e Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Tue, 23 Dec 2025 21:36:15 +0000 Subject: [PATCH 3/3] not a fix --- Python/optimizer_bytecodes.c | 15 +++++---------- Python/optimizer_cases.c.h | 17 ++++++++--------- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 4c696b7ecae4a3..de1774e7a57e24 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -88,12 +88,10 @@ dummy_func(void) { op(_LOAD_FAST, (-- value)) { value = GETLOCAL(oparg); PyObject *const_val = sym_get_const(ctx, value); - PyCodeObject *co = get_current_code_object(ctx); - // We don't reason about free variables yet, so we need to forbid - // anything with those. - if (const_val != NULL && co->co_nfreevars == 0) { + if (const_val != NULL) { // It's safe to always borrow here, for - // the same reason as _LOAD_CONST. + // the same reason as _LOAD_CONST.. + value = PyJitRef_Borrow(value); REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); } } @@ -101,10 +99,7 @@ dummy_func(void) { op(_LOAD_FAST_BORROW, (-- value)) { value = PyJitRef_Borrow(GETLOCAL(oparg)); PyObject *const_val = sym_get_const(ctx, value); - PyCodeObject *co = get_current_code_object(ctx); - // We don't reason about free variables yet, so we need to forbid - // anything with those. - if (const_val != NULL && co->co_nfreevars == 0) { + if (const_val != NULL) { // It's safe to always borrow here, because // _LOAD_FAST_BORROW guarantees it. REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); @@ -128,7 +123,7 @@ dummy_func(void) { } op(_STORE_FAST, (value --)) { - GETLOCAL(oparg) = value; + GETLOCAL(oparg) = PyJitRef_StripReferenceInfo(value); } op(_STORE_SUBSCR_LIST_INT, (value, list_st, sub_st -- ls, ss)) { diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 347cfc5f1c4ca7..15d0b56ebddf93 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -44,14 +44,14 @@ JitOptRef value; value = GETLOCAL(oparg); PyObject *const_val = sym_get_const(ctx, value); + if (const_val != NULL) { + value = PyJitRef_Borrow(value); + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } CHECK_STACK_BOUNDS(1); stack_pointer[0] = value; stack_pointer += 1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); - PyCodeObject *co = get_current_code_object(ctx); - if (const_val != NULL && co->co_nfreevars == 0) { - REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); - } break; } @@ -59,14 +59,13 @@ JitOptRef value; value = PyJitRef_Borrow(GETLOCAL(oparg)); PyObject *const_val = sym_get_const(ctx, value); + if (const_val != NULL) { + REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); + } CHECK_STACK_BOUNDS(1); stack_pointer[0] = value; stack_pointer += 1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); - PyCodeObject *co = get_current_code_object(ctx); - if (const_val != NULL && co->co_nfreevars == 0) { - REPLACE_OP(this_instr, _LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)const_val); - } break; } @@ -112,7 +111,7 @@ case _STORE_FAST: { JitOptRef value; value = stack_pointer[-1]; - GETLOCAL(oparg) = value; + GETLOCAL(oparg) = PyJitRef_StripReferenceInfo(value); CHECK_STACK_BOUNDS(-1); stack_pointer += -1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);