GH-128563: Move some labels, to simplify implementing tailcalling interpreter. (GH-129525)

Author: markshannon

Python/bytecodes.c

@@ -5303,14 +5303,40 @@ dummy_func(
                 tstate->c_recursion_remaining += PY_EVAL_C_STACK_UNITS;
                 return NULL;
             }
-            goto resume_with_error;
+            next_instr = frame->instr_ptr;
+            stack_pointer = _PyFrame_GetStackPointer(frame);
+            goto error;
         }
 
-        label(resume_with_error) {
+        label(start_frame) {
+            if (_Py_EnterRecursivePy(tstate)) {
+                goto exit_unwind;
+            }
             next_instr = frame->instr_ptr;
             stack_pointer = _PyFrame_GetStackPointer(frame);
-            goto error;
+
+        #ifdef LLTRACE
+            {
+                int lltrace = maybe_lltrace_resume_frame(frame, GLOBALS());
+                frame->lltrace = lltrace;
+                if (lltrace < 0) {
+                    goto exit_unwind;
+                }
+            }
+        #endif
+
+        #ifdef Py_DEBUG
+            /* _PyEval_EvalFrameDefault() must not be called with an exception set,
+            because it can clear it (directly or indirectly) and so the
+            caller loses its exception */
+            assert(!_PyErr_Occurred(tstate));
+        #endif
+
+            DISPATCH();
         }
+
+
+
 // END BYTECODES //
 
     }
@@ -5320,7 +5346,6 @@ dummy_func(
  exit_unwind:
  handle_eval_breaker:
  resume_frame:
- resume_with_error:
  start_frame:
  unbound_local_error:
     ;

Python/ceval.c

@@ -792,6 +792,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
         return NULL;
     }
 
+    /* Local "register" variables.
+     * These are cached values from the frame and code object.  */
+    _Py_CODEUNIT *next_instr;
+    _PyStackRef *stack_pointer;
+
 #if defined(Py_DEBUG) && !defined(Py_STACKREF_DEBUG)
     /* Set these to invalid but identifiable values for debugging. */
     entry_frame.f_funcobj = (_PyStackRef){.bits = 0xaaa0};
@@ -819,67 +824,36 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
     /* support for generator.throw() */
     if (throwflag) {
         if (_Py_EnterRecursivePy(tstate)) {
-            goto exit_unwind;
+            goto early_exit;
         }
-        /* Because this avoids the RESUME,
-         * we need to update instrumentation */
 #ifdef Py_GIL_DISABLED
         /* Load thread-local bytecode */
         if (frame->tlbc_index != ((_PyThreadStateImpl *)tstate)->tlbc_index) {
             _Py_CODEUNIT *bytecode =
                 _PyEval_GetExecutableCode(tstate, _PyFrame_GetCode(frame));
             if (bytecode == NULL) {
-                goto exit_unwind;
+                goto early_exit;
             }
             ptrdiff_t off = frame->instr_ptr - _PyFrame_GetBytecode(frame);
             frame->tlbc_index = ((_PyThreadStateImpl *)tstate)->tlbc_index;
             frame->instr_ptr = bytecode + off;
         }
 #endif
+        /* Because this avoids the RESUME, we need to update instrumentation */
         _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp);
-        monitor_throw(tstate, frame, frame->instr_ptr);
-        /* TO DO -- Monitor throw entry. */
-        goto resume_with_error;
+        next_instr = frame->instr_ptr;
+        stack_pointer = _PyFrame_GetStackPointer(frame);
+        monitor_throw(tstate, frame, next_instr);
+        goto error;
     }
 
-    /* Local "register" variables.
-     * These are cached values from the frame and code object.  */
-    _Py_CODEUNIT *next_instr;
-    _PyStackRef *stack_pointer;
-
 #if defined(_Py_TIER2) && !defined(_Py_JIT)
     /* Tier 2 interpreter state */
     _PyExecutorObject *current_executor = NULL;
     const _PyUOpInstruction *next_uop = NULL;
 #endif
 
-start_frame:
-    if (_Py_EnterRecursivePy(tstate)) {
-        goto exit_unwind;
-    }
-
-    next_instr = frame->instr_ptr;
-resume_frame:
-    stack_pointer = _PyFrame_GetStackPointer(frame);
-
-#ifdef LLTRACE
-    {
-        int lltrace = maybe_lltrace_resume_frame(frame, GLOBALS());
-        frame->lltrace = lltrace;
-        if (lltrace < 0) {
-            goto exit_unwind;
-        }
-    }
-#endif
-
-#ifdef Py_DEBUG
-    /* _PyEval_EvalFrameDefault() must not be called with an exception set,
-       because it can clear it (directly or indirectly) and so the
-       caller loses its exception */
-    assert(!_PyErr_Occurred(tstate));
-#endif
-
-    DISPATCH();
+    goto start_frame;
 
 #include "generated_cases.c.h"
 
@@ -983,10 +957,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
     OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
     assert(next_uop[-1].format == UOP_FORMAT_TARGET);
     frame->return_offset = 0;  // Don't leave this random
-    _PyFrame_SetStackPointer(frame, stack_pointer);
     Py_DECREF(current_executor);
     tstate->previous_executor = NULL;
-    goto resume_with_error;
+    next_instr = frame->instr_ptr;
+    goto error;
 
 jump_to_jump_target:
     assert(next_uop[-1].format == UOP_FORMAT_JUMP);
@@ -1018,6 +992,20 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
 
 #endif // _Py_TIER2
 
+early_exit:
+    assert(_PyErr_Occurred(tstate));
+    _Py_LeaveRecursiveCallPy(tstate);
+    assert(frame->owner != FRAME_OWNED_BY_INTERPRETER);
+    // GH-99729: We need to unlink the frame *before* clearing it:
+    _PyInterpreterFrame *dying = frame;
+    frame = tstate->current_frame = dying->previous;
+    _PyEval_FrameClearAndPop(tstate, dying);
+    frame->return_offset = 0;
+    assert(frame->owner == FRAME_OWNED_BY_INTERPRETER);
+    /* Restore previous frame and exit */
+    tstate->current_frame = frame->previous;
+    tstate->c_recursion_remaining += PY_EVAL_C_STACK_UNITS;
+    return NULL;
 }
 
 #if defined(__GNUC__)

Python/ceval_macros.h

@@ -381,7 +381,9 @@ do {                                                   \
     tstate->previous_executor = NULL;                  \
     frame = tstate->current_frame;                     \
     if (next_instr == NULL) {                          \
-        goto resume_with_error;                        \
+        next_instr = frame->instr_ptr;                 \
+        stack_pointer = _PyFrame_GetStackPointer(frame); \
+        goto error;                                    \
     }                                                  \
     stack_pointer = _PyFrame_GetStackPointer(frame);   \
     DISPATCH();                                        \

Python/generated_cases.c.h

@@ -511,7 +511,6 @@ def has_error_with_pop(op: parser.InstDef) -> bool:
         variable_used(op, "ERROR_IF")
         or variable_used(op, "pop_1_error")
         or variable_used(op, "exception_unwind")
-        or variable_used(op, "resume_with_error")
     )
 
 
@@ -520,7 +519,6 @@ def has_error_without_pop(op: parser.InstDef) -> bool:
         variable_used(op, "ERROR_NO_POP")
         or variable_used(op, "pop_1_error")
         or variable_used(op, "exception_unwind")
-        or variable_used(op, "resume_with_error")
     )