Skip to content

Commit 274f844

Browse files
markshannonmarkshannon
authored
GH-120619: Clean up RETURN_VALUE instruction (GH-120624)
* Rename _POP_FRAME to _RETURN_VALUE as it returns a value as well as popping a frame. * Remove remaining _POP_FRAMEs
1 parent 79e09e6 commit 274f844

11 files changed

+67
-61
lines changed

‎Include/internal/pycore_opcode_metadata.h

+4-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_uop_ids.h

+28-28
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_uop_metadata.h

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Lib/test/test_capi/test_opt.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ def testfunc(n):
10241024
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
10251025
uop_names = [uop[0] for uop in uops_and_operands]
10261026
self.assertEqual(uop_names.count("_PUSH_FRAME"), 2)
1027-
self.assertEqual(uop_names.count("_POP_FRAME"), 2)
1027+
self.assertEqual(uop_names.count("_RETURN_VALUE"), 2)
10281028
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
10291029
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
10301030
# sequential calls: max(12, 13) == 13
@@ -1051,7 +1051,7 @@ def testfunc(n):
10511051
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
10521052
uop_names = [uop[0] for uop in uops_and_operands]
10531053
self.assertEqual(uop_names.count("_PUSH_FRAME"), 2)
1054-
self.assertEqual(uop_names.count("_POP_FRAME"), 2)
1054+
self.assertEqual(uop_names.count("_RETURN_VALUE"), 2)
10551055
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
10561056
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
10571057
# nested calls: 15 + 12 == 27
@@ -1086,7 +1086,7 @@ def testfunc(n):
10861086
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
10871087
uop_names = [uop[0] for uop in uops_and_operands]
10881088
self.assertEqual(uop_names.count("_PUSH_FRAME"), 4)
1089-
self.assertEqual(uop_names.count("_POP_FRAME"), 4)
1089+
self.assertEqual(uop_names.count("_RETURN_VALUE"), 4)
10901090
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
10911091
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
10921092
# max(12, 18 + max(12, 13)) == 31
@@ -1122,7 +1122,7 @@ def testfunc(n):
11221122
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
11231123
uop_names = [uop[0] for uop in uops_and_operands]
11241124
self.assertEqual(uop_names.count("_PUSH_FRAME"), 4)
1125-
self.assertEqual(uop_names.count("_POP_FRAME"), 4)
1125+
self.assertEqual(uop_names.count("_RETURN_VALUE"), 4)
11261126
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
11271127
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
11281128
# max(18 + max(12, 13), 12) == 31
@@ -1166,7 +1166,7 @@ def testfunc(n):
11661166
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
11671167
uop_names = [uop[0] for uop in uops_and_operands]
11681168
self.assertEqual(uop_names.count("_PUSH_FRAME"), 15)
1169-
self.assertEqual(uop_names.count("_POP_FRAME"), 15)
1169+
self.assertEqual(uop_names.count("_RETURN_VALUE"), 15)
11701170

11711171
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
11721172
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
@@ -1260,7 +1260,7 @@ def testfunc(n):
12601260
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
12611261
uop_names = [uop[0] for uop in uops_and_operands]
12621262
self.assertEqual(uop_names.count("_PUSH_FRAME"), 2)
1263-
self.assertEqual(uop_names.count("_POP_FRAME"), 0)
1263+
self.assertEqual(uop_names.count("_RETURN_VALUE"), 0)
12641264
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 1)
12651265
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
12661266
largest_stack = _testinternalcapi.get_co_framesize(dummy15.__code__)

‎Python/bytecodes.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ dummy_func(
827827
// We definitely pop the return value off the stack on entry.
828828
// We also push it onto the stack on exit, but that's a
829829
// different frame, and it's accounted for by _PUSH_FRAME.
830-
op(_POP_FRAME, (retval --)) {
830+
inst(RETURN_VALUE, (retval -- res)) {
831831
#if TIER_ONE
832832
assert(frame != &entry_frame);
833833
#endif
@@ -839,15 +839,12 @@ dummy_func(
839839
_PyInterpreterFrame *dying = frame;
840840
frame = tstate->current_frame = dying->previous;
841841
_PyEval_FrameClearAndPop(tstate, dying);
842-
_PyFrame_StackPush(frame, retval);
843842
LOAD_SP();
844843
LOAD_IP(frame->return_offset);
844+
res = retval;
845845
LLTRACE_RESUME_FRAME();
846846
}
847847

848-
macro(RETURN_VALUE) =
849-
_POP_FRAME;
850-
851848
inst(INSTRUMENTED_RETURN_VALUE, (retval --)) {
852849
int err = _Py_call_instrumentation_arg(
853850
tstate, PY_MONITORING_EVENT_PY_RETURN,
@@ -869,7 +866,7 @@ dummy_func(
869866

870867
macro(RETURN_CONST) =
871868
LOAD_CONST +
872-
_POP_FRAME;
869+
RETURN_VALUE;
873870

874871
inst(INSTRUMENTED_RETURN_CONST, (--)) {
875872
PyObject *retval = GETITEM(FRAME_CO_CONSTS, oparg);

‎Python/executor_cases.c.h

+5-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/generated_cases.c.h

+9-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/optimizer.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ add_to_trace(
537537
// Reserve space for N uops, plus 3 for _SET_IP, _CHECK_VALIDITY and _EXIT_TRACE
538538
#define RESERVE(needed) RESERVE_RAW((needed) + 3, _PyUOpName(opcode))
539539

540-
// Trace stack operations (used by _PUSH_FRAME, _POP_FRAME)
540+
// Trace stack operations (used by _PUSH_FRAME, _RETURN_VALUE)
541541
#define TRACE_STACK_PUSH() \
542542
if (trace_stack_depth >= TRACE_STACK_SIZE) { \
543543
DPRINTF(2, "Trace stack overflow\n"); \
@@ -748,10 +748,10 @@ translate_bytecode_to_trace(
748748
int nuops = expansion->nuops;
749749
RESERVE(nuops + 1); /* One extra for exit */
750750
int16_t last_op = expansion->uops[nuops-1].uop;
751-
if (last_op == _POP_FRAME || last_op == _RETURN_GENERATOR || last_op == _YIELD_VALUE) {
751+
if (last_op == _RETURN_VALUE || last_op == _RETURN_GENERATOR || last_op == _YIELD_VALUE) {
752752
// Check for trace stack underflow now:
753753
// We can't bail e.g. in the middle of
754-
// LOAD_CONST + _POP_FRAME.
754+
// LOAD_CONST + _RETURN_VALUE.
755755
if (trace_stack_depth == 0) {
756756
DPRINTF(2, "Trace stack underflow\n");
757757
OPT_STAT_INC(trace_stack_underflow);
@@ -810,7 +810,7 @@ translate_bytecode_to_trace(
810810
Py_FatalError("garbled expansion");
811811
}
812812

813-
if (uop == _POP_FRAME || uop == _RETURN_GENERATOR || uop == _YIELD_VALUE) {
813+
if (uop == _RETURN_VALUE || uop == _RETURN_GENERATOR || uop == _YIELD_VALUE) {
814814
TRACE_STACK_POP();
815815
/* Set the operand to the function or code object returned to,
816816
* to assist optimization passes. (See _PUSH_FRAME below.)

‎Python/optimizer_analysis.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,
265265
}
266266
break;
267267
}
268-
case _POP_FRAME:
268+
case _RETURN_VALUE:
269269
{
270270
builtins_watched >>= 1;
271271
globals_watched >>= 1;
@@ -365,13 +365,13 @@ eliminate_pop_guard(_PyUOpInstruction *this_instr, bool exit)
365365
}
366366
}
367367

368-
/* _PUSH_FRAME/_POP_FRAME's operand can be 0, a PyFunctionObject *, or a
368+
/* _PUSH_FRAME/_RETURN_VALUE's operand can be 0, a PyFunctionObject *, or a
369369
* PyCodeObject *. Retrieve the code object if possible.
370370
*/
371371
static PyCodeObject *
372372
get_code(_PyUOpInstruction *op)
373373
{
374-
assert(op->opcode == _PUSH_FRAME || op->opcode == _POP_FRAME || op->opcode == _RETURN_GENERATOR);
374+
assert(op->opcode == _PUSH_FRAME || op->opcode == _RETURN_VALUE || op->opcode == _RETURN_GENERATOR);
375375
PyCodeObject *co = NULL;
376376
uint64_t operand = op->operand;
377377
if (operand == 0) {

‎Python/optimizer_bytecodes.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ dummy_func(void) {
606606
ctx->done = true;
607607
}
608608

609-
op(_POP_FRAME, (retval -- res)) {
609+
op(_RETURN_VALUE, (retval -- res)) {
610610
SYNC_SP();
611611
ctx->frame->stack_pointer = stack_pointer;
612612
frame_pop(ctx);

‎Python/optimizer_cases.c.h

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)