https://github.com/python/cpython/commit/434525398196db692196661d15e678b3d514aa54 commit: 434525398196db692196661d15e678b3d514aa54 branch: main author: Nadeshiko Manju <[email protected]> committer: Fidget-Spinner <[email protected]> date: 2025-12-16T22:21:04Z summary:
gh-134584: Eliminate redundant refcounting from `_STORE_ATTR_WITH_HINT` (GH-142767) Signed-off-by: Manjusaka <[email protected]> files: A Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-05-24-24.gh-issue-134584.tJ1usH.rst M Include/internal/pycore_opcode_metadata.h M Include/internal/pycore_uop_ids.h M Include/internal/pycore_uop_metadata.h M Lib/test/test_capi/test_opt.py M Python/bytecodes.c M Python/executor_cases.c.h M Python/generated_cases.c.h M Python/optimizer_bytecodes.c M Python/optimizer_cases.c.h diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index c0e3725db1d7ca..a4f1f50e96d6d1 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1484,7 +1484,7 @@ _PyOpcode_macro_expansion[256] = { [STORE_ATTR] = { .nuops = 1, .uops = { { _STORE_ATTR, OPARG_SIMPLE, 3 } } }, [STORE_ATTR_INSTANCE_VALUE] = { .nuops = 4, .uops = { { _GUARD_TYPE_VERSION_AND_LOCK, 2, 1 }, { _GUARD_DORV_NO_DICT, OPARG_SIMPLE, 3 }, { _STORE_ATTR_INSTANCE_VALUE, 1, 3 }, { _POP_TOP, OPARG_SIMPLE, 4 } } }, [STORE_ATTR_SLOT] = { .nuops = 3, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _STORE_ATTR_SLOT, 1, 3 }, { _POP_TOP, OPARG_SIMPLE, 4 } } }, - [STORE_ATTR_WITH_HINT] = { .nuops = 2, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _STORE_ATTR_WITH_HINT, 1, 3 } } }, + [STORE_ATTR_WITH_HINT] = { .nuops = 3, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _STORE_ATTR_WITH_HINT, 1, 3 }, { _POP_TOP, OPARG_SIMPLE, 4 } } }, [STORE_DEREF] = { .nuops = 1, .uops = { { _STORE_DEREF, OPARG_SIMPLE, 0 } } }, [STORE_FAST] = { .nuops = 1, .uops = { { _STORE_FAST, OPARG_SIMPLE, 0 } } }, [STORE_FAST_LOAD_FAST] = { .nuops = 2, .uops = { { _STORE_FAST, OPARG_TOP, 0 }, { _LOAD_FAST, OPARG_BOTTOM, 0 } } }, diff --git a/Include/internal/pycore_uop_ids.h b/Include/internal/pycore_uop_ids.h index 5519e14e9020c4..8678ccf5e1b823 100644 --- a/Include/internal/pycore_uop_ids.h +++ b/Include/internal/pycore_uop_ids.h @@ -1038,7 +1038,7 @@ extern "C" { #define _STORE_ATTR_r20 1231 #define _STORE_ATTR_INSTANCE_VALUE_r21 1232 #define _STORE_ATTR_SLOT_r21 1233 -#define _STORE_ATTR_WITH_HINT_r20 1234 +#define _STORE_ATTR_WITH_HINT_r21 1234 #define _STORE_DEREF_r10 1235 #define _STORE_FAST_r10 1236 #define _STORE_FAST_0_r10 1237 diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index f0af30df0634e8..c6d3670b193123 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -1837,7 +1837,7 @@ const _PyUopCachingInfo _PyUop_Caching[MAX_UOP_ID+1] = { .entries = { { -1, -1, -1 }, { -1, -1, -1 }, - { 0, 2, _STORE_ATTR_WITH_HINT_r20 }, + { 1, 2, _STORE_ATTR_WITH_HINT_r21 }, { -1, -1, -1 }, }, }, @@ -3546,7 +3546,7 @@ const uint16_t _PyUop_Uncached[MAX_UOP_REGS_ID+1] = { [_GUARD_DORV_NO_DICT_r22] = _GUARD_DORV_NO_DICT, [_GUARD_DORV_NO_DICT_r33] = _GUARD_DORV_NO_DICT, [_STORE_ATTR_INSTANCE_VALUE_r21] = _STORE_ATTR_INSTANCE_VALUE, - [_STORE_ATTR_WITH_HINT_r20] = _STORE_ATTR_WITH_HINT, + [_STORE_ATTR_WITH_HINT_r21] = _STORE_ATTR_WITH_HINT, [_STORE_ATTR_SLOT_r21] = _STORE_ATTR_SLOT, [_COMPARE_OP_r21] = _COMPARE_OP, [_COMPARE_OP_FLOAT_r01] = _COMPARE_OP_FLOAT, @@ -4786,7 +4786,7 @@ const char *const _PyOpcode_uop_name[MAX_UOP_REGS_ID+1] = { [_STORE_ATTR_SLOT] = "_STORE_ATTR_SLOT", [_STORE_ATTR_SLOT_r21] = "_STORE_ATTR_SLOT_r21", [_STORE_ATTR_WITH_HINT] = "_STORE_ATTR_WITH_HINT", - [_STORE_ATTR_WITH_HINT_r20] = "_STORE_ATTR_WITH_HINT_r20", + [_STORE_ATTR_WITH_HINT_r21] = "_STORE_ATTR_WITH_HINT_r21", [_STORE_DEREF] = "_STORE_DEREF", [_STORE_DEREF_r10] = "_STORE_DEREF_r10", [_STORE_FAST] = "_STORE_FAST", diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 3ae87fead94d3b..bed8d1d52e93b2 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -2612,6 +2612,26 @@ class C: self.assertNotIn("_POP_TOP", uops) self.assertIn("_POP_TOP_NOP", uops) + def test_store_attr_with_hint(self): + def testfunc(n): + class C: + pass + c = C() + for i in range(_testinternalcapi.SHARED_KEYS_MAX_SIZE - 1): + setattr(c, f"_{i}", None) + + for i in range(n): + c.x = i + return c.x + res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD) + self.assertEqual(res, TIER2_THRESHOLD - 1) + self.assertIsNotNone(ex) + uops = get_opnames(ex) + + self.assertIn("_STORE_ATTR_WITH_HINT", uops) + self.assertNotIn("_POP_TOP", uops) + self.assertIn("_POP_TOP_NOP", uops) + def test_store_subscr_int(self): def testfunc(n): l = [0, 0, 0, 0] diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-05-24-24.gh-issue-134584.tJ1usH.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-05-24-24.gh-issue-134584.tJ1usH.rst new file mode 100644 index 00000000000000..aa096fc827dbc4 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-05-24-24.gh-issue-134584.tJ1usH.rst @@ -0,0 +1 @@ +Eliminate redundant refcounting from ``_STORE_ATTR_WITH_HINT``. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f126c44749254b..25e730f7e6e8ec 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2603,7 +2603,7 @@ dummy_func( _STORE_ATTR_INSTANCE_VALUE + POP_TOP; - op(_STORE_ATTR_WITH_HINT, (hint/1, value, owner --)) { + op(_STORE_ATTR_WITH_HINT, (hint/1, value, owner -- o)) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); assert(Py_TYPE(owner_o)->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictObject *dict = _PyObject_GetManagedDict(owner_o); @@ -2633,14 +2633,16 @@ dummy_func( // old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault, // when dict only holds the strong reference to value in ep->me_value. STAT_INC(STORE_ATTR, hit); - PyStackRef_CLOSE(owner); + o = owner; + DEAD(owner); Py_XDECREF(old_value); } macro(STORE_ATTR_WITH_HINT) = unused/1 + _GUARD_TYPE_VERSION + - _STORE_ATTR_WITH_HINT; + _STORE_ATTR_WITH_HINT + + POP_TOP; op(_STORE_ATTR_SLOT, (index/1, value, owner -- o)) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index e64934483a2841..55c4495f2dc59d 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -8775,11 +8775,12 @@ break; } - case _STORE_ATTR_WITH_HINT_r20: { + case _STORE_ATTR_WITH_HINT_r21: { CHECK_CURRENT_CACHED_VALUES(2); assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE()); _PyStackRef owner; _PyStackRef value; + _PyStackRef o; _PyStackRef _stack_item_0 = _tos_cache0; _PyStackRef _stack_item_1 = _tos_cache1; oparg = CURRENT_OPARG(); @@ -8848,16 +8849,19 @@ FT_ATOMIC_STORE_PTR_RELEASE(ep->me_value, PyStackRef_AsPyObjectSteal(value)); UNLOCK_OBJECT(dict); STAT_INC(STORE_ATTR, hit); - stack_pointer += -2; + o = owner; + stack_pointer[-2] = o; + stack_pointer += -1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(owner); Py_XDECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); - _tos_cache0 = PyStackRef_ZERO_BITS; + _tos_cache0 = o; _tos_cache1 = PyStackRef_ZERO_BITS; _tos_cache2 = PyStackRef_ZERO_BITS; - SET_CURRENT_CACHED_VALUES(0); + SET_CURRENT_CACHED_VALUES(1); + stack_pointer += -1; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE()); break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index cd52a20be9fd11..3c18960a39d7d7 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -10747,6 +10747,7 @@ static_assert(INLINE_CACHE_ENTRIES_STORE_ATTR == 4, "incorrect cache size"); _PyStackRef owner; _PyStackRef value; + _PyStackRef o; /* Skip 1 cache entry */ // _GUARD_TYPE_VERSION { @@ -10812,13 +10813,23 @@ FT_ATOMIC_STORE_PTR_RELEASE(ep->me_value, PyStackRef_AsPyObjectSteal(value)); UNLOCK_OBJECT(dict); STAT_INC(STORE_ATTR, hit); - stack_pointer += -2; + o = owner; + stack_pointer[-2] = o; + stack_pointer += -1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(owner); Py_XDECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); } + // _POP_TOP + { + value = o; + stack_pointer += -1; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_XCLOSE(value); + stack_pointer = _PyFrame_GetStackPointer(frame); + } DISPATCH(); } diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index e9e5bd5db1f04f..31ec88e1c84912 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -104,6 +104,11 @@ dummy_func(void) { o = owner; } + op(_STORE_ATTR_WITH_HINT, (hint/1, value, owner -- o)) { + (void)value; + o = owner; + } + op(_STORE_FAST, (value --)) { GETLOCAL(oparg) = value; } diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 9c61b06d5f7eed..6fde87a17fbaa3 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -1699,8 +1699,17 @@ } case _STORE_ATTR_WITH_HINT: { - CHECK_STACK_BOUNDS(-2); - stack_pointer += -2; + JitOptRef owner; + JitOptRef value; + JitOptRef o; + owner = stack_pointer[-1]; + value = stack_pointer[-2]; + uint16_t hint = (uint16_t)this_instr->operand0; + (void)value; + o = owner; + CHECK_STACK_BOUNDS(-1); + stack_pointer[-2] = o; + stack_pointer += -1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); break; } _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
