Skip to content

gh-145866: Convert END_[FOR/SEND] to leave its inputs on the stack to be cleaned up by _POP_TOP - #148477

Closed
NekoAsakura wants to merge 3 commits into
python:mainfrom
NekoAsakura:gh-145866/end-for-end-send-macro
Closed

gh-145866: Convert END_[FOR/SEND] to leave its inputs on the stack to be cleaned up by _POP_TOP#148477
NekoAsakura wants to merge 3 commits into
python:mainfrom
NekoAsakura:gh-145866/end-for-end-send-macro

Conversation

@NekoAsakura

@NekoAsakuraNekoAsakura commented Apr 12, 2026

Copy link
Copy Markdown
Member

If I (and opus...) am not mistaken, no code path actually jumps to END_FOR. It's only there for FOR_ITER's oparg calibration and as an instrumentation slot (gets swapped to INSTRUMENTED_END_FOR). So converting it to macro(END_FOR) = POP_TOP should be safe. PyStackRef_CLOSE or PyStackRef_XCLOSE doesn't matter.

ref:

  1. Generic _FOR_ITER
    JUMPBY(oparg+1);
  2. INSTRUMENTED_FOR_ITER
    JUMPBY(oparg+1);
  3. _ITER_JUMP_LIST
    JUMPBY(oparg+1);
  4. _ITER_NEXT_LIST (free-threaded)
    JUMPBY(oparg+1);
  5. _ITER_JUMP_TUPLE
    JUMPBY(oparg+1);
  6. _ITER_JUMP_RANGE
    JUMPBY(oparg+1);
  7. _FOR_ITER_TIER_TWO

    cpython/Python/bytecodes.c

    Lines 3644 to 3645 in 03d2f03

    /* The translator sets the deopt target just past the matching END_FOR */
    EXIT_IF(true);
  8. _FOR_ITER_GEN_FRAME
    frame->return_offset= (uint16_t)(INSTRUCTION_SIZE+oparg);
  9. codegen_unwind_fblock

    cpython/Python/codegen.c

    Lines 546 to 553 in 03d2f03

    caseCOMPILE_FBLOCK_FOR_LOOP:
    /* Pop the iterator */
    if (preserve_tos) {
    ADDOP_I(c, *ploc, SWAP, 3);
    }
    ADDOP(c, *ploc, POP_TOP);
    ADDOP(c, *ploc, POP_TOP);
    returnSUCCESS;

@NekoAsakura

Copy link
Copy Markdown
MemberAuthor

#146185 (comment)
I’ve done that lldb thing here as well. Everything looks good to me. Let me know if you’d like me to add any specific unit tests.

Comment threadPython/bytecodes.c
Comment on lines -393 to -398
no_save_ip inst(END_FOR, (value -- )) {
/* Don't update instr_ptr, so that POP_ITER sees
* the FOR_ITER as the previous instruction.
* This has the benign side effect that if value is
* finalized it will see the location as the FOR_ITER's.
*/

@cocolatococolatoApr 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the no_save_ip will change the INSTRUMENTED_POP_ITER's prev_instr from FOR_ITER to END_FOR, this may affect the sys.monitoring callback and may cause differences in behavior between for loops that use generators and other for loops, such as for x in [1,2,3].

Comment threadPython/bytecodes.c
res = PyStackRef_NULL;
}

no_save_ip inst(END_FOR, (value -- )) {

@cocolatococolatoApr 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And END_FOR will be executed. You can try:

def foo():
yield 42
for x in foo():
pass

@NekoAsakura

Copy link
Copy Markdown
MemberAuthor

Good catch, thanks for pointing that out. You're right, I added a new test that captures this.

I'm starting to wonder whether this change is actually worth it for END_FOR/END_SEND though — they're never in a tier 2 trace, so they can't actually benefit from the _POP_TOP optimisation. Let's see what code owners think.

@NekoAsakura

Copy link
Copy Markdown
MemberAuthor

Close as END_FOR/END_SEND never appear in tier 2 trace, no benefit.

@NekoAsakura
NekoAsakura deleted the gh-145866/end-for-end-send-macro branch April 18, 2026 20:09
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@NekoAsakura@cocolato@Fidget-Spinner