Since_PUSH_FRAME is justframe->return_offset = 0; DISPATCH_INLINED(new_frame);, it would make sense to spell outDISPATCH_INLINED to clarify which operations that need to be different for tier 1 and tier 2.
Something like:

op(_PUSH_FRAME, (new_frame:_PyInterpreterFrame*--unused)) {SAVE_FRAME_STATE();// Equivalent to frame->prev_instr = next_instr - 1; _PyFrame_SetStackPointer(frame, stack_pointer);frame->return_offset=0;new_frame->previous=frame;frame=cframe.current_frame=new_frame;CALL_STAT_INC(inlined_py_calls);if (_Py_EnterRecursivePy(tstate)) {        gotoexit_unwind;    }START_FRAME();// Equivalent to next_instr = frame->prev_instr  + 1; stack_pointer =stack_pointer=_PyFrame_GetStackPointer(frame);}

For example:2b3e6f2

In which case the code generators needs to know to push the temporary stack values to the real stack beforeSAVE_FRAME_STATE()

I have to study this more. A problem is that the Tier 1 and Tier 2 versions of_PUSH_FRAME are so different. I am working on a mechanism to be able to say

#if TIER_ONE<code for Tier 1>#else<code for Tier 2>#endif

I'm not sure yet what you mean with your last remark about pushing temp stack values.

Comparing carefully the two versions ofDISPATCH_INLINED (addingframe->return_offset = 0 which precedes it in both cases):

In Tier 1:

        frame->return_offset = 0;        assert(tstate->interp->eval_frame == NULL);        _PyFrame_SetStackPointer(frame, stack_pointer);        frame->prev_instr = next_instr - 1;        (NEW_FRAME)->previous = frame;        frame = cframe.current_frame = (NEW_FRAME);        CALL_STAT_INC(inlined_py_calls);        goto start_frame;

In Tier 2:

        frame->return_offset = 0;        assert(tstate->interp->eval_frame == NULL);        _PyFrame_SetStackPointer(frame, stack_pointer);        frame->prev_instr -= 1;        (NEW_FRAME)->previous = frame;        frame = tstate->cframe->current_frame = (NEW_FRAME);        CALL_STAT_INC(inlined_py_calls);        stack_pointer = _PyFrame_GetStackPointer(frame);        ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;

Diff:

@@ -1,8 +1,9 @@         frame->return_offset = 0;         assert(tstate->interp->eval_frame == NULL);         _PyFrame_SetStackPointer(frame, stack_pointer);-        frame->prev_instr = next_instr - 1;+        frame->prev_instr -= 1;         (NEW_FRAME)->previous = frame;-        frame = cframe.current_frame = (NEW_FRAME);+        frame = tstate->cframe->current_frame = (NEW_FRAME);         CALL_STAT_INC(inlined_py_calls);-        goto start_frame;+        stack_pointer = _PyFrame_GetStackPointer(frame);+        ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;

Note that the Tier 2 version must be preceded by aSAVE_IP, which does the equivalent offrame->prev_instr = next_instr. If we had a Tier 1 version ofSAVE_IP we could include it in the macro definition:

        macro(CALL_PY_EXACT_ARGS) =            unused/1 + // Skip over the counter            _CHECK_PEP_523 +            _CHECK_FUNCTION_EXACT_ARGS +            _CHECK_STACK_SPACE +            _INIT_CALL_PY_EXACT_ARGS +            SAVE_IP +      // <-------------- added            _PUSH_FRAME;

which would reduce the special-casing in the code generator a bit (it would still need to do something special forSAVE_IP to ensure that itsoparg has the right value, different from theoparg of the macro (which is the argument count). This would take care of the first diff chunk (what to assign toframe->prev_inst), but it would still be pretty fragile. (Like my current version, it would entice the optimizer to incorrectly try to remove theSAVE_IP uop.)

The second diff chunk relates to how we setcframe.current_frame -- in Tier 2 we must access this through thetstate.

The third and final diff chunk relates to really start using the new frame. In Tier 1, this must actually do the following:

• Check recursion
• Loadstack_pointer
• Loadnext_instr
• Dispatch to the next opcode.

This is done by the code atstart_frame.

In Tier 2 there is nostart_frame label (the only uop that can go to a label isEXIT_TRACE, and of courseDEOPT_IF andERROR_IF also jump). So we loadstack_frame here. There is no direct equivalent tonext_instr, but we have to setip_offset, whichSAVE_IP adds to itsoparg to get theprev_instr value. (This variable is a cache forframe->code->co_code_adaptive, to save some memory loads, so wheneverframe changes we must update it.)

(More later.)

There's another thing though, and I think that is what Mark meant. In Tier 1 the code generation for macros is special-cased for_PUSH_FRAME so that both the stack adjustmentand thenext_instr adjustment are emittedbefore the_PUSH_FRAME opcode. This is done so that the flushing of these variables to the frame in theDISPATCH_INLINED macro flush the correct values.

But this is really ugly and unprincipled, and the logic is much hairier than the other special cases for_PUSH_FRAME. One of Mark's ideas here is to make this special case look for uops using theSAVE_FRAME_STATE macro rather than for the specific uop_PUSH_FRAME. But detecting when to trigger the special case is only part of the problem -- IMO the worse problem is that the special case itself is so ugly:

        dispatch_inlined_special_case = False        if mgr is managers[-1] and mgr.instr.always_exits.startswith("DISPATCH_INLINED") and mgr.instr.name == "_PUSH_FRAME":            dispatch_inlined_special_case = True            temp = mgr.final_offset.clone()            temp.deeper(StackEffect(UNUSED))  # Hack            out.stack_adjust(temp.deep, temp.high)            # Use clone() since adjust_inverse() mutates final_offset            mgr.adjust_inverse(mgr.final_offset.clone())            if cache_adjust:                out.emit(f"next_instr += {cache_adjust};")

The last 4 lines here, starting with# Use clone(), occur further down too, for the normal case (after the final uop). I don't even recall why thetemp.deeper() call is needed!

I'll mull this over some more.

I think I have addressed this.@markshannon Please have another look. Assuming the tests look okay I'll un-draft this.