Skip to content

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

Open
NekoAsakura wants to merge 3 commits intopython:mainfrom
NekoAsakura:gh-145866/end-for-end-send-macro
Open

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 intopython:mainfrom
NekoAsakura:gh-145866/end-for-end-send-macro

Conversation

@NekoAsakura
Copy link
Copy Markdown
Contributor

@NekoAsakura NekoAsakura commented Apr 12, 2026

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

    case COMPILE_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);
    return SUCCESS;

@NekoAsakura
Copy link
Copy Markdown
Contributor Author

#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 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.
*/
Copy link
Copy Markdown
Member

@cocolato cocolato Apr 13, 2026

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].

res = PyStackRef_NULL;
}

no_save_ip inst(END_FOR, (value -- )) {
Copy link
Copy Markdown
Member

@cocolato cocolato Apr 13, 2026

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
Contributor Author

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.

Sign up for free to 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