diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 926c0041c34c28..938597dadb2897 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 = 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 } } }, + [RETURN_VALUE] = { .nuops = 1, .uops = { { _POP_FRAME, 0, 0 } } }, + [RETURN_CONST] = { .nuops = 2, .uops = { { LOAD_CONST, 0, 0 }, { _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 } } }, diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-10-16-15-51-37.gh-issue-109214.-RGTFH.rst b/Misc/NEWS.d/next/Core and Builtins/2023-10-16-15-51-37.gh-issue-109214.-RGTFH.rst new file mode 100644 index 00000000000000..c24f18cee71fa0 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-10-16-15-51-37.gh-issue-109214.-RGTFH.rst @@ -0,0 +1 @@ +Remove unnecessary instruction pointer updates before returning from frames. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index d7e2ecdd24dcee..2ff3805c563de1 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -803,7 +803,6 @@ dummy_func( } macro(RETURN_VALUE) = - _SAVE_CURRENT_IP + // Sets frame->prev_instr _POP_FRAME; inst(INSTRUMENTED_RETURN_VALUE, (retval --)) { @@ -827,7 +826,6 @@ dummy_func( macro(RETURN_CONST) = LOAD_CONST + - _SAVE_CURRENT_IP + // Sets frame->prev_instr _POP_FRAME; inst(INSTRUMENTED_RETURN_CONST, (--)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 6fbe80fe03a128..506c02a6f74728 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -987,36 +987,28 @@ TARGET(RETURN_VALUE) { PyObject *retval; - // _SAVE_CURRENT_IP - { - TIER_ONE_ONLY - frame->prev_instr = next_instr - 1; - } - // _POP_FRAME retval = stack_pointer[-1]; STACK_SHRINK(1); - { - assert(EMPTY()); - #if TIER_ONE - assert(frame != &entry_frame); - #endif - STORE_SP(); - _Py_LeaveRecursiveCallPy(tstate); - // GH-99729: We need to unlink the frame *before* clearing it: - _PyInterpreterFrame *dying = frame; - frame = tstate->current_frame = dying->previous; - _PyEval_FrameClearAndPop(tstate, dying); - frame->prev_instr += frame->return_offset; - _PyFrame_StackPush(frame, retval); - LOAD_SP(); - LOAD_IP(); - #if LLTRACE && TIER_ONE - lltrace = maybe_lltrace_resume_frame(frame, &entry_frame, GLOBALS()); - if (lltrace < 0) { - goto exit_unwind; - } - #endif + assert(EMPTY()); + #if TIER_ONE + assert(frame != &entry_frame); + #endif + STORE_SP(); + _Py_LeaveRecursiveCallPy(tstate); + // GH-99729: We need to unlink the frame *before* clearing it: + _PyInterpreterFrame *dying = frame; + frame = tstate->current_frame = dying->previous; + _PyEval_FrameClearAndPop(tstate, dying); + frame->prev_instr += frame->return_offset; + _PyFrame_StackPush(frame, retval); + LOAD_SP(); + LOAD_IP(); +#if LLTRACE && TIER_ONE + lltrace = maybe_lltrace_resume_frame(frame, &entry_frame, GLOBALS()); + if (lltrace < 0) { + goto exit_unwind; } +#endif DISPATCH(); } @@ -1049,11 +1041,6 @@ value = GETITEM(FRAME_CO_CONSTS, oparg); Py_INCREF(value); } - // _SAVE_CURRENT_IP - { - TIER_ONE_ONLY - frame->prev_instr = next_instr - 1; - } // _POP_FRAME retval = value; { diff --git a/Python/optimizer.c b/Python/optimizer.c index 8d19de220d3d3d..3dfd4f3df66db0 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -701,7 +701,9 @@ translate_bytecode_to_trace( case OPARG_BOTTOM: // Second half of super-instr oparg = orig_oparg & 0xF; break; - case OPARG_SET_IP: // op==_SET_IP; oparg=next instr + case OPARG_SET_IP: // uop=_SET_IP; oparg=next_instr-1 + // The number of caches is smuggled in via offset: + assert(offset == _PyOpcode_Caches[_PyOpcode_Deopt[opcode]]); oparg = INSTR_IP(instr + offset, code); uop = _SET_IP; break; @@ -850,11 +852,7 @@ remove_unneeded_uops(_PyUOpInstruction *trace, int trace_length) bool need_ip = true; for (int pc = 0; pc < trace_length; pc++) { int opcode = trace[pc].opcode; - if (opcode == _SAVE_CURRENT_IP) { - // Special case: never remove preceding _SET_IP - last_set_ip = -1; - } - else if (opcode == _SET_IP) { + if (opcode == _SET_IP) { if (!need_ip && last_set_ip >= 0) { trace[last_set_ip].opcode = NOP; } @@ -866,8 +864,8 @@ remove_unneeded_uops(_PyUOpInstruction *trace, int trace_length) break; } else { - // If opcode has ERROR or DEOPT, set need_up to true - if (_PyOpcode_opcode_metadata[opcode].flags & (HAS_ERROR_FLAG | HAS_DEOPT_FLAG)) { + // If opcode has ERROR or DEOPT, set need_ip to true + if (_PyOpcode_opcode_metadata[opcode].flags & (HAS_ERROR_FLAG | HAS_DEOPT_FLAG) || opcode == _PUSH_FRAME) { need_ip = true; } }