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.

Single commit, rebased on upstream 24e5a55ccb. This fork's main has been fast-forwarded to the same commit, so the diff is exactly this change.

(Supersedes #1, which targeted a temporary base branch and was auto-closed by the squash.)

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 guard and frame push are deliberately a single uop. Splitting them, mirroring _BINARY_OP_SUBSCR_CHECK_FUNC, puts the looked-up function on the caller's value stack as a temporary; STORE_SUBSCR already occupies three stack slots, so that extra slot overruns co_stacksize and trips ASSERT_WITHIN_STACK_BOUNDS:

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

BINARY_OP_SUBSCR_GETITEM survives it because BINARY_OP occupies only two. Worth knowing this is invisible to both a release build and the code generators — only --with-pydebug catches it.

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.

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 before this goes upstream

• 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. A release build + the pyperf script is needed.
• Full test suite not run to completion — the 14 suites above passed, but the full sweep was stopped early.
• 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 a PR. Given this needs new interpreter machinery, worth floating the shim design on the issue first.

🤖 Generated with Claude Code