https://github.com/python/cpython/commit/abaaeee6a0abe96ed4c4cfb22018e8b871a67102 commit: abaaeee6a0abe96ed4c4cfb22018e8b871a67102 branch: main author: Donghee Na <[email protected]> committer: Fidget-Spinner <[email protected]> date: 2025-12-15T16:42:32Z summary:
gh-134584: Eliminate redundant refcounting from _STORE_SUBSCR_DICT (GH-142712) Co-authored-by: Ken Jin <[email protected]> files: 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 9708ce817a1d37..fda52806561430 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1493,7 +1493,7 @@ _PyOpcode_macro_expansion[256] = { [STORE_NAME] = { .nuops = 1, .uops = { { _STORE_NAME, OPARG_SIMPLE, 0 } } }, [STORE_SLICE] = { .nuops = 1, .uops = { { _STORE_SLICE, OPARG_SIMPLE, 0 } } }, [STORE_SUBSCR] = { .nuops = 1, .uops = { { _STORE_SUBSCR, OPARG_SIMPLE, 0 } } }, - [STORE_SUBSCR_DICT] = { .nuops = 2, .uops = { { _GUARD_NOS_DICT, OPARG_SIMPLE, 0 }, { _STORE_SUBSCR_DICT, OPARG_SIMPLE, 1 } } }, + [STORE_SUBSCR_DICT] = { .nuops = 3, .uops = { { _GUARD_NOS_DICT, OPARG_SIMPLE, 0 }, { _STORE_SUBSCR_DICT, OPARG_SIMPLE, 1 }, { _POP_TOP, OPARG_SIMPLE, 1 } } }, [STORE_SUBSCR_LIST_INT] = { .nuops = 5, .uops = { { _GUARD_TOS_INT, OPARG_SIMPLE, 0 }, { _GUARD_NOS_LIST, OPARG_SIMPLE, 0 }, { _STORE_SUBSCR_LIST_INT, OPARG_SIMPLE, 1 }, { _POP_TOP_INT, OPARG_SIMPLE, 1 }, { _POP_TOP, OPARG_SIMPLE, 1 } } }, [SWAP] = { .nuops = 1, .uops = { { _SWAP, OPARG_SIMPLE, 0 } } }, [TO_BOOL] = { .nuops = 1, .uops = { { _TO_BOOL, OPARG_SIMPLE, 2 } } }, diff --git a/Include/internal/pycore_uop_ids.h b/Include/internal/pycore_uop_ids.h index febbb54b607355..42793d940b51b7 100644 --- a/Include/internal/pycore_uop_ids.h +++ b/Include/internal/pycore_uop_ids.h @@ -1070,7 +1070,7 @@ extern "C" { #define _STORE_NAME_r10 1263 #define _STORE_SLICE_r30 1264 #define _STORE_SUBSCR_r30 1265 -#define _STORE_SUBSCR_DICT_r30 1266 +#define _STORE_SUBSCR_DICT_r31 1266 #define _STORE_SUBSCR_LIST_INT_r32 1267 #define _SWAP_r11 1268 #define _SWAP_2_r02 1269 diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 7c774e747de83e..ec47c526ff122d 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -1274,7 +1274,7 @@ const _PyUopCachingInfo _PyUop_Caching[MAX_UOP_ID+1] = { { -1, -1, -1 }, { -1, -1, -1 }, { -1, -1, -1 }, - { 0, 0, _STORE_SUBSCR_DICT_r30 }, + { 1, 0, _STORE_SUBSCR_DICT_r31 }, }, }, [_DELETE_SUBSCR] = { @@ -3499,7 +3499,7 @@ const uint16_t _PyUop_Uncached[MAX_UOP_REGS_ID+1] = { [_SET_ADD_r10] = _SET_ADD, [_STORE_SUBSCR_r30] = _STORE_SUBSCR, [_STORE_SUBSCR_LIST_INT_r32] = _STORE_SUBSCR_LIST_INT, - [_STORE_SUBSCR_DICT_r30] = _STORE_SUBSCR_DICT, + [_STORE_SUBSCR_DICT_r31] = _STORE_SUBSCR_DICT, [_DELETE_SUBSCR_r20] = _DELETE_SUBSCR, [_CALL_INTRINSIC_1_r11] = _CALL_INTRINSIC_1, [_CALL_INTRINSIC_2_r21] = _CALL_INTRINSIC_2, @@ -4873,7 +4873,7 @@ const char *const _PyOpcode_uop_name[MAX_UOP_REGS_ID+1] = { [_STORE_SUBSCR] = "_STORE_SUBSCR", [_STORE_SUBSCR_r30] = "_STORE_SUBSCR_r30", [_STORE_SUBSCR_DICT] = "_STORE_SUBSCR_DICT", - [_STORE_SUBSCR_DICT_r30] = "_STORE_SUBSCR_DICT_r30", + [_STORE_SUBSCR_DICT_r31] = "_STORE_SUBSCR_DICT_r31", [_STORE_SUBSCR_LIST_INT] = "_STORE_SUBSCR_LIST_INT", [_STORE_SUBSCR_LIST_INT_r32] = "_STORE_SUBSCR_LIST_INT_r32", [_SWAP] = "_SWAP", diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 0063c48b0b9541..e17367ca71ea38 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -2525,10 +2525,29 @@ def testfunc(n): self.assertEqual(res, 10) self.assertIsNotNone(ex) uops = get_opnames(ex) + self.assertIn("_STORE_SUBSCR_LIST_INT", uops) self.assertNotIn("_POP_TOP", uops) self.assertNotIn("_POP_TOP_INT", uops) self.assertIn("_POP_TOP_NOP", uops) + def test_store_susbscr_dict(self): + def testfunc(n): + d = {} + for _ in range(n): + d['a'] = 1 + d['b'] = 2 + d['c'] = 3 + d['d'] = 4 + return sum(d.values()) + + res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD) + self.assertEqual(res, 10) + self.assertIsNotNone(ex) + uops = get_opnames(ex) + self.assertIn("_STORE_SUBSCR_DICT", uops) + self.assertNotIn("_POP_TOP", uops) + self.assertIn("_POP_TOP_NOP", uops) + def test_attr_promotion_failure(self): # We're not testing for any specific uops here, just # testing it doesn't crash. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index ba94a4c57b1447..3e6147961f1822 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1158,9 +1158,9 @@ dummy_func( } macro(STORE_SUBSCR_DICT) = - _GUARD_NOS_DICT + unused/1 + _STORE_SUBSCR_DICT; + _GUARD_NOS_DICT + unused/1 + _STORE_SUBSCR_DICT + POP_TOP; - op(_STORE_SUBSCR_DICT, (value, dict_st, sub -- )) { + op(_STORE_SUBSCR_DICT, (value, dict_st, sub -- st)) { PyObject *dict = PyStackRef_AsPyObjectBorrow(dict_st); assert(PyDict_CheckExact(dict)); @@ -1168,8 +1168,12 @@ dummy_func( int err = _PyDict_SetItem_Take2((PyDictObject *)dict, PyStackRef_AsPyObjectSteal(sub), PyStackRef_AsPyObjectSteal(value)); - PyStackRef_CLOSE(dict_st); - ERROR_IF(err); + if (err) { + PyStackRef_CLOSE(dict_st); + ERROR_IF(1); + } + DEAD(dict_st); + st = dict_st; } inst(DELETE_SUBSCR, (container, sub --)) { diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 41b72ff562561b..e7f6bb2ed0ce0d 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -5765,12 +5765,13 @@ break; } - case _STORE_SUBSCR_DICT_r30: { + case _STORE_SUBSCR_DICT_r31: { CHECK_CURRENT_CACHED_VALUES(3); assert(WITHIN_STACK_BOUNDS_WITH_CACHE()); _PyStackRef sub; _PyStackRef dict_st; _PyStackRef value; + _PyStackRef st; _PyStackRef _stack_item_0 = _tos_cache0; _PyStackRef _stack_item_1 = _tos_cache1; _PyStackRef _stack_item_2 = _tos_cache2; @@ -5790,19 +5791,22 @@ PyStackRef_AsPyObjectSteal(sub), PyStackRef_AsPyObjectSteal(value)); stack_pointer = _PyFrame_GetStackPointer(frame); - stack_pointer += -3; - ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); - _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(dict_st); - stack_pointer = _PyFrame_GetStackPointer(frame); if (err) { + stack_pointer += -3; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_CLOSE(dict_st); + stack_pointer = _PyFrame_GetStackPointer(frame); SET_CURRENT_CACHED_VALUES(0); JUMP_TO_ERROR(); } - _tos_cache0 = PyStackRef_ZERO_BITS; + st = dict_st; + _tos_cache0 = st; _tos_cache1 = PyStackRef_ZERO_BITS; _tos_cache2 = PyStackRef_ZERO_BITS; - SET_CURRENT_CACHED_VALUES(0); + SET_CURRENT_CACHED_VALUES(1); + stack_pointer += -3; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); assert(WITHIN_STACK_BOUNDS_WITH_CACHE()); break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 006c06b67732a9..9c828cba877ad4 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -10987,6 +10987,7 @@ _PyStackRef value; _PyStackRef dict_st; _PyStackRef sub; + _PyStackRef st; // _GUARD_NOS_DICT { nos = stack_pointer[-2]; @@ -11011,14 +11012,24 @@ PyStackRef_AsPyObjectSteal(sub), PyStackRef_AsPyObjectSteal(value)); stack_pointer = _PyFrame_GetStackPointer(frame); + if (err) { + stack_pointer += -3; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_CLOSE(dict_st); + stack_pointer = _PyFrame_GetStackPointer(frame); + JUMP_TO_LABEL(error); + } + st = dict_st; + } + // _POP_TOP + { + value = st; stack_pointer += -3; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(dict_st); + PyStackRef_XCLOSE(value); stack_pointer = _PyFrame_GetStackPointer(frame); - if (err) { - JUMP_TO_LABEL(error); - } } DISPATCH(); } diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index faed3e81cbc5f4..5023f84213b359 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -109,6 +109,11 @@ dummy_func(void) { ss = sub_st; } + op(_STORE_SUBSCR_DICT, (value, dict_st, sub -- st)) { + (void)value; + st = dict_st; + } + op(_PUSH_NULL, (-- res)) { res = sym_new_null(ctx); } diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index a833367aea64af..72564ea32db772 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -1148,8 +1148,16 @@ } case _STORE_SUBSCR_DICT: { - CHECK_STACK_BOUNDS(-3); - stack_pointer += -3; + JitOptRef dict_st; + JitOptRef value; + JitOptRef st; + dict_st = stack_pointer[-2]; + value = stack_pointer[-3]; + (void)value; + st = dict_st; + CHECK_STACK_BOUNDS(-2); + stack_pointer[-3] = st; + stack_pointer += -2; 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]
