gh-143732: Specialize __setitem__ dunder method for STORE_SUBSCR

[johng]

Reworked take on python#156033, which was closed with:

This does not implement the specialization of STORE_SUBSCR for __setitem__ implemented in Python, as described in the issue.

Review base: this targets base-gh-143732, a branch pinned at upstream 24e5a55ccb, so the diff is exactly the two commits here. main on this fork is 579 commits behind upstream and would have buried the change.

Why the old approach didn't satisfy the issue

The previous PR called the dunder through an ordinary re-entrant vectorcall (_PyObject_VectorcallTstate). pythongh-143732 asks these specializations to jump directly into the method, using LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN as the template, so the JIT can trace through the call. A vectorcall gets the interpreter win but not the inlined frame, which is the actual point.

The obstacle

You cannot simply push the __setitem__ frame the way BINARY_OP_SUBSCR_GETITEM pushes __getitem__:

• _RETURN_VALUE is (retval -- res). A returning frame always pushes exactly one value onto the caller.
• BINARY_OP_SUBSCR_GETITEM is declared pops 2 / pushes 0; the +1 arrives on return, netting −1 to match generic BINARY_OP.
• STORE_SUBSCR must net −3. A frame-pushing member pops 3, declares 0 pushed, gains +1 on return → −2. Off by one, with no fourth operand to absorb it.

__setitem__ returns a None that nothing consumes.

Approach

A shim frame (_Py_SetItemCleanup) is pushed underneath the __setitem__ frame, exactly as CALL_ALLOC_AND_ENTER_INIT uses _Py_InitCleanup for __init__. The dunder returns into the shim; the shim's sole instruction EXIT_SETITEM discards that value and pops the shim without pushing, keeping the net effect at −3.

The function is cached on the heap type (_spec_cache.setitem, mirroring _spec_cache.getitem) rather than in the inline cache, so STORE_SUBSCR's cache size stays at 1 — the old PR grew it to 3, bloating every STORE_SUBSCR.

Tier 1 only; tier 2 / JIT support is deliberately left for a follow-up. executor_cases.c.h, optimizer_cases.c.h and pycore_uop_ids.h are untouched.

Second commit

The first commit split guard and frame-push into two uops, mirroring _BINARY_OP_SUBSCR_CHECK_FUNC. That pushes the looked-up function onto the caller's stack as a temporary, which overran co_stacksize and tripped ASSERT_WITHIN_STACK_BOUNDS:

Stack overflow (depth = 6) at Python/generated_cases.c.h:12933

BINARY_OP_SUBSCR_GETITEM survives that extra slot because BINARY_OP occupies two; STORE_SUBSCR occupies three. The second commit merges the uops so the function stays in a C local. Kept as a separate commit because it is the one non-obvious constraint here — squash before submitting upstream.

Note this was invisible to both a release build and the code generators; only --with-pydebug caught it.

Testing

Built --with-pydebug and verified:

Check	Result
test_opcache	82/82, incl. 4 new sub-tests
14 targeted suites	4,369 tests pass
Specialization	c[i] = v → STORE_SUBSCR_PY_DUNDER
Traceback	['<module>', '__setitem__'] — shim invisible
sys.settrace	sees only __setitem__
Deopt on rebinding __setitem__	correct old→new dispatch
Deep recursion	clean RecursionError
Refcounts	flat (delta 2 over 1000 stores)
*args / defaults / kw-only	correctly refused

That last row is why assert(fcode->co_argcount == 3) is an assert rather than a deopt, matching _BINARY_OP_SUBSCR_CHECK_FUNC.

Open items

• No benchmark number. Debug build only, so the ~5% claim from gh-143732: Specialize __setitem__ dunder method for STORE_SUBSCR python/cpython#156033 is unverified here. A release build + the pyperf script is needed before submitting upstream.
• The issue's first step also wants a C __dunder__ fallback specialization; not included.
• Mark asked that design clarification go on Broader specialization in the Specializing Adaptive Interpreter for better JIT performance python/cpython#143732 rather than the PR. Given this needs new interpreter machinery, worth floating the shim design on the issue first.

🤖 Generated with Claude Code