Skip to content

Commit fe843d8

Browse files
committed
Split RETURN_{VALUE,CONST} into uops (mostly works)
1 parent 00ae0fa commit fe843d8

8 files changed

Lines changed: 159 additions & 72 deletions

File tree

Include/internal/pycore_ceval.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ void _PyEval_FormatKwargsError(PyThreadState *tstate, PyObject *func, PyObject *
171171
PyObject *_PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, Py_ssize_t nargs, PyObject *kwargs);
172172
PyObject *_PyEval_MatchKeys(PyThreadState *tstate, PyObject *map, PyObject *keys);
173173
int _PyEval_UnpackIterable(PyThreadState *tstate, PyObject *v, int argcnt, int argcntafter, PyObject **sp);
174+
void _PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame *frame);
174175

175176

176177
#ifdef __cplusplus

Include/internal/pycore_opcode_metadata.h

Lines changed: 36 additions & 28 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/abstract_interp_cases.c.h

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/bytecodes.c

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -764,21 +764,39 @@ dummy_func(
764764
return retval;
765765
}
766766

767-
inst(RETURN_VALUE, (retval --)) {
768-
STACK_SHRINK(1);
767+
// The stack effect here is ambiguous.
768+
// We definitely pop the return value off the stack on entry.
769+
// We also push it onto the stack on exit, but that's a
770+
// different frame, and it's accounted for by _PUSH_FRAME.
771+
op(_POP_FRAME, (retval --)) {
769772
assert(EMPTY());
770-
_PyFrame_SetStackPointer(frame, stack_pointer);
773+
SAVE_FRAME_STATE(); // Signals to the code generator
774+
#if TIER_ONE
771775
_Py_LeaveRecursiveCallPy(tstate);
772776
assert(frame != &entry_frame);
777+
#endif
773778
// GH-99729: We need to unlink the frame *before* clearing it:
774779
_PyInterpreterFrame *dying = frame;
780+
#if TIER_ONE
775781
frame = cframe.current_frame = dying->previous;
776-
_PyEvalFrameClearAndPop(tstate, dying);
782+
#endif
783+
#if TIER_TWO
784+
frame = tstate->cframe->current_frame = dying->previous;
785+
#endif
786+
_PyEval_FrameClearAndPop(tstate, dying);
777787
frame->prev_instr += frame->return_offset;
778788
_PyFrame_StackPush(frame, retval);
789+
#if TIER_ONE
779790
goto resume_frame;
791+
#endif
792+
#if TIER_TWO
793+
stack_pointer = _PyFrame_GetStackPointer(frame);
794+
ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;
795+
#endif
780796
}
781797

798+
macro(RETURN_VALUE) = _POP_FRAME;
799+
782800
inst(INSTRUMENTED_RETURN_VALUE, (retval --)) {
783801
int err = _Py_call_instrumentation_arg(
784802
tstate, PY_MONITORING_EVENT_PY_RETURN,
@@ -792,27 +810,13 @@ dummy_func(
792810
// GH-99729: We need to unlink the frame *before* clearing it:
793811
_PyInterpreterFrame *dying = frame;
794812
frame = cframe.current_frame = dying->previous;
795-
_PyEvalFrameClearAndPop(tstate, dying);
813+
_PyEval_FrameClearAndPop(tstate, dying);
796814
frame->prev_instr += frame->return_offset;
797815
_PyFrame_StackPush(frame, retval);
798816
goto resume_frame;
799817
}
800818

801-
inst(RETURN_CONST, (--)) {
802-
PyObject *retval = GETITEM(FRAME_CO_CONSTS, oparg);
803-
Py_INCREF(retval);
804-
assert(EMPTY());
805-
_PyFrame_SetStackPointer(frame, stack_pointer);
806-
_Py_LeaveRecursiveCallPy(tstate);
807-
assert(frame != &entry_frame);
808-
// GH-99729: We need to unlink the frame *before* clearing it:
809-
_PyInterpreterFrame *dying = frame;
810-
frame = cframe.current_frame = dying->previous;
811-
_PyEvalFrameClearAndPop(tstate, dying);
812-
frame->prev_instr += frame->return_offset;
813-
_PyFrame_StackPush(frame, retval);
814-
goto resume_frame;
815-
}
819+
macro(RETURN_CONST) = LOAD_CONST + _POP_FRAME;
816820

817821
inst(INSTRUMENTED_RETURN_CONST, (--)) {
818822
PyObject *retval = GETITEM(FRAME_CO_CONSTS, oparg);
@@ -828,7 +832,7 @@ dummy_func(
828832
// GH-99729: We need to unlink the frame *before* clearing it:
829833
_PyInterpreterFrame *dying = frame;
830834
frame = cframe.current_frame = dying->previous;
831-
_PyEvalFrameClearAndPop(tstate, dying);
835+
_PyEval_FrameClearAndPop(tstate, dying);
832836
frame->prev_instr += frame->return_offset;
833837
_PyFrame_StackPush(frame, retval);
834838
goto resume_frame;

Python/ceval.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,6 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func,
222222
static _PyInterpreterFrame *
223223
_PyEvalFramePushAndInit_Ex(PyThreadState *tstate, PyFunctionObject *func,
224224
PyObject *locals, Py_ssize_t nargs, PyObject *callargs, PyObject *kwargs);
225-
static void
226-
_PyEvalFrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame *frame);
227225

228226
#ifdef HAVE_ERRNO_H
229227
#include <errno.h>
@@ -925,7 +923,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
925923
// GH-99729: We need to unlink the frame *before* clearing it:
926924
_PyInterpreterFrame *dying = frame;
927925
frame = cframe.current_frame = dying->previous;
928-
_PyEvalFrameClearAndPop(tstate, dying);
926+
_PyEval_FrameClearAndPop(tstate, dying);
929927
frame->return_offset = 0;
930928
if (frame == &entry_frame) {
931929
/* Restore previous cframe and exit */
@@ -1495,8 +1493,8 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame)
14951493
frame->previous = NULL;
14961494
}
14971495

1498-
static void
1499-
_PyEvalFrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame * frame)
1496+
void
1497+
_PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame * frame)
15001498
{
15011499
if (frame->owner == FRAME_OWNED_BY_THREAD) {
15021500
clear_thread_frame(tstate, frame);

Python/executor_cases.c.h

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/generated_cases.c.h

Lines changed: 52 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/optimizer.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,12 @@ translate_bytecode_to_trace(
619619
expansion->uops[i].offset);
620620
Py_FatalError("garbled expansion");
621621
}
622+
if (expansion->uops[i].uop == _POP_FRAME) {
623+
// TODO: Move this code *after* adding it to the trace
624+
// TODO: Continue in the previous code object, if any
625+
ADD_TO_TRACE(SAVE_IP, INSTR_IP(instr, code), 0);
626+
goto done;
627+
}
622628
ADD_TO_TRACE(expansion->uops[i].uop, oparg, operand);
623629
if (expansion->uops[i].uop == _PUSH_FRAME) {
624630
assert(i + 1 == nuops);

0 commit comments

Comments
 (0)