Skip to content

Commit

Permalink
pythonGH-120619: Clean up RETURN_VALUE instruction (pythonGH-120624)
Browse files Browse the repository at this point in the history
* Rename _POP_FRAME to _RETURN_VALUE as it returns a value as well as popping a frame.

* Remove remaining _POP_FRAMEs
  • Loading branch information
markshannon authored and mrahtz committed Jun 30, 2024
1 parent 404fc99 commit cd55103
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 61 deletions.
8 changes: 4 additions & 4 deletions Include/internal/pycore_opcode_metadata.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 28 additions & 28 deletions Include/internal/pycore_uop_ids.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Include/internal/pycore_uop_metadata.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions Lib/test/test_capi/test_opt.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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__)
Expand Down
9 changes: 3 additions & 6 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions Python/executor_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions Python/optimizer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"); \
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.)
Expand Down
6 changes: 3 additions & 3 deletions Python/optimizer_analysis.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
}
break;
}
case _POP_FRAME:
case _RETURN_VALUE:
{
builtins_watched >>= 1;
globals_watched >>= 1;
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion Python/optimizer_bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Python/optimizer_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit cd55103

Please sign in to comment.