From 059bd4d299b384d2b434ccb106f91cb4bb03bbb1 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 31 Aug 2023 11:34:52 +0100 Subject: [PATCH] GH-108614: Remove non-debug uses of `#if TIER_ONE` and `#if TIER_TWO` from `_POP_FRAME` op. (GH-108685) --- Python/bytecodes.c | 23 +++++++------- Python/ceval.c | 51 +++++++++++++++++++------------ Python/ceval_macros.h | 36 ++++++++++++++++++++++ Python/executor.c | 2 +- Python/executor_cases.c.h | 23 +++++++------- Python/generated_cases.c.h | 50 +++++++++++++++++------------- Tools/cases_generator/stacking.py | 4 +-- 7 files changed, 123 insertions(+), 66 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 93926c03421..7f398391c5d 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -768,24 +768,25 @@ dummy_func( // different frame, and it's accounted for by _PUSH_FRAME. op(_POP_FRAME, (retval --)) { assert(EMPTY()); - _PyFrame_SetStackPointer(frame, stack_pointer); - _Py_LeaveRecursiveCallPy(tstate); - // GH-99729: We need to unlink the frame *before* clearing it: - _PyInterpreterFrame *dying = frame; #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); - #if TIER_ONE - goto resume_frame; - #endif - #if TIER_TWO - stack_pointer = _PyFrame_GetStackPointer(frame); - ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; - #endif + LOAD_SP(); + LOAD_IP(); +#if LLTRACE && TIER_ONE + lltrace = maybe_lltrace_resume_frame(frame, &entry_frame, GLOBALS()); + if (lltrace < 0) { + goto exit_unwind; + } +#endif } macro(RETURN_VALUE) = diff --git a/Python/ceval.c b/Python/ceval.c index a56d31ea073..a22852ec13e 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -189,6 +189,34 @@ lltrace_resume_frame(_PyInterpreterFrame *frame) fflush(stdout); PyErr_SetRaisedException(exc); } + +static int +maybe_lltrace_resume_frame(_PyInterpreterFrame *frame, _PyInterpreterFrame *skip_frame, PyObject *globals) +{ + if (globals == NULL) { + return 0; + } + if (frame == skip_frame) { + return 0; + } + int r = PyDict_Contains(globals, &_Py_ID(__lltrace__)); + if (r < 0) { + return -1; + } + int lltrace = r; + if (!lltrace) { + // When tracing executed uops, also trace bytecode + char *uop_debug = Py_GETENV("PYTHONUOPSDEBUG"); + if (uop_debug != NULL && *uop_debug >= '0') { + lltrace = (*uop_debug - '0') >= 5; // TODO: Parse an int and all that + } + } + if (lltrace) { + lltrace_resume_frame(frame); + } + return lltrace; +} + #endif static void monitor_raise(PyThreadState *tstate, @@ -576,6 +604,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) return _PyEval_EvalFrame(tstate, f->f_frame, throwflag); } +#define TIER_ONE 1 #include "ceval_macros.h" @@ -714,24 +743,9 @@ resume_frame: SET_LOCALS_FROM_FRAME(); #ifdef LLTRACE - { - if (frame != &entry_frame && GLOBALS()) { - int r = PyDict_Contains(GLOBALS(), &_Py_ID(__lltrace__)); - if (r < 0) { - goto exit_unwind; - } - lltrace = r; - if (!lltrace) { - // When tracing executed uops, also trace bytecode - char *uop_debug = Py_GETENV("PYTHONUOPSDEBUG"); - if (uop_debug != NULL && *uop_debug >= '0') { - lltrace = (*uop_debug - '0') >= 5; // TODO: Parse an int and all that - } - } - } - if (lltrace) { - lltrace_resume_frame(frame); - } + lltrace = maybe_lltrace_resume_frame(frame, &entry_frame, GLOBALS()); + if (lltrace < 0) { + goto exit_unwind; } #endif @@ -752,7 +766,6 @@ resume_frame: #endif { -#define TIER_ONE 1 #include "generated_cases.c.h" /* INSTRUMENTED_LINE has to be here, rather than in bytecodes.c, diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h index 635b8e501e5..4b7c4448e0e 100644 --- a/Python/ceval_macros.h +++ b/Python/ceval_macros.h @@ -373,3 +373,39 @@ static inline int _Py_EnterRecursivePy(PyThreadState *tstate) { static inline void _Py_LeaveRecursiveCallPy(PyThreadState *tstate) { tstate->py_recursion_remaining++; } + + +/* Implementation of "macros" that modify the instruction pointer, + * stack pointer, or frame pointer. + * These need to treated differently by tier 1 and 2. */ + +#if TIER_ONE + +#define LOAD_IP() \ +do { next_instr = frame->prev_instr+1; } while (0) + +#define STORE_SP() \ +_PyFrame_SetStackPointer(frame, stack_pointer) + +#define LOAD_SP() \ +stack_pointer = _PyFrame_GetStackPointer(frame); + +#endif + + +#if TIER_TWO + +#define LOAD_IP() \ +do { ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; } while (0) + +#define STORE_SP() \ +_PyFrame_SetStackPointer(frame, stack_pointer) + +#define LOAD_SP() \ +stack_pointer = _PyFrame_GetStackPointer(frame); + +#endif + + + + diff --git a/Python/executor.c b/Python/executor.c index 0ff5106fe44..9b3262ed416 100644 --- a/Python/executor.c +++ b/Python/executor.c @@ -17,6 +17,7 @@ #include "pycore_sliceobject.h" #include "pycore_uops.h" +#define TIER_TWO 2 #include "ceval_macros.h" @@ -83,7 +84,6 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject OBJECT_STAT_INC(optimization_uops_executed); switch (opcode) { -#define TIER_TWO 2 #include "executor_cases.c.h" default: diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 1283cc7ebbf..e63a36d6157 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -690,24 +690,25 @@ retval = stack_pointer[-1]; STACK_SHRINK(1); assert(EMPTY()); - _PyFrame_SetStackPointer(frame, stack_pointer); - _Py_LeaveRecursiveCallPy(tstate); - // GH-99729: We need to unlink the frame *before* clearing it: - _PyInterpreterFrame *dying = frame; #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); - #if TIER_ONE - goto resume_frame; - #endif - #if TIER_TWO - stack_pointer = _PyFrame_GetStackPointer(frame); - ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; - #endif + LOAD_SP(); + LOAD_IP(); +#if LLTRACE && TIER_ONE + lltrace = maybe_lltrace_resume_frame(frame, &entry_frame, GLOBALS()); + if (lltrace < 0) { + goto exit_unwind; + } +#endif break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 5940c185817..a7bf20b9d1c 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -990,25 +990,27 @@ STACK_SHRINK(1); { assert(EMPTY()); - _PyFrame_SetStackPointer(frame, stack_pointer); - _Py_LeaveRecursiveCallPy(tstate); - // GH-99729: We need to unlink the frame *before* clearing it: - _PyInterpreterFrame *dying = frame; #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); - #if TIER_ONE - goto resume_frame; - #endif - #if TIER_TWO - stack_pointer = _PyFrame_GetStackPointer(frame); - ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; - #endif + 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(); } TARGET(INSTRUMENTED_RETURN_VALUE) { @@ -1055,25 +1057,27 @@ retval = value; { assert(EMPTY()); - _PyFrame_SetStackPointer(frame, stack_pointer); - _Py_LeaveRecursiveCallPy(tstate); - // GH-99729: We need to unlink the frame *before* clearing it: - _PyInterpreterFrame *dying = frame; #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); - #if TIER_ONE - goto resume_frame; - #endif - #if TIER_TWO - stack_pointer = _PyFrame_GetStackPointer(frame); - ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; - #endif + 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(); } TARGET(INSTRUMENTED_RETURN_CONST) { @@ -3887,6 +3891,7 @@ ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; #endif } + DISPATCH(); } TARGET(CALL_PY_EXACT_ARGS) { @@ -3963,6 +3968,7 @@ ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; #endif } + DISPATCH(); } TARGET(CALL_PY_WITH_DEFAULTS) { diff --git a/Tools/cases_generator/stacking.py b/Tools/cases_generator/stacking.py index 2d006ba8e59..dcdfc7ec054 100644 --- a/Tools/cases_generator/stacking.py +++ b/Tools/cases_generator/stacking.py @@ -369,8 +369,8 @@ def write_macro_instr( next_instr_is_set = write_components(parts, out, TIER_ONE, mac.cache_offset) except AssertionError as err: raise AssertionError(f"Error writing macro {mac.name}") from err - if not parts[-1].instr.always_exits and not next_instr_is_set: - if mac.cache_offset: + if not parts[-1].instr.always_exits: + if not next_instr_is_set and mac.cache_offset: out.emit(f"next_instr += {mac.cache_offset};") out.emit("DISPATCH();")