diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index dd1bf2d1d2b51a..965b50fac25dbc 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1353,7 +1353,7 @@ _PyOpcode_macro_expansion[256] = { [CALL_BUILTIN_O] = { .nuops = 2, .uops = { { _CALL_BUILTIN_O, OPARG_SIMPLE, 3 }, { _CHECK_PERIODIC, OPARG_SIMPLE, 3 } } }, [CALL_INTRINSIC_1] = { .nuops = 1, .uops = { { _CALL_INTRINSIC_1, OPARG_SIMPLE, 0 } } }, [CALL_INTRINSIC_2] = { .nuops = 1, .uops = { { _CALL_INTRINSIC_2, OPARG_SIMPLE, 0 } } }, - [CALL_ISINSTANCE] = { .nuops = 3, .uops = { { _GUARD_THIRD_NULL, OPARG_SIMPLE, 3 }, { _GUARD_CALLABLE_ISINSTANCE, OPARG_SIMPLE, 3 }, { _CALL_ISINSTANCE, OPARG_SIMPLE, 3 } } }, + [CALL_ISINSTANCE] = { .nuops = 6, .uops = { { _GUARD_THIRD_NULL, OPARG_SIMPLE, 3 }, { _GUARD_CALLABLE_ISINSTANCE, OPARG_SIMPLE, 3 }, { _CALL_ISINSTANCE, OPARG_SIMPLE, 3 }, { _POP_TOP, OPARG_SIMPLE, 3 }, { _POP_TOP, OPARG_SIMPLE, 3 }, { _POP_TOP, OPARG_SIMPLE, 3 } } }, [CALL_KW_BOUND_METHOD] = { .nuops = 6, .uops = { { _CHECK_PEP_523, OPARG_SIMPLE, 1 }, { _CHECK_METHOD_VERSION_KW, 2, 1 }, { _EXPAND_METHOD_KW, OPARG_SIMPLE, 3 }, { _PY_FRAME_KW, OPARG_SIMPLE, 3 }, { _SAVE_RETURN_OFFSET, OPARG_SAVE_RETURN_OFFSET, 3 }, { _PUSH_FRAME, OPARG_SIMPLE, 3 } } }, [CALL_KW_NON_PY] = { .nuops = 3, .uops = { { _CHECK_IS_NOT_PY_CALLABLE_KW, OPARG_SIMPLE, 3 }, { _CALL_KW_NON_PY, OPARG_SIMPLE, 3 }, { _CHECK_PERIODIC, OPARG_SIMPLE, 3 } } }, [CALL_KW_PY] = { .nuops = 5, .uops = { { _CHECK_PEP_523, OPARG_SIMPLE, 1 }, { _CHECK_FUNCTION_VERSION_KW, 2, 1 }, { _PY_FRAME_KW, OPARG_SIMPLE, 3 }, { _SAVE_RETURN_OFFSET, OPARG_SAVE_RETURN_OFFSET, 3 }, { _PUSH_FRAME, OPARG_SIMPLE, 3 } } }, diff --git a/Include/internal/pycore_uop_ids.h b/Include/internal/pycore_uop_ids.h index a9432401525ebb..8643727fc9b7cc 100644 --- a/Include/internal/pycore_uop_ids.h +++ b/Include/internal/pycore_uop_ids.h @@ -336,24 +336,25 @@ extern "C" { #define _SWAP 537 #define _SWAP_2 538 #define _SWAP_3 539 -#define _TIER2_RESUME_CHECK 540 -#define _TO_BOOL 541 +#define _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW 540 +#define _TIER2_RESUME_CHECK 541 +#define _TO_BOOL 542 #define _TO_BOOL_BOOL TO_BOOL_BOOL #define _TO_BOOL_INT TO_BOOL_INT -#define _TO_BOOL_LIST 542 +#define _TO_BOOL_LIST 543 #define _TO_BOOL_NONE TO_BOOL_NONE -#define _TO_BOOL_STR 543 +#define _TO_BOOL_STR 544 #define _UNARY_INVERT UNARY_INVERT #define _UNARY_NEGATIVE UNARY_NEGATIVE #define _UNARY_NOT UNARY_NOT #define _UNPACK_EX UNPACK_EX -#define _UNPACK_SEQUENCE 544 -#define _UNPACK_SEQUENCE_LIST 545 -#define _UNPACK_SEQUENCE_TUPLE 546 -#define _UNPACK_SEQUENCE_TWO_TUPLE 547 +#define _UNPACK_SEQUENCE 545 +#define _UNPACK_SEQUENCE_LIST 546 +#define _UNPACK_SEQUENCE_TUPLE 547 +#define _UNPACK_SEQUENCE_TWO_TUPLE 548 #define _WITH_EXCEPT_START WITH_EXCEPT_START #define _YIELD_VALUE YIELD_VALUE -#define MAX_UOP_ID 547 +#define MAX_UOP_ID 548 #ifdef __cplusplus } diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 52cbc2fffe484e..acd7e0a051f4f0 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -327,6 +327,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_POP_CALL_LOAD_CONST_INLINE_BORROW] = HAS_ESCAPES_FLAG, [_POP_CALL_ONE_LOAD_CONST_INLINE_BORROW] = HAS_ESCAPES_FLAG, [_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW] = HAS_ESCAPES_FLAG, + [_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW] = HAS_ESCAPES_FLAG, [_LOAD_CONST_UNDER_INLINE] = 0, [_LOAD_CONST_UNDER_INLINE_BORROW] = 0, [_CHECK_FUNCTION] = HAS_DEOPT_FLAG, @@ -648,6 +649,7 @@ const char *const _PyOpcode_uop_name[MAX_UOP_ID+1] = { [_SWAP] = "_SWAP", [_SWAP_2] = "_SWAP_2", [_SWAP_3] = "_SWAP_3", + [_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW] = "_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW", [_TIER2_RESUME_CHECK] = "_TIER2_RESUME_CHECK", [_TO_BOOL] = "_TO_BOOL", [_TO_BOOL_BOOL] = "_TO_BOOL_BOOL", @@ -1283,6 +1285,8 @@ int _PyUop_num_popped(int opcode, int oparg) return 3; case _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW: return 4; + case _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW: + return 4; case _LOAD_CONST_UNDER_INLINE: return 1; case _LOAD_CONST_UNDER_INLINE_BORROW: diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index e4c9a463855a69..3c4bd137f6819b 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -2093,6 +2093,19 @@ def testfunc(n): self.assertNotIn("_POP_CALL_ONE_LOAD_CONST_INLINE_BORROW", uops) self.assertNotIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops) + def test_call_isinstance_guards_pop_top(self): + def testfunc(n): + x = 0 + for _ in range(n): + x += isinstance(42, int) + return x + + res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD) + self.assertEqual(res, TIER2_THRESHOLD) + self.assertIsNotNone(ex) + uops = get_opnames(ex) + self.assertIn("_POP_TOP_NOP", uops) + def test_call_list_append(self): def testfunc(n): a = [] diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-28-16-56-56.gh-issue-134584.Hnd1DZ.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-28-16-56-56.gh-issue-134584.Hnd1DZ.rst new file mode 100644 index 00000000000000..2d0e3927858767 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-28-16-56-56.gh-issue-134584.Hnd1DZ.rst @@ -0,0 +1,2 @@ +Eliminate redundant refcounting from ``_CALL_ISINSTANCE``. Patch by Noam +Cohen diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 535e552e047475..4f6a51fb81c32c 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -4349,7 +4349,7 @@ dummy_func( DEOPT_IF(callable_o != interp->callable_cache.isinstance); } - op(_CALL_ISINSTANCE, (callable, null, instance, cls -- res)) { + op(_CALL_ISINSTANCE, (callable, null, instance, cls -- res, c1, i, c2)) { /* isinstance(o, o2) */ STAT_INC(CALL, hit); PyObject *inst_o = PyStackRef_AsPyObjectBorrow(instance); @@ -4358,11 +4358,10 @@ dummy_func( if (retval < 0) { ERROR_NO_POP(); } - (void)null; // Silence compiler warnings about unused variables - PyStackRef_CLOSE(cls); - PyStackRef_CLOSE(instance); - DEAD(null); - PyStackRef_CLOSE(callable); + INPUTS_DEAD(); + c1 = callable; + i = instance; + c2 = cls; res = retval ? PyStackRef_True : PyStackRef_False; assert((!PyStackRef_IsNull(res)) ^ (_PyErr_Occurred(tstate) != NULL)); } @@ -4372,7 +4371,10 @@ dummy_func( unused/2 + _GUARD_THIRD_NULL + _GUARD_CALLABLE_ISINSTANCE + - _CALL_ISINSTANCE; + _CALL_ISINSTANCE + + POP_TOP + + POP_TOP + + POP_TOP; macro(CALL_LIST_APPEND) = unused/1 + @@ -5360,6 +5362,18 @@ dummy_func( value = PyStackRef_FromPyObjectBorrow(ptr); } + tier2 op(_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW, (ptr/4, callable, null, instance, cls -- value, c1, i, c2)) { + PyStackRef_CLOSE(cls); + PyStackRef_CLOSE(instance); + (void)null; // Silence compiler warnings about unused variables + DEAD(null); + PyStackRef_CLOSE(callable); + value = PyStackRef_FromPyObjectBorrow(ptr); + c1 = callable; + i = instance; + c2 = cls; + } + tier2 op(_LOAD_CONST_UNDER_INLINE, (ptr/4, old -- value, new)) { new = old; DEAD(old); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 46fc164a5b3bc2..c420fcfef7f4d5 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -5989,12 +5989,13 @@ case _CALL_ISINSTANCE: { _PyStackRef cls; _PyStackRef instance; - _PyStackRef null; _PyStackRef callable; _PyStackRef res; + _PyStackRef c1; + _PyStackRef i; + _PyStackRef c2; cls = stack_pointer[-1]; instance = stack_pointer[-2]; - null = stack_pointer[-3]; callable = stack_pointer[-4]; STAT_INC(CALL, hit); PyObject *inst_o = PyStackRef_AsPyObjectBorrow(instance); @@ -6005,27 +6006,15 @@ if (retval < 0) { JUMP_TO_ERROR(); } - (void)null; - stack_pointer += -1; - assert(WITHIN_STACK_BOUNDS()); - _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(cls); - stack_pointer = _PyFrame_GetStackPointer(frame); - stack_pointer += -1; - assert(WITHIN_STACK_BOUNDS()); - _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(instance); - stack_pointer = _PyFrame_GetStackPointer(frame); - stack_pointer += -2; - assert(WITHIN_STACK_BOUNDS()); - _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(callable); - stack_pointer = _PyFrame_GetStackPointer(frame); + c1 = callable; + i = instance; + c2 = cls; res = retval ? PyStackRef_True : PyStackRef_False; assert((!PyStackRef_IsNull(res)) ^ (_PyErr_Occurred(tstate) != NULL)); - stack_pointer[0] = res; - stack_pointer += 1; - assert(WITHIN_STACK_BOUNDS()); + stack_pointer[-4] = res; + stack_pointer[-3] = c1; + stack_pointer[-2] = i; + stack_pointer[-1] = c2; break; } @@ -7402,6 +7391,49 @@ break; } + case _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW: { + _PyStackRef cls; + _PyStackRef instance; + _PyStackRef null; + _PyStackRef callable; + _PyStackRef value; + _PyStackRef c1; + _PyStackRef i; + _PyStackRef c2; + cls = stack_pointer[-1]; + instance = stack_pointer[-2]; + null = stack_pointer[-3]; + callable = stack_pointer[-4]; + PyObject *ptr = (PyObject *)CURRENT_OPERAND0(); + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_CLOSE(cls); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_CLOSE(instance); + stack_pointer = _PyFrame_GetStackPointer(frame); + (void)null; + stack_pointer += -2; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_CLOSE(callable); + stack_pointer = _PyFrame_GetStackPointer(frame); + value = PyStackRef_FromPyObjectBorrow(ptr); + c1 = callable; + i = instance; + c2 = cls; + stack_pointer[0] = value; + stack_pointer[1] = c1; + stack_pointer[2] = i; + stack_pointer[3] = c2; + stack_pointer += 4; + assert(WITHIN_STACK_BOUNDS()); + break; + } + case _LOAD_CONST_UNDER_INLINE: { _PyStackRef old; _PyStackRef value; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 8f7932f0033c6f..9299df11301ebc 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2784,6 +2784,10 @@ _PyStackRef instance; _PyStackRef cls; _PyStackRef res; + _PyStackRef c1; + _PyStackRef i; + _PyStackRef c2; + _PyStackRef value; /* Skip 1 cache entry */ /* Skip 2 cache entries */ // _GUARD_THIRD_NULL @@ -2819,28 +2823,42 @@ if (retval < 0) { JUMP_TO_LABEL(error); } - (void)null; + c1 = callable; + i = instance; + c2 = cls; + res = retval ? PyStackRef_True : PyStackRef_False; + assert((!PyStackRef_IsNull(res)) ^ (_PyErr_Occurred(tstate) != NULL)); + } + // _POP_TOP + { + value = c2; + stack_pointer[-4] = res; + stack_pointer[-3] = c1; + stack_pointer[-2] = i; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(cls); + PyStackRef_XCLOSE(value); stack_pointer = _PyFrame_GetStackPointer(frame); + } + // _POP_TOP + { + value = i; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(instance); + PyStackRef_XCLOSE(value); stack_pointer = _PyFrame_GetStackPointer(frame); - stack_pointer += -2; + } + // _POP_TOP + { + value = c1; + stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(callable); + PyStackRef_XCLOSE(value); stack_pointer = _PyFrame_GetStackPointer(frame); - res = retval ? PyStackRef_True : PyStackRef_False; - assert((!PyStackRef_IsNull(res)) ^ (_PyErr_Occurred(tstate) != NULL)); } - stack_pointer[0] = res; - stack_pointer += 1; - assert(WITHIN_STACK_BOUNDS()); DISPATCH(); } diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 145a8c118d3612..8660e93ba91a17 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -567,6 +567,7 @@ const uint16_t op_without_push[MAX_UOP_ID + 1] = { [_POP_TOP_LOAD_CONST_INLINE_BORROW] = _POP_TOP, [_POP_TWO_LOAD_CONST_INLINE_BORROW] = _POP_TWO, [_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW] = _POP_CALL_TWO, + [_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW] = _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW, }; const bool op_skip[MAX_UOP_ID + 1] = { diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index f8a0484bdc2b04..c091bae0392cd9 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -567,6 +567,13 @@ dummy_func(void) { value = PyJitRef_Borrow(sym_new_const(ctx, ptr)); } + op(_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW, (ptr/4, callable, null, instance, cls -- value, c1, i, c2)) { + value = PyJitRef_Borrow(sym_new_const(ctx, ptr)); + c1 = callable; + i = instance; + c2 = cls; + } + op(_POP_TOP, (value -- )) { PyTypeObject *typ = sym_get_type(value); if (PyJitRef_IsBorrowed(value) || @@ -997,7 +1004,7 @@ dummy_func(void) { } } - op(_CALL_ISINSTANCE, (unused, unused, instance, cls -- res)) { + op(_CALL_ISINSTANCE, (unused, unused, instance, cls -- res, unused, unused, unused)) { // the result is always a bool, but sometimes we can // narrow it down to True or False res = sym_new_type(ctx, &PyBool_Type); @@ -1013,7 +1020,7 @@ dummy_func(void) { out = Py_True; } sym_set_const(res, out); - REPLACE_OP(this_instr, _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out); + REPLACE_OP(this_instr, _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out); } } diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 10767ccdbd57f5..0827fb874c5308 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -2341,11 +2341,9 @@ out = Py_True; } sym_set_const(res, out); - REPLACE_OP(this_instr, _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out); + REPLACE_OP(this_instr, _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out); } stack_pointer[-4] = res; - stack_pointer += -3; - assert(WITHIN_STACK_BOUNDS()); break; } @@ -2797,6 +2795,29 @@ break; } + case _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW: { + JitOptRef cls; + JitOptRef instance; + JitOptRef callable; + JitOptRef value; + JitOptRef c1; + JitOptRef i; + JitOptRef c2; + cls = stack_pointer[-1]; + instance = stack_pointer[-2]; + callable = stack_pointer[-4]; + PyObject *ptr = (PyObject *)this_instr->operand0; + value = PyJitRef_Borrow(sym_new_const(ctx, ptr)); + c1 = callable; + i = instance; + c2 = cls; + stack_pointer[-4] = value; + stack_pointer[-3] = c1; + stack_pointer[-2] = i; + stack_pointer[-1] = c2; + break; + } + case _LOAD_CONST_UNDER_INLINE: { JitOptRef value; JitOptRef new;