From 19b7ead5eb2fd1a0d19403e800a6f3adffbaac69 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 12 Oct 2023 10:34:32 +0100 Subject: [PATCH] GH-109214: Convert _SAVE_CURRENT_IP to _SET_IP in tier 2 trace creation. (GH-110755) --- Include/internal/pycore_opcode_metadata.h | 8 +++---- Python/abstract_interp_cases.c.h | 4 ---- Python/bytecodes.c | 12 ++-------- Python/ceval_macros.h | 3 +++ Python/executor_cases.c.h | 12 +--------- Python/generated_cases.c.h | 28 ++++------------------- Python/optimizer.c | 8 ++++--- Tools/cases_generator/generate_cases.py | 6 ++--- 8 files changed, 22 insertions(+), 59 deletions(-) diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 8ef398c5db0..926c0041c34 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1644,8 +1644,8 @@ const struct opcode_macro_expansion _PyOpcode_macro_expansion[OPCODE_MACRO_EXPAN [DELETE_SUBSCR] = { .nuops = 1, .uops = { { DELETE_SUBSCR, 0, 0 } } }, [CALL_INTRINSIC_1] = { .nuops = 1, .uops = { { CALL_INTRINSIC_1, 0, 0 } } }, [CALL_INTRINSIC_2] = { .nuops = 1, .uops = { { CALL_INTRINSIC_2, 0, 0 } } }, - [RETURN_VALUE] = { .nuops = 3, .uops = { { _SET_IP, 7, 0 }, { _SAVE_CURRENT_IP, 0, 0 }, { _POP_FRAME, 0, 0 } } }, - [RETURN_CONST] = { .nuops = 4, .uops = { { LOAD_CONST, 0, 0 }, { _SET_IP, 7, 0 }, { _SAVE_CURRENT_IP, 0, 0 }, { _POP_FRAME, 0, 0 } } }, + [RETURN_VALUE] = { .nuops = 2, .uops = { { _SAVE_CURRENT_IP, 7, -1 }, { _POP_FRAME, 0, 0 } } }, + [RETURN_CONST] = { .nuops = 3, .uops = { { LOAD_CONST, 0, 0 }, { _SAVE_CURRENT_IP, 7, -1 }, { _POP_FRAME, 0, 0 } } }, [GET_AITER] = { .nuops = 1, .uops = { { GET_AITER, 0, 0 } } }, [GET_ANEXT] = { .nuops = 1, .uops = { { GET_ANEXT, 0, 0 } } }, [GET_AWAITABLE] = { .nuops = 1, .uops = { { GET_AWAITABLE, 0, 0 } } }, @@ -1719,8 +1719,8 @@ const struct opcode_macro_expansion _PyOpcode_macro_expansion[OPCODE_MACRO_EXPAN [LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES] = { .nuops = 4, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _GUARD_DORV_VALUES_INST_ATTR_FROM_DICT, 0, 0 }, { _GUARD_KEYS_VERSION, 2, 3 }, { _LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES, 4, 5 } } }, [LOAD_ATTR_NONDESCRIPTOR_NO_DICT] = { .nuops = 2, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _LOAD_ATTR_NONDESCRIPTOR_NO_DICT, 4, 5 } } }, [LOAD_ATTR_METHOD_LAZY_DICT] = { .nuops = 3, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _CHECK_ATTR_METHOD_LAZY_DICT, 0, 0 }, { _LOAD_ATTR_METHOD_LAZY_DICT, 4, 5 } } }, - [CALL_BOUND_METHOD_EXACT_ARGS] = { .nuops = 9, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _INIT_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SET_IP, 7, 3 }, { _SAVE_CURRENT_IP, 0, 0 }, { _PUSH_FRAME, 0, 0 } } }, - [CALL_PY_EXACT_ARGS] = { .nuops = 7, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SET_IP, 7, 3 }, { _SAVE_CURRENT_IP, 0, 0 }, { _PUSH_FRAME, 0, 0 } } }, + [CALL_BOUND_METHOD_EXACT_ARGS] = { .nuops = 8, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _INIT_CALL_BOUND_METHOD_EXACT_ARGS, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SAVE_CURRENT_IP, 7, 2 }, { _PUSH_FRAME, 0, 0 } } }, + [CALL_PY_EXACT_ARGS] = { .nuops = 6, .uops = { { _CHECK_PEP_523, 0, 0 }, { _CHECK_FUNCTION_EXACT_ARGS, 2, 1 }, { _CHECK_STACK_SPACE, 0, 0 }, { _INIT_CALL_PY_EXACT_ARGS, 0, 0 }, { _SAVE_CURRENT_IP, 7, 2 }, { _PUSH_FRAME, 0, 0 } } }, [CALL_TYPE_1] = { .nuops = 1, .uops = { { CALL_TYPE_1, 0, 0 } } }, [CALL_STR_1] = { .nuops = 1, .uops = { { CALL_STR_1, 0, 0 } } }, [CALL_TUPLE_1] = { .nuops = 1, .uops = { { CALL_TUPLE_1, 0, 0 } } }, diff --git a/Python/abstract_interp_cases.c.h b/Python/abstract_interp_cases.c.h index 04fe07fad39..44115da8629 100644 --- a/Python/abstract_interp_cases.c.h +++ b/Python/abstract_interp_cases.c.h @@ -920,10 +920,6 @@ break; } - case _SAVE_CURRENT_IP: { - break; - } - case _EXIT_TRACE: { break; } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 9b733ce4a8c..62dc548abd6 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -803,7 +803,6 @@ dummy_func( } macro(RETURN_VALUE) = - _SET_IP + // Tier 2 only; special-cased oparg _SAVE_CURRENT_IP + // Sets frame->prev_instr _POP_FRAME; @@ -828,7 +827,6 @@ dummy_func( macro(RETURN_CONST) = LOAD_CONST + - _SET_IP + // Tier 2 only; special-cased oparg _SAVE_CURRENT_IP + // Sets frame->prev_instr _POP_FRAME; @@ -3099,7 +3097,6 @@ dummy_func( _CHECK_FUNCTION_EXACT_ARGS + _CHECK_STACK_SPACE + _INIT_CALL_PY_EXACT_ARGS + - _SET_IP + // Tier 2 only; special-cased oparg _SAVE_CURRENT_IP + // Sets frame->prev_instr _PUSH_FRAME; @@ -3109,7 +3106,6 @@ dummy_func( _CHECK_FUNCTION_EXACT_ARGS + _CHECK_STACK_SPACE + _INIT_CALL_PY_EXACT_ARGS + - _SET_IP + // Tier 2 only; special-cased oparg _SAVE_CURRENT_IP + // Sets frame->prev_instr _PUSH_FRAME; @@ -3948,17 +3944,13 @@ dummy_func( } op(_SET_IP, (--)) { + TIER_TWO_ONLY frame->prev_instr = ip_offset + oparg; } op(_SAVE_CURRENT_IP, (--)) { - #if TIER_ONE + TIER_ONE_ONLY frame->prev_instr = next_instr - 1; - #endif - #if TIER_TWO - // Relies on a preceding _SET_IP - frame->prev_instr--; - #endif } op(_EXIT_TRACE, (--)) { diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h index 872e0a2b7f9..bd28126b0b7 100644 --- a/Python/ceval_macros.h +++ b/Python/ceval_macros.h @@ -372,6 +372,9 @@ static inline void _Py_LeaveRecursiveCallPy(PyThreadState *tstate) { /* Marker to specify tier 1 only instructions */ #define TIER_ONE_ONLY +/* Marker to specify tier 2 only instructions */ +#define TIER_TWO_ONLY + /* Implementation of "macros" that modify the instruction pointer, * stack pointer, or frame pointer. * These need to treated differently by tier 1 and 2. */ diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index e2f4f9805b7..119e77b9369 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -3270,21 +3270,11 @@ } case _SET_IP: { + TIER_TWO_ONLY frame->prev_instr = ip_offset + oparg; break; } - case _SAVE_CURRENT_IP: { - #if TIER_ONE - frame->prev_instr = next_instr - 1; - #endif - #if TIER_TWO - // Relies on a preceding _SET_IP - frame->prev_instr--; - #endif - break; - } - case _EXIT_TRACE: { frame->prev_instr--; // Back up to just before destination _PyFrame_SetStackPointer(frame, stack_pointer); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index eac136846b1..8ae9bd2f457 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -989,13 +989,8 @@ PyObject *retval; // _SAVE_CURRENT_IP { - #if TIER_ONE + TIER_ONE_ONLY frame->prev_instr = next_instr - 1; - #endif - #if TIER_TWO - // Relies on a preceding _SET_IP - frame->prev_instr--; - #endif } // _POP_FRAME retval = stack_pointer[-1]; @@ -1056,13 +1051,8 @@ } // _SAVE_CURRENT_IP { - #if TIER_ONE + TIER_ONE_ONLY frame->prev_instr = next_instr - 1; - #endif - #if TIER_TWO - // Relies on a preceding _SET_IP - frame->prev_instr--; - #endif } // _POP_FRAME retval = value; @@ -3941,13 +3931,8 @@ // _SAVE_CURRENT_IP next_instr += 3; { - #if TIER_ONE + TIER_ONE_ONLY frame->prev_instr = next_instr - 1; - #endif - #if TIER_TWO - // Relies on a preceding _SET_IP - frame->prev_instr--; - #endif } // _PUSH_FRAME STACK_SHRINK(oparg); @@ -4019,13 +4004,8 @@ // _SAVE_CURRENT_IP next_instr += 3; { - #if TIER_ONE + TIER_ONE_ONLY frame->prev_instr = next_instr - 1; - #endif - #if TIER_TWO - // Relies on a preceding _SET_IP - frame->prev_instr--; - #endif } // _PUSH_FRAME STACK_SHRINK(oparg); diff --git a/Python/optimizer.c b/Python/optimizer.c index 65b9638be25..955ac812177 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -648,6 +648,7 @@ pop_jump_if_bool: uint32_t orig_oparg = oparg; // For OPARG_TOP/BOTTOM for (int i = 0; i < nuops; i++) { oparg = orig_oparg; + uint32_t uop = expansion->uops[i].uop; uint64_t operand = 0; // Add one to account for the actual opcode/oparg pair: int offset = expansion->uops[i].offset + 1; @@ -680,6 +681,7 @@ pop_jump_if_bool: break; case OPARG_SET_IP: // op==_SET_IP; oparg=next instr oparg = INSTR_IP(instr + offset, code); + uop = _SET_IP; break; default: @@ -690,8 +692,8 @@ pop_jump_if_bool: expansion->uops[i].offset); Py_FatalError("garbled expansion"); } - ADD_TO_TRACE(expansion->uops[i].uop, oparg, operand); - if (expansion->uops[i].uop == _POP_FRAME) { + ADD_TO_TRACE(uop, oparg, operand); + if (uop == _POP_FRAME) { TRACE_STACK_POP(); DPRINTF(2, "Returning to %s (%s:%d) at byte offset %d\n", @@ -701,7 +703,7 @@ pop_jump_if_bool: 2 * INSTR_IP(instr, code)); goto top; } - if (expansion->uops[i].uop == _PUSH_FRAME) { + if (uop == _PUSH_FRAME) { assert(i + 1 == nuops); int func_version_offset = offsetof(_PyCallCache, func_version)/sizeof(_Py_CODEUNIT) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index dbb16418c0c..04d617ba9f5 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -658,7 +658,7 @@ class Generator(Analyzer): for part in parts: if isinstance(part, Component): # All component instructions must be viable uops - if not part.instr.is_viable_uop(): + if not part.instr.is_viable_uop() and part.instr.name != "_SAVE_CURRENT_IP": # This note just reminds us about macros that cannot # be expanded to Tier 2 uops. It is not an error. # It is sometimes emitted for macros that have a @@ -671,8 +671,8 @@ class Generator(Analyzer): ) return if not part.active_caches: - if part.instr.name == "_SET_IP": - size, offset = OPARG_SIZES["OPARG_SET_IP"], cache_offset + if part.instr.name == "_SAVE_CURRENT_IP": + size, offset = OPARG_SIZES["OPARG_SET_IP"], cache_offset - 1 else: size, offset = OPARG_SIZES["OPARG_FULL"], 0 else: