From 9a56d83f84edc1da610ecdc496a4a3a05465a62c Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 17 Jun 2024 10:50:17 +0100 Subject: [PATCH 1/2] Rename _POP_FRAME to _RETURN_VALUE as it returns a value as well as popping a frame. --- Include/internal/pycore_opcode_metadata.h | 8 ++-- Include/internal/pycore_uop_ids.h | 56 +++++++++++------------ Include/internal/pycore_uop_metadata.h | 6 +-- Python/bytecodes.c | 9 ++-- Python/executor_cases.c.h | 7 ++- Python/generated_cases.c.h | 12 +++-- Python/optimizer.c | 8 ++-- Python/optimizer_analysis.c | 2 +- Python/optimizer_bytecodes.c | 2 +- Python/optimizer_cases.c.h | 2 +- 10 files changed, 59 insertions(+), 53 deletions(-) diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 0b835230974e39..1e9c61953d86d8 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -831,11 +831,11 @@ int _PyOpcode_num_pushed(int opcode, int oparg) { case RESUME_CHECK: return 0; case RETURN_CONST: - return 0; + return 1; case RETURN_GENERATOR: return 1; case RETURN_VALUE: - return 0; + return 1; case SEND: return 2; case SEND_GEN: @@ -1346,9 +1346,9 @@ _PyOpcode_macro_expansion[256] = { [PUSH_EXC_INFO] = { .nuops = 1, .uops = { { _PUSH_EXC_INFO, 0, 0 } } }, [PUSH_NULL] = { .nuops = 1, .uops = { { _PUSH_NULL, 0, 0 } } }, [RESUME_CHECK] = { .nuops = 1, .uops = { { _RESUME_CHECK, 0, 0 } } }, - [RETURN_CONST] = { .nuops = 2, .uops = { { _LOAD_CONST, 0, 0 }, { _POP_FRAME, 0, 0 } } }, + [RETURN_CONST] = { .nuops = 2, .uops = { { _LOAD_CONST, 0, 0 }, { _RETURN_VALUE, 0, 0 } } }, [RETURN_GENERATOR] = { .nuops = 1, .uops = { { _RETURN_GENERATOR, 0, 0 } } }, - [RETURN_VALUE] = { .nuops = 1, .uops = { { _POP_FRAME, 0, 0 } } }, + [RETURN_VALUE] = { .nuops = 1, .uops = { { _RETURN_VALUE, 0, 0 } } }, [SETUP_ANNOTATIONS] = { .nuops = 1, .uops = { { _SETUP_ANNOTATIONS, 0, 0 } } }, [SET_ADD] = { .nuops = 1, .uops = { { _SET_ADD, 0, 0 } } }, [SET_FUNCTION_ATTRIBUTE] = { .nuops = 1, .uops = { { _SET_FUNCTION_ATTRIBUTE, 0, 0 } } }, diff --git a/Include/internal/pycore_uop_ids.h b/Include/internal/pycore_uop_ids.h index 19e2b823ed0140..e824d510bf207e 100644 --- a/Include/internal/pycore_uop_ids.h +++ b/Include/internal/pycore_uop_ids.h @@ -226,51 +226,51 @@ extern "C" { #define _MATCH_SEQUENCE MATCH_SEQUENCE #define _NOP NOP #define _POP_EXCEPT POP_EXCEPT -#define _POP_FRAME 426 -#define _POP_JUMP_IF_FALSE 427 -#define _POP_JUMP_IF_TRUE 428 +#define _POP_JUMP_IF_FALSE 426 +#define _POP_JUMP_IF_TRUE 427 #define _POP_TOP POP_TOP -#define _POP_TOP_LOAD_CONST_INLINE_BORROW 429 +#define _POP_TOP_LOAD_CONST_INLINE_BORROW 428 #define _PUSH_EXC_INFO PUSH_EXC_INFO -#define _PUSH_FRAME 430 +#define _PUSH_FRAME 429 #define _PUSH_NULL PUSH_NULL -#define _PY_FRAME_GENERAL 431 -#define _REPLACE_WITH_TRUE 432 +#define _PY_FRAME_GENERAL 430 +#define _REPLACE_WITH_TRUE 431 #define _RESUME_CHECK RESUME_CHECK #define _RETURN_GENERATOR RETURN_GENERATOR -#define _SAVE_RETURN_OFFSET 433 -#define _SEND 434 +#define _RETURN_VALUE RETURN_VALUE +#define _SAVE_RETURN_OFFSET 432 +#define _SEND 433 #define _SEND_GEN SEND_GEN #define _SETUP_ANNOTATIONS SETUP_ANNOTATIONS #define _SET_ADD SET_ADD #define _SET_FUNCTION_ATTRIBUTE SET_FUNCTION_ATTRIBUTE #define _SET_UPDATE SET_UPDATE -#define _START_EXECUTOR 435 -#define _STORE_ATTR 436 -#define _STORE_ATTR_INSTANCE_VALUE 437 -#define _STORE_ATTR_SLOT 438 -#define _STORE_ATTR_WITH_HINT 439 +#define _START_EXECUTOR 434 +#define _STORE_ATTR 435 +#define _STORE_ATTR_INSTANCE_VALUE 436 +#define _STORE_ATTR_SLOT 437 +#define _STORE_ATTR_WITH_HINT 438 #define _STORE_DEREF STORE_DEREF -#define _STORE_FAST 440 -#define _STORE_FAST_0 441 -#define _STORE_FAST_1 442 -#define _STORE_FAST_2 443 -#define _STORE_FAST_3 444 -#define _STORE_FAST_4 445 -#define _STORE_FAST_5 446 -#define _STORE_FAST_6 447 -#define _STORE_FAST_7 448 +#define _STORE_FAST 439 +#define _STORE_FAST_0 440 +#define _STORE_FAST_1 441 +#define _STORE_FAST_2 442 +#define _STORE_FAST_3 443 +#define _STORE_FAST_4 444 +#define _STORE_FAST_5 445 +#define _STORE_FAST_6 446 +#define _STORE_FAST_7 447 #define _STORE_FAST_LOAD_FAST STORE_FAST_LOAD_FAST #define _STORE_FAST_STORE_FAST STORE_FAST_STORE_FAST #define _STORE_GLOBAL STORE_GLOBAL #define _STORE_NAME STORE_NAME #define _STORE_SLICE STORE_SLICE -#define _STORE_SUBSCR 449 +#define _STORE_SUBSCR 448 #define _STORE_SUBSCR_DICT STORE_SUBSCR_DICT #define _STORE_SUBSCR_LIST_INT STORE_SUBSCR_LIST_INT #define _SWAP SWAP -#define _TIER2_RESUME_CHECK 450 -#define _TO_BOOL 451 +#define _TIER2_RESUME_CHECK 449 +#define _TO_BOOL 450 #define _TO_BOOL_BOOL TO_BOOL_BOOL #define _TO_BOOL_INT TO_BOOL_INT #define _TO_BOOL_LIST TO_BOOL_LIST @@ -280,13 +280,13 @@ extern "C" { #define _UNARY_NEGATIVE UNARY_NEGATIVE #define _UNARY_NOT UNARY_NOT #define _UNPACK_EX UNPACK_EX -#define _UNPACK_SEQUENCE 452 +#define _UNPACK_SEQUENCE 451 #define _UNPACK_SEQUENCE_LIST UNPACK_SEQUENCE_LIST #define _UNPACK_SEQUENCE_TUPLE UNPACK_SEQUENCE_TUPLE #define _UNPACK_SEQUENCE_TWO_TUPLE UNPACK_SEQUENCE_TWO_TUPLE #define _WITH_EXCEPT_START WITH_EXCEPT_START #define _YIELD_VALUE YIELD_VALUE -#define MAX_UOP_ID 452 +#define MAX_UOP_ID 451 #ifdef __cplusplus } diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 690ae34a6eef98..94a82ad018f389 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -87,7 +87,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_DELETE_SUBSCR] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_CALL_INTRINSIC_1] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_CALL_INTRINSIC_2] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, - [_POP_FRAME] = 0, + [_RETURN_VALUE] = 0, [_GET_AITER] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_GET_ANEXT] = HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, [_GET_AWAITABLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, @@ -453,7 +453,6 @@ const char *const _PyOpcode_uop_name[MAX_UOP_ID+1] = { [_MATCH_SEQUENCE] = "_MATCH_SEQUENCE", [_NOP] = "_NOP", [_POP_EXCEPT] = "_POP_EXCEPT", - [_POP_FRAME] = "_POP_FRAME", [_POP_TOP] = "_POP_TOP", [_POP_TOP_LOAD_CONST_INLINE_BORROW] = "_POP_TOP_LOAD_CONST_INLINE_BORROW", [_PUSH_EXC_INFO] = "_PUSH_EXC_INFO", @@ -463,6 +462,7 @@ const char *const _PyOpcode_uop_name[MAX_UOP_ID+1] = { [_REPLACE_WITH_TRUE] = "_REPLACE_WITH_TRUE", [_RESUME_CHECK] = "_RESUME_CHECK", [_RETURN_GENERATOR] = "_RETURN_GENERATOR", + [_RETURN_VALUE] = "_RETURN_VALUE", [_SAVE_RETURN_OFFSET] = "_SAVE_RETURN_OFFSET", [_SETUP_ANNOTATIONS] = "_SETUP_ANNOTATIONS", [_SET_ADD] = "_SET_ADD", @@ -650,7 +650,7 @@ int _PyUop_num_popped(int opcode, int oparg) return 1; case _CALL_INTRINSIC_2: return 2; - case _POP_FRAME: + case _RETURN_VALUE: return 1; case _GET_AITER: return 1; diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 05c17ac334b69f..31db28426006fe 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -827,7 +827,7 @@ dummy_func( // We definitely pop the return value off the stack on entry. // We also push it onto the stack on exit, but that's a // different frame, and it's accounted for by _PUSH_FRAME. - op(_POP_FRAME, (retval --)) { + inst(RETURN_VALUE, (retval -- res)) { #if TIER_ONE assert(frame != &entry_frame); #endif @@ -839,15 +839,12 @@ dummy_func( _PyInterpreterFrame *dying = frame; frame = tstate->current_frame = dying->previous; _PyEval_FrameClearAndPop(tstate, dying); - _PyFrame_StackPush(frame, retval); LOAD_SP(); LOAD_IP(frame->return_offset); + res = retval; LLTRACE_RESUME_FRAME(); } - macro(RETURN_VALUE) = - _POP_FRAME; - inst(INSTRUMENTED_RETURN_VALUE, (retval --)) { int err = _Py_call_instrumentation_arg( tstate, PY_MONITORING_EVENT_PY_RETURN, @@ -869,7 +866,7 @@ dummy_func( macro(RETURN_CONST) = LOAD_CONST + - _POP_FRAME; + RETURN_VALUE; inst(INSTRUMENTED_RETURN_CONST, (--)) { PyObject *retval = GETITEM(FRAME_CO_CONSTS, oparg); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 470c82d938ab7c..d390c9fc2f6ed6 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -971,8 +971,9 @@ break; } - case _POP_FRAME: { + case _RETURN_VALUE: { PyObject *retval; + PyObject *res; retval = stack_pointer[-1]; #if TIER_ONE assert(frame != &entry_frame); @@ -985,10 +986,12 @@ _PyInterpreterFrame *dying = frame; frame = tstate->current_frame = dying->previous; _PyEval_FrameClearAndPop(tstate, dying); - _PyFrame_StackPush(frame, retval); LOAD_SP(); LOAD_IP(frame->return_offset); + res = retval; LLTRACE_RESUME_FRAME(); + stack_pointer[0] = res; + stack_pointer += 1; break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 0274f8b7a48c3c..8a6f5ff784f58d 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5202,12 +5202,13 @@ INSTRUCTION_STATS(RETURN_CONST); PyObject *value; PyObject *retval; + PyObject *res; // _LOAD_CONST { value = GETITEM(FRAME_CO_CONSTS, oparg); Py_INCREF(value); } - // _POP_FRAME + // _RETURN_VALUE retval = value; { #if TIER_ONE @@ -5220,11 +5221,13 @@ _PyInterpreterFrame *dying = frame; frame = tstate->current_frame = dying->previous; _PyEval_FrameClearAndPop(tstate, dying); - _PyFrame_StackPush(frame, retval); LOAD_SP(); LOAD_IP(frame->return_offset); + res = retval; LLTRACE_RESUME_FRAME(); } + stack_pointer[0] = res; + stack_pointer += 1; DISPATCH(); } @@ -5265,6 +5268,7 @@ next_instr += 1; INSTRUCTION_STATS(RETURN_VALUE); PyObject *retval; + PyObject *res; retval = stack_pointer[-1]; #if TIER_ONE assert(frame != &entry_frame); @@ -5277,10 +5281,12 @@ _PyInterpreterFrame *dying = frame; frame = tstate->current_frame = dying->previous; _PyEval_FrameClearAndPop(tstate, dying); - _PyFrame_StackPush(frame, retval); LOAD_SP(); LOAD_IP(frame->return_offset); + res = retval; LLTRACE_RESUME_FRAME(); + stack_pointer[0] = res; + stack_pointer += 1; DISPATCH(); } diff --git a/Python/optimizer.c b/Python/optimizer.c index 4dc3438b6c23a4..c9b187d2e108dd 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -537,7 +537,7 @@ add_to_trace( // Reserve space for N uops, plus 3 for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE #define RESERVE(needed) RESERVE_RAW((needed) + 3, _PyUOpName(opcode)) -// Trace stack operations (used by _PUSH_FRAME, _POP_FRAME) +// Trace stack operations (used by _PUSH_FRAME, _RETURN_VALUE) #define TRACE_STACK_PUSH() \ if (trace_stack_depth >= TRACE_STACK_SIZE) { \ DPRINTF(2, "Trace stack overflow\n"); \ @@ -748,10 +748,10 @@ translate_bytecode_to_trace( int nuops = expansion->nuops; RESERVE(nuops + 1); /* One extra for exit */ int16_t last_op = expansion->uops[nuops-1].uop; - if (last_op == _POP_FRAME || last_op == _RETURN_GENERATOR || last_op == _YIELD_VALUE) { + if (last_op == _RETURN_VALUE || last_op == _RETURN_GENERATOR || last_op == _YIELD_VALUE) { // Check for trace stack underflow now: // We can't bail e.g. in the middle of - // LOAD_CONST + _POP_FRAME. + // LOAD_CONST + _RETURN_VALUE. if (trace_stack_depth == 0) { DPRINTF(2, "Trace stack underflow\n"); OPT_STAT_INC(trace_stack_underflow); @@ -810,7 +810,7 @@ translate_bytecode_to_trace( Py_FatalError("garbled expansion"); } - if (uop == _POP_FRAME || uop == _RETURN_GENERATOR || uop == _YIELD_VALUE) { + if (uop == _RETURN_VALUE || uop == _RETURN_GENERATOR || uop == _YIELD_VALUE) { TRACE_STACK_POP(); /* Set the operand to the function or code object returned to, * to assist optimization passes. (See _PUSH_FRAME below.) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 75d1d9f6b2a794..b603dd62062664 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -265,7 +265,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer, } break; } - case _POP_FRAME: + case _RETURN_VALUE: { builtins_watched >>= 1; globals_watched >>= 1; diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index e6fb85a90603eb..121ca928fed946 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -606,7 +606,7 @@ dummy_func(void) { ctx->done = true; } - op(_POP_FRAME, (retval -- res)) { + op(_RETURN_VALUE, (retval -- res)) { SYNC_SP(); ctx->frame->stack_pointer = stack_pointer; frame_pop(ctx); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 18f3ca4cb73e5a..53959a39b0b067 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -558,7 +558,7 @@ break; } - case _POP_FRAME: { + case _RETURN_VALUE: { _Py_UopsSymbol *retval; _Py_UopsSymbol *res; retval = stack_pointer[-1]; From 29bca8f7bc6d4aa5efe0997b32713dcda489a234 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 17 Jun 2024 12:11:43 +0100 Subject: [PATCH 2/2] Remove remaining _POP_FRAMEs --- Lib/test/test_capi/test_opt.py | 12 ++++++------ Python/optimizer_analysis.c | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index fc6d8b0a3f01d2..328b6424772061 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -1024,7 +1024,7 @@ def testfunc(n): uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex] uop_names = [uop[0] for uop in uops_and_operands] self.assertEqual(uop_names.count("_PUSH_FRAME"), 2) - self.assertEqual(uop_names.count("_POP_FRAME"), 2) + self.assertEqual(uop_names.count("_RETURN_VALUE"), 2) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1) # sequential calls: max(12, 13) == 13 @@ -1051,7 +1051,7 @@ def testfunc(n): uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex] uop_names = [uop[0] for uop in uops_and_operands] self.assertEqual(uop_names.count("_PUSH_FRAME"), 2) - self.assertEqual(uop_names.count("_POP_FRAME"), 2) + self.assertEqual(uop_names.count("_RETURN_VALUE"), 2) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1) # nested calls: 15 + 12 == 27 @@ -1086,7 +1086,7 @@ def testfunc(n): uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex] uop_names = [uop[0] for uop in uops_and_operands] self.assertEqual(uop_names.count("_PUSH_FRAME"), 4) - self.assertEqual(uop_names.count("_POP_FRAME"), 4) + self.assertEqual(uop_names.count("_RETURN_VALUE"), 4) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1) # max(12, 18 + max(12, 13)) == 31 @@ -1122,7 +1122,7 @@ def testfunc(n): uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex] uop_names = [uop[0] for uop in uops_and_operands] self.assertEqual(uop_names.count("_PUSH_FRAME"), 4) - self.assertEqual(uop_names.count("_POP_FRAME"), 4) + self.assertEqual(uop_names.count("_RETURN_VALUE"), 4) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1) # max(18 + max(12, 13), 12) == 31 @@ -1166,7 +1166,7 @@ def testfunc(n): uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex] uop_names = [uop[0] for uop in uops_and_operands] self.assertEqual(uop_names.count("_PUSH_FRAME"), 15) - self.assertEqual(uop_names.count("_POP_FRAME"), 15) + self.assertEqual(uop_names.count("_RETURN_VALUE"), 15) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1) @@ -1260,7 +1260,7 @@ def testfunc(n): uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex] uop_names = [uop[0] for uop in uops_and_operands] self.assertEqual(uop_names.count("_PUSH_FRAME"), 2) - self.assertEqual(uop_names.count("_POP_FRAME"), 0) + self.assertEqual(uop_names.count("_RETURN_VALUE"), 0) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 1) self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1) largest_stack = _testinternalcapi.get_co_framesize(dummy15.__code__) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index b603dd62062664..0e45bd8e31a54d 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -365,13 +365,13 @@ eliminate_pop_guard(_PyUOpInstruction *this_instr, bool exit) } } -/* _PUSH_FRAME/_POP_FRAME's operand can be 0, a PyFunctionObject *, or a +/* _PUSH_FRAME/_RETURN_VALUE's operand can be 0, a PyFunctionObject *, or a * PyCodeObject *. Retrieve the code object if possible. */ static PyCodeObject * get_code(_PyUOpInstruction *op) { - assert(op->opcode == _PUSH_FRAME || op->opcode == _POP_FRAME || op->opcode == _RETURN_GENERATOR); + assert(op->opcode == _PUSH_FRAME || op->opcode == _RETURN_VALUE || op->opcode == _RETURN_GENERATOR); PyCodeObject *co = NULL; uint64_t operand = op->operand; if (operand == 0) {