gh-145866: Implement POP_ITER as macro op using POP_TOP

[brijkapadia]

As the JIT can optimize _POP_TOP uops, I implemented POP_TOP for the POP_ITER instruction. I am fairly confident that this is the right solution but I am new to the JIT and would welcome any feedback.

• Issue: Convert more uops to leave their inputs on the stack to be cleaned up by _POP_TOP #145866

[Fidget-Spinner]

This test seems wrong. It's not testing for POP_ITER, or at least it's not in the trace

[brijkapadia]

What do you think I should change? When I wrote the test, I just wanted to make sure that a POP_ITER instruction would be converted into only one _POP_TOP uop as that was the goal of the original issue. Maybe it might be helpful to rename the test to something like test_pop_iter_uop_expansion?

[Fidget-Spinner]

Yes I understand the point of the issue, but what I'm saying is that this test does not test for the expansion.

Try dumping the trace of this specific test, you will see what I mean

[brijkapadia]

Okay I spent some time thinking about it and after I ran my test on main I realized what you were saying. However, I am not exactly sure how I would write a test for the expansion of POP_ITER to _POP_TOP_NOP; _POP_TOP. Furthermore, is a test actually necessary since this is such a minor change?

[Fidget-Spinner]

However, I am not exactly sure how I would write a test for the expansion of POP_ITER to _POP_TOP_NOP; _POP_TOP.

I'm not 100% sure either. IIRC, POP_ITER occurs after for loops or after certain types of generators. So you might want to try building with --enable-experimental-jit=interpreter and seeing if you can dump a PYTHON_LLTRACE=2 which has the POP_ITER opcode within for a test case using generators/certain types of for loops.

Furthermore, is a test actually necessary since this is such a minor change?

This is not a minor change. We're changing the very core of the Python interpreter itself. Anything wrong here could lead to more wasted time for other contributors to track down bugs/fix things. I'm guilty of introducing bugs into this part of the Python interpreter, so I'm very careful when it comes to this.

[NekoAsakura]

@Fidget-Spinner I think POP_ITER never makes it into a tier-2 trace.

---

Build: CC=clang-21 ./configure --with-pydebug --enable-experimental-jit=interpreter && make -j$(nproc)

test_pop_iter.py:

total = 0
for i in range(5000):
    total += i
print(total)

$ PYTHON_LLTRACE=2 ./python test_pop_iter.py 2>&1 | grep -iE 'POP_ITER|_POP_TOP_NOP'

Zero return.

---

https://github.com/brijkapadia/cpython/blob/a421f9005e7/Python/generated_cases.c.h#L10176-L10201
(No preview available as it's not merged)
Since POP_ITER runs in tier-1 where the macro is expanded inline in generated_cases.c.h, we can use lldb to step through it. The _POP_TOP_NOP assert (L10188-L10189) would crash if the stack order were wrong.

pop_iter_lldb.script:

b generated_cases.c.h:10187
run
n
n
n
n
n
n
n
quit

$ lldb --source pop_iter_lldb.script -- ./python test_pop_iter.py

(lldb) b generated_cases.c.h:10187
Breakpoint 1: where = python`_PyEval_EvalFrameDefault + 190382 at generated_cases.c.h:10187:25

(lldb) run
* thread #1, stop reason = breakpoint 1.1
   10185            // _POP_TOP_NOP
   10186            {
-> 10187                value = stack_pointer[-1];
   10188                assert(PyStackRef_IsNull(value) || (!PyStackRef_RefcountOnObject(value)) ||

(lldb) n    // assert passes — index_or_null is null/non-refcounted
   10191            // _POP_TOP
   10192            {
-> 10193                value = stack_pointer[-2];

(lldb) n
-> 10197                PyStackRef_XCLOSE(value);    // closes iter

(lldb) n
-> 10200            DISPATCH();

Expansion works sound.
But for unit test, a functional test is the best we can do here.