Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-120619: Clean up RETURN_VALUE instruction #120624

Merged
merged 2 commits into from
Jun 17, 2024
Merged

GH-120619: Clean up RETURN_VALUE instruction #120624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a56d83
Rename _POP_FRAME to _RETURN_VALUE as it returns a value as well as p…
markshannon Jun 17, 2024
29bca8f
Remove remaining _POP_FRAMEs
markshannon Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Include/internal/pycore_opcode_metadata.h
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

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
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

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

Loading