jit: drop the in-place-append write-barrier residual, accept the full specialized binop tag set in the FOR_ITER inline gate - #782
Merged
Conversation
WalkthroughThe JIT now detects GC-backed in-place list appends, suppresses duplicate residual write-barrier recording, and applies stricter numeric replay-safety checks. Diagnostics and arithmetic and GC stress benchmarks support the updated behavior. ChangesJIT append barrier and replay safety
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant orthodox_list_append_commit
participant list_object_helper
participant residual_dispatch
participant list_write_barrier
orthodox_list_append_commit->>list_object_helper: check GC-backed in-place storage
list_object_helper-->>orthodox_list_append_commit: return coverage status
orthodox_list_append_commit->>residual_dispatch: provide covered receiver
residual_dispatch->>residual_dispatch: omit duplicate residual OpRef
residual_dispatch->>list_write_barrier: execute barrier concretely
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit bb1e8a0). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patch
2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
The #171 append fold descends `w_list_append` and folds the in-place Object arm's store to `SetarrayitemGc`. The arm also calls `list_write_barrier`, which is `#[dont_look_inside]`, so the sub-walk left it in the trace as a `CallN` residual — one per iteration of an Object-strategy comprehension. The backend GC rewrite already marks that same store: `SetarrayitemGc` with a Ref value routes through `handle_write_barrier_setarrayitem` (rewrite.py:936-944) into `COND_CALL_GC_WB_ARRAY`, and the rewriter runs in production (`runner.rs` invokes it whenever `gc_sync::is_initialized()`). The in-place arm does not replace the list's `items` pointer, so remembering the `W_ListObject` adds nothing the array barrier does not already do. Upstream emits no barrier call of its own: pyjitpl never executes one (executor.py:446) and `COND_CALL_GC_WB` is neither can-raise nor a call (resoperation.py:1124-1125). Carry the receiver on `FbwWalkMode` when the fold enters that arm, and skip recording a `list_write_barrier` call for exactly that receiver. The barrier still runs concretely during the walk, because the walk mutates the live heap. The suppression requires a GC-managed items block. Under `PYRE_GC_ITEMSBLOCK=0` the block is `std::alloc` memory with no GC header, the collector reaches its slots only through the remembered `W_ListObject`, and the barrier is kept. Two comments claiming pyre has no backend GC-rewrite pass are corrected. comprehension_object_append_hot: 30 -> 29 ops in the steady loop, and the loop carries no residual call. Measured -5.0% on that bench (user-CPU, startup-subtracted, interleaved median-9); a probe that appended a second barrier per iteration cost +9.0%, which bounds what the removed call was worth. Adds list_append_write_barrier_gc, which appends young elements to lists whose items block has aged out of the nursery, and gives the bench a max-pypy-ratio gate (it had none). Assisted-by: Claude
… gate `fbw_callee_body_replay_safety` admits a FOR_ITER-in-flight callee whose only unproven residual is a BINARY_OP a specialization table lowers to a native op. The accepted tag set was `Add` alone, so a body of `a - i`, `a * 2` or `a += i` stayed a per-iteration residual call. Accept every tag whose lowering has no runtime decline left. Both `try_walker_specialize_binary_op_int` and `..._float` key each in-place tag to the same arm as its plain form, so `Add` / `Subtract` / `Multiply` and `InplaceAdd` / `InplaceSubtract` / `InplaceMultiply` are accepted together. `And` / `Or` / `Xor` and their in-place tags are in the int table only, so they are accepted under a new exact-plain-int argument classification. The divide, remainder, shift, power, subscript and matrix-multiply tags stay excluded; the per-tag reasons are in the predicate's doc comment. Derive the argument classification from `is_plain_int1` / `is_plain_float_strict` rather than `is_int` / `is_float`. The latter are `ob_type` checks a numeric subclass passes, while `walker_int_specialization_operands` and `walker_float_specialization_operands` both require `is_exact_builtin_instance`; the predicate claimed a specialization that would not happen, admitting a body whose surviving residual a replay re-applied (`synth/polymorphic_binary_receiver` printed 112692374 for 112973374). `bool` arguments are no longer admitted. Add `PYRE_FBW_INLINE_DIAG` prints at the inline entry, the callee resolution and the FOR_ITER gate, and `bench/synth/inlined_helper_arith_hot.py` with one loop per accepted tag. Assisted-by: Claude
youknowone
force-pushed
the
single-walker
branch
from
July 25, 2026 13:03
22846b5 to
bb1e8a0
Compare
youknowone
changed the title
youknowone
added a commit
that referenced
this pull request
Jul 26, 2026
… rejection; object: root the block across a list regrow; interpreter: PyObject_Format fast path (#807) * jit: reject numeric subclasses in the float binop/compare specialization `walker_int_specialization_operands` rejects an operand that is not an exact builtin instance, because the raw int path bypasses special-method dispatch. `walker_float_specialization_operands` carried no such check: it classified its operands with `is_int` / `is_float`, which read `ob_type` and are true for a subclass. The `guard_class` the consumers emit reads `ob_type` too, so the subclass was not caught at runtime either. A float subclass overriding `__add__` / `__radd__` / `__sub__` / `__mul__` / `__truediv__` therefore lost the override once the loop compiled, on both backends. The helper is shared with the float COMPARE_OP specialization, so `__lt__` / `__eq__` were bypassed as well. Add the same exactness check to the float helper. `bench/synth/float_subclass_binop_dispatch.py` drives each dunder hot; with the check reverted it reports the raw IEEE result for 7 of its 9 lines, and its `int` subclass control is unaffected either way. Drop the `max-pypy-ratio` gate from `comprehension_object_append_hot`. pypy runs it in ~0.019s, two ticks of the 10ms user-CPU resolution; a CI run in which pyre went from 1.22s to 1.09s crossed the bound because pypy measured 0.03s instead of 0.04s. The bench keeps its output check. Assisted-by: Claude * jit: prove operand provenance before exempting a BINARY_OP from the FOR_ITER inline gate `residual_call_is_specialized_plain_numeric_binop` admitted a callee body on `args_all_exact_numeric` / `args_all_exact_plain_int`, which describe the callee's incoming arguments, and claimed from that the body's `BINARY_OP` residual would be specialized away. The two are only the same fact when the binop's operands ARE those arguments. In `def f(x): return G - x` the left operand is a module global reached through a `LoadGlobal` the body scan already treats as a replay-safe read, so a numeric subclass there declines the specialization and leaves a residual the admission promised was not coming. `fbw_callee_body_replay_safety` now carries the missing fact alongside the freshness it already tracks: which registers hold a value whose class is provably an immutable builtin. Three sources close over it — an incoming argument, read out of a `localsplus` slot the exact-positional entry convention bound (tracked through `getarrayitem_vable_r` / `setarrayitem_vable_r`, so `LOAD_FAST` / `STORE_FAST` propagate); a `LoadConst` / `BoxInt` / `NewtupleFromArray` result; and an already-accepted `BINARY_OP` result. The callee's ref constant pool is seeded by testing the constant objects outright with the exactness the specialization itself demands, since they are reachable at gate time. `LoadGlobal` is excluded. The predicate now decodes the residual's R-list and requires both operand registers to be in that set. The accepted tag set is unchanged: all 16 admitted tags still admit and the 7 excluded ones still decline, measured per-tag. No observable wrong answer was constructed either way — with the operand proof disabled the hazardous shapes are admitted yet the effect still lands once, because the cases that realize the replay are caught downstream — so this closes a latent gap rather than a live bug. `bench/synth/inline_gate_operand_provenance.py` covers the three shapes that reach an unproven operand plus an arguments-only control. Reported by the Codex parity review on #782 (sections 1 and 3; one bug, widened in blast radius by that PR's larger tag set). Assisted-by: Claude * object: root the old items block, not each of its items, across a list regrow `grow_list_items_block_gc` pinned every one of the old block's `live_len` items before allocating the new block, then read each one back off the shadow stack. Both halves call `gc_current_object_address`, so a resize cost `2 * live_len` GC ownership queries — each a TLS lookup, a `RefCell` borrow, an arena range test and a rawmalloced-set probe. The block itself is a traced GC array of GC pointers (`PY_OBJECT_ARRAY_GC_TYPE_ID` is registered `TypeInfo::varsize` over `PyObjectRef` slots), so rooting it keeps every item reachable and lets a collection inside the allocation relocate the slots in place. One root replaces `live_len` of them and the copy becomes a `copy_nonoverlapping`. A block that the nursery declined and `std::alloc` supplied cannot move, so it is not rooted at all. That is also the upstream shape: `_ll_list_resize_really` (rlist.py:262-267) mallocs the new array and `ll_arraycopy`s into it, with no per-item root bracket. Resizes are ~1% of the appends in a list build but dominated its profile: building 8M elements, work-thread samples under `w_list_grow_items_block` were 56% of the run, almost all inside `pin_root` / `shadow_stack_get` → `dynasm_gc_owns_object` → `OldGen::contains`. Interleaved A/B, same binary, 7 rounds, median user-CPU, startup subtracted, `[<expr> for i in range(n)]` over 8M elements: form per-item pin block root speedup none 0.2656s 0.1062s 2.50x str 0.2631s 0.1046s 2.51x tuple 0.3215s 0.2089s 1.54x int 0.1237s 0.1284s 0.96x (Integer strategy; not this path) `bench/synth/list_append_write_barrier_gc.py` gains `big_live_len_regrow`, which drives resizes at the largest live lengths it can with allocation pressure against the growth boundary and distinct per-slot identities, so a copy that read pre-collection addresses would show up. The whole fixture matches CPython 3.14 and PyPy across dynasm/cranelift x JIT/no-JIT/`PYRE_GC_ITEMSBLOCK=0`. Assisted-by: Claude * interpreter: take PyObject_Format's exact-str/int fast path, and enforce the int digit limit in the long conversion `format_value_dispatch` gated its empty-spec fast path on the resolved `__format__` being a `BUILTIN_FUNCTION_TYPE`, which no type-dict entry ever is: type-slot builtins are built by `make_builtin_function_with_arity` → `function_new_with_fixed_code`, so they are `FUNCTION_TYPE` with `can_change_code = false` (they have to be descriptors to bind), and `BUILTIN_FUNCTION_TYPE` is reserved for module-level builtins, which are deliberately not descriptors. `type(int.__format__)` is therefore `function`, the guard was always true, and every `f"{i}"` resolved and called `int.__format__` through the generic call machinery to reach a body that computes exactly what the skipped fast path computes. `PyObject_Format` answers this with an exactness test ahead of the lookup: an empty spec on an exact `str` or `int` cannot reach an override, so nothing is resolved or called. `bool` stays out — `PyLong_CheckExact` rejects it and `f"{True}"` is `True`, not `1`. `sys.set_int_max_str_digits` was enforced only inside `int_to_decimal_string`, whose one caller was the `int.__repr__` descriptor, so a conversion that did not go through that descriptor skipped the limit. `long_to_decimal_string` enforces it in the conversion, so `builtin_leaf_repr_string`'s `long` arm now runs `int_to_decimal_string` and the helper returns a `Result`. The `int` arm needs no check: a machine int is at most 19 digits, below the 640 floor `sys.set_int_max_str_digits` accepts. `[f"{i}" for i in range(1000)]` x 2000, median-5 user-CPU with startup subtracted, against CPython 3.14: 24.35x -> 12.95x. check.py dynasm 323/323, cranelift 323/323, wasm 320/320. Assisted-by: Claude
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent levers on the same FOR_ITER-body inline path. Commit 1 removes
a residual the walker was still recording inside an admitted callee; commit 2
widens which callees get admitted in the first place.
1. Stop recording
list_write_barrieron the in-place Object-append armThe #171 append fold descends
w_list_appendand folds the in-place Objectarm's store to
SetarrayitemGc. That arm also callslist_write_barrier,which is
#[dont_look_inside], so the sub-walk left it in the trace as aCallNresidual — one per iteration of an Object-strategy comprehension. Itwas the only residual call left in that loop.
The backend GC rewrite already marks the same store: a
SetarrayitemGcwith aRef value routes through
handle_write_barrier_setarrayitem(
rewrite.py:936-944) intoCOND_CALL_GC_WB_ARRAY, and the rewriter runs inproduction (
runner.rsinvokes it whenevergc_sync::is_initialized()). Thein-place arm does not replace the list's
itemspointer, so remembering theW_ListObjectadds nothing the array barrier does not already do.Upstream emits no barrier call of its own — pyjitpl never executes one
(
executor.py:446), andCOND_CALL_GC_WBis neither can-raise nor a call(
resoperation.py:1124-1125). The barrier in a compiled loop is only ever theone the backend rewrite inserts.
What this is not
It is not "delete the barrier from
w_list_append". That function runs inthe interpreter too, where no rewriter exists — an empty body SIGABRTs.
Upstream carries two mechanisms for one source: the GC transform covers
translated code, the backend rewrite covers traces. pyre's hand-written barrier
is the transform stand-in, so it stays; only the recording of it stops.
FbwWalkMode::append_inplace_wb_covered_receivercarries the receiver address(not a flag, so a barrier reached for any other list inside the sub-walk is
still recorded).
orthodox_list_append_commitsets it when the fold enters thein-place arm; the record site in
residual_call.rsconsumes it. The barrierstill runs concretely during the walk, because the walk mutates the live heap.
Suppression requires a GC-managed items block. Under
PYRE_GC_ITEMSBLOCK=0theblock is
std::allocmemory with no GC header, the collector reaches its slotsonly through the remembered
W_ListObject, and the barrier is load-bearing —verified 0 barrier calls in the loop by default, 10 retained off-GC.
Two comments claiming pyre has no backend GC-rewrite pass are corrected
(
residual_call.rs,jit_fnaddr.rs); both are stale on this branch.Effect
comprehension_object_append_hotsteady loop: 30 → 29 ops, and no residualcall remains (
call_may_force=0both before and after).-5.0% on that bench (user-CPU, startup-subtracted, interleaved median-9).
Sizing was established by a probe rather than inferred: appending a second
barrier per iteration — safe, since the barrier is idempotent — cost +9.0%,
which bounds what the removed call was worth. An earlier estimate of 20-55%
circulating from an RCA was not measured and is wrong.
2. Accept the full specialized binop tag set in the FOR_ITER inline gate
fbw_callee_body_replay_safetyadmits a FOR_ITER-in-flight callee whose onlyunproven residual is a
BINARY_OPa specialization table lowers to a nativeop — nothing survives for a replay to double. The accepted tag set was
Addalone. A one-line helper of
a - iora += itherefore stayed a residualcall on every iteration.
The tag audit
Accept every tag whose lowering has no runtime decline path left. Both
try_walker_specialize_binary_op_intand..._floatkey each in-place tag tothe same match arm as its plain form, so the two forms are admitted
together:
Add/Subtract/MultiplyIntAddOvf/IntSubOvf/IntMulOvf,needs_concrete_check=falseFloatAdd/FloatSub/FloatMulAnd/Or/XorIntAnd/IntOr/IntXor,needs_concrete_check=falseFloorDivide/Remainderi64::MIN / -1divisorTrueDividedescr_truedivstays recordedLshiftSHLmasks the count mod 64; the guarded form breaks the cranelift bridge)Rshift>= LONG_BITcountPower_powbut keeps a cold-path residualSubscr/MatrixMultiplyThe same partition shows up independently in the bignum table added by #781:
try_walker_specialize_binary_op_long_intcovers exactly Add/Sub/Mul/And/Or/Xorplus their in-place forms, with Lshift/Rshift split into a separate function.
Because
And/Or/Xorare int-table-only, they need a stricter argumentclassification than the other six — hence the second flag rather than widening
the existing one.
The exactness fix
The argument classification now derives from
is_plain_int1/is_plain_float_strictinstead ofis_int/is_float. The latter areob_typechecks that a numeric subclass passes, while bothwalker_int_specialization_operandsandwalker_float_specialization_operandsrequire
is_exact_builtin_instance— correctly, since a subclass keeps thebuiltin layout while its Python-visible class lives in
w_classand may defineits own
__add__.So the predicate was claiming a specialization that would not happen: the
residual survived, and a replay re-applied it. This is not hypothetical —
widening the tag set with the old predicate made
synth/polymorphic_binary_receiverprint112692374for112973374on bothbackends. The fix also closes that hole on the pre-existing
Addpath.boolarguments are no longer admitted.
A limitation is documented rather than papered over: both flags describe the
callee's incoming arguments, not the operands of the binop itself, which this
straight-line scan cannot name. A body can reach a non-numeric operand through
LoadConst/LoadGlobal, which the scan already treats as replay-safe reads.The in-place tags do not widen that hole — an in-place result must be
stored back, and every store target outside the callee's own registers
(
STORE_GLOBAL/STORE_ATTR/STORE_SUBSCR) is itself an unproven residualthat fails the scan first. Verified: a
G += [i]body is still classifiedDirty.Effect
One micro-bench per tag, gate metric (user CPU, startup-subtracted), dynasm.
The "before" column is the state after
Subtract/Multiplywere added butbefore the audit widened the set further, so
add/sub/mul/f*are alreadyCleanthere and serve as controls:add/sub/mul(control)Clean→Cleanfadd/fsub/fmul(control)Clean→CleaniaddDirty→CleanisubDirty→CleanimulDirty→CleanfiaddDirty→Cleanand/or/xorDirty→Cleaniand/ior/ixorDirty→Cleanfloordiv/mod/truediv/ftruedivDirty→Dirtylshift/rshift/powDirty→DirtyTen tags cross from
DirtytoCleanand land on the control's timing. Theseven excluded tags stay
Dirtyand stay at ~0.20-0.26s, as intended.Verification
check.py --backend dynasm,cranelift— 315/315 bothcargo test --all --no-default-features --features dynasm— no failurescargo fmt --all --check— cleanlist_append_write_barrier_gc.py: appends young elements tolists whose items block has aged out of the nursery, then reads every element
back. Matches CPython 3.14 across dynasm/cranelift × JIT /
PYRE_NO_JIT=1/PYRE_GC_ITEMSBLOCK=0.inlined_helper_arith_hot.py: one hot loop per accepted tag, soa tag dropping back out of the set shows up as the callee reappearing as a
residual. Output matches CPython 3.14 on both backends; measured x5.7
(dynasm) / x5.4 (cranelift) against a
max-pypy-ratio=12header.against CPython 3.14 on dynasm, cranelift, and
PYRE_NO_JIT=1.comprehension_object_append_hot.pygets amax-pypy-ratio=40header; it hadno gate at all. Measured x29.6 (dynasm) / x30.3 (cranelift) on a loaded box.
A 5% win cannot be gated by a pypy ratio — the noise band is wider than the
signal, and
.jitstatscarries onlyloops_compiled/bridges_compiled("watches what the JIT compiles, never how well"), so it cannot gate op
counts either. 40 is set to catch a large structural regression without
becoming a chronic red.
🤖 Generated with Claude Code