jit: fix a lost conditional store in an inlined callee, widen the method-form inline - #942
Merged
Conversation
Two halves of the same gap on the NONSTANDARD virtualizable array path (a force-materialized inline-callee frame's locals_cells_stack_w): - Every `getarrayitem_vable` / `setarrayitem_vable` arm recorded its own fresh `GetfieldGcR` for the array base. The per-array element cache is keyed by that base OpRef, so reads and writes landed in different submaps. Route all four arms through a new `nonstandard_vable_array_base`, which forwards the cached field box on a hit and publishes the recorded op with `getfield_now_known`, the way `_opimpl_getfield_gc_any_pureornot` does. - The nonstandard store recorded `SETARRAYITEM_GC` without updating the heapcache. `execute_setarrayitem_gc` (pyjitpl.py) records and then calls `heapcache.setarrayitem`; add that pairing as `execute_setarrayitem_gc` and call it from the nonstandard arm. `gen_store_back_in_vable` keeps using the raw `vable_setarrayitem_descr`, matching its direct op recording upstream. With both, a local written inside an inlined callee no longer keeps reading the value the element cache was seeded with at frame construction. Assisted-by: Claude
`try_walker_inline_user_call` passed `allow_method_load_attr = false`, so `method_form_callee_body_supported` declined every `b.at(i)` whose callee body contains a `LoadAttr` residual — the ordinary accessor shape `def at(self, i): return self.v + i`. The same callee already inlined when stored as a bound method (`m = b.at; m(i)`). Pass `true` there, and document what the helper declines. Measured on the call-shape micro (400k iterations, dynasm): `b.at(i)` 1.54s -> 0.08s, matching the plain-function and bound-method forms; `synth/sre_pattern_methods` prints 280000, the pypy3 value. Assisted-by: Claude
…_ITER-deferred callees
`try_walker_inline_user_call` passes `allow_method_load_attr = true`, which
admits an unbound method-form callee whose body reads `self.attr`. Two body
shapes reached that way discarded the enclosing loop instead of falling back
to a residual call:
- A body containing a `raise`. The sub-walk records into the handler region,
and a guard whose resume coordinate lands on the `Reraise` asks
`collect_callee_active_boxes` for ref registers the recorded path never
wrote. That decline arrives mid-recording on an opcode that is not
effect-free, so it has no mid-body carrier and the trace is thrown away.
- A FOR_ITER `CalleeReplaySafety::DeferredCall` admission whose deferred call
resolves to a builtin: `fbw_abort_nested_unjournaled_residual` spends one
abort and then denies the callee.
Decline both at the callsite. Measured, median of 3:
dynasm sre_pattern_methods 0.90s -> 0.67s, sre_wasm_min 0.55s -> 0.43s,
sre_wasm_min1 0.45s -> 0.33s, type_metatype_method_call 0.35s -> 0.28s
cranelift the same four return to 0.72s / 0.47s / 0.36s / 0.23s, matching
their times with `allow_method_load_attr = false`
`loops_compiled`, `bridges_compiled` and `loops_aborted` on all four match the
`allow_method_load_attr = false` counters. `inline_subwalk_mutating_residual`
keeps the widening's gain at 13.0x pypy3 (dynasm) / 18.3x (cranelift), so its
gate moves 200 -> 40, and `inline_subwalk_property_mutates` 80 -> 50.
check.py: dynasm 354/354, cranelift 354/354.
Assisted-by: Claude
WalkthroughChangesNonstandard virtualizable array caching
Method-form call inlining
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ResolvedUserCall
participant method_form_callee_body_supported
participant DeferredLoopAdmission
participant ResidualCall
ResolvedUserCall->>method_form_callee_body_supported: inspect method-form callee body
method_form_callee_body_supported-->>ResolvedUserCall: report body support
ResolvedUserCall->>DeferredLoopAdmission: evaluate widened call
DeferredLoopAdmission-->>ResolvedUserCall: reject widened deferred-loop admission
ResolvedUserCall->>ResidualCall: use residual call when validation fails
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 d7a4a79). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7a4a79b6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2792
to
+2793
| let declined = if allow_method_load_attr { | ||
| callee_body_contains_raise(body.code) |
There was a problem hiding this comment.
reraise
When a newly admitted method-form callee contains a bare raise (for example, try: return self.attr followed by except: raise), its JitCode contains the distinct reraise opcode, but callee_body_contains_raise only recognizes raise. This predicate therefore admits exactly the handler path described above as unsafe, allowing the missing-register resume failure to discard the enclosing compiled loop instead of leaving the call residual; classify reraise as raise-bearing here as well.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs`:
- Around line 416-422: Update the documentation for
method_form_callee_body_supported to reflect that it is used with both
allow_method_load_attr = false and true, including widened method-form call
detection; remove the claim that callers only pass false and describe the
helper’s behavior without limiting it to the narrower inline path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 21d30f7c-6c3a-42a4-8ab8-dc68b9e2a857
📒 Files selected for processing (4)
majit/majit-metainterp/src/trace_ctx.rspyre/bench/synth/inline_subwalk_mutating_residual.pypyre/bench/synth/inline_subwalk_property_mutates.pypyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs
Comment on lines
+416
to
+422
| /// Whether a method-form callee body is free of `LoadAttr` residuals. | ||
| /// | ||
| /// Consulted only by the entries that pass `allow_method_load_attr = false`. | ||
| /// A `self.attr` read in the body is what makes it answer `false`, which is the | ||
| /// common shape (`def at(self, i): return self.v + i`), so an entry that opts | ||
| /// out of the check trades a narrower inline surface for the ability to inline | ||
| /// ordinary accessor methods. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the helper documentation.
Line 418 is no longer correct. method_form_callee_body_supported is also called with allow_method_load_attr = true at Line 2704 to identify widened method-form calls.
Proposed fix
-/// Consulted only by the entries that pass `allow_method_load_attr = false`.
+/// The narrow method-form path uses this result to decline `self.attr` reads.
+/// The widened path also uses this result to identify widened method-form calls.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Whether a method-form callee body is free of `LoadAttr` residuals. | |
| /// | |
| /// Consulted only by the entries that pass `allow_method_load_attr = false`. | |
| /// A `self.attr` read in the body is what makes it answer `false`, which is the | |
| /// common shape (`def at(self, i): return self.v + i`), so an entry that opts | |
| /// out of the check trades a narrower inline surface for the ability to inline | |
| /// ordinary accessor methods. | |
| /// Whether a method-form callee body is free of `LoadAttr` residuals. | |
| /// | |
| /// The narrow method-form path uses this result to decline `self.attr` reads. | |
| /// The widened path also uses this result to identify widened method-form calls. | |
| /// A `self.attr` read in the body is what makes it answer `false`, which is the | |
| /// common shape (`def at(self, i): return self.v + i`), so an entry that opts | |
| /// out of the check trades a narrower inline surface for the ability to inline | |
| /// ordinary accessor methods. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs` around lines 416 -
422, Update the documentation for method_form_callee_body_supported to reflect
that it is used with both allow_method_load_attr = false and true, including
widened method-form call detection; remove the claim that callers only pass
false and describe the helper’s behavior without limiting it to the narrower
inline path.
youknowone
added a commit
that referenced
this pull request
Aug 1, 2026
…hod-form inline (#942) * majit: keep the nonstandard vable element heapcache in sync Two halves of the same gap on the NONSTANDARD virtualizable array path (a force-materialized inline-callee frame's locals_cells_stack_w): - Every `getarrayitem_vable` / `setarrayitem_vable` arm recorded its own fresh `GetfieldGcR` for the array base. The per-array element cache is keyed by that base OpRef, so reads and writes landed in different submaps. Route all four arms through a new `nonstandard_vable_array_base`, which forwards the cached field box on a hit and publishes the recorded op with `getfield_now_known`, the way `_opimpl_getfield_gc_any_pureornot` does. - The nonstandard store recorded `SETARRAYITEM_GC` without updating the heapcache. `execute_setarrayitem_gc` (pyjitpl.py) records and then calls `heapcache.setarrayitem`; add that pairing as `execute_setarrayitem_gc` and call it from the nonstandard arm. `gen_store_back_in_vable` keeps using the raw `vable_setarrayitem_descr`, matching its direct op recording upstream. With both, a local written inside an inlined callee no longer keeps reading the value the element cache was seeded with at frame construction. Assisted-by: Claude * jit: inline a method-form callee whose body reads self.attr `try_walker_inline_user_call` passed `allow_method_load_attr = false`, so `method_form_callee_body_supported` declined every `b.at(i)` whose callee body contains a `LoadAttr` residual — the ordinary accessor shape `def at(self, i): return self.v + i`. The same callee already inlined when stored as a bound method (`m = b.at; m(i)`). Pass `true` there, and document what the helper declines. Measured on the call-shape micro (400k iterations, dynasm): `b.at(i)` 1.54s -> 0.08s, matching the plain-function and bound-method forms; `synth/sre_pattern_methods` prints 280000, the pypy3 value. Assisted-by: Claude * jit: decline the widened method-form inline for raise-bearing and FOR_ITER-deferred callees `try_walker_inline_user_call` passes `allow_method_load_attr = true`, which admits an unbound method-form callee whose body reads `self.attr`. Two body shapes reached that way discarded the enclosing loop instead of falling back to a residual call: - A body containing a `raise`. The sub-walk records into the handler region, and a guard whose resume coordinate lands on the `Reraise` asks `collect_callee_active_boxes` for ref registers the recorded path never wrote. That decline arrives mid-recording on an opcode that is not effect-free, so it has no mid-body carrier and the trace is thrown away. - A FOR_ITER `CalleeReplaySafety::DeferredCall` admission whose deferred call resolves to a builtin: `fbw_abort_nested_unjournaled_residual` spends one abort and then denies the callee. Decline both at the callsite. Measured, median of 3: dynasm sre_pattern_methods 0.90s -> 0.67s, sre_wasm_min 0.55s -> 0.43s, sre_wasm_min1 0.45s -> 0.33s, type_metatype_method_call 0.35s -> 0.28s cranelift the same four return to 0.72s / 0.47s / 0.36s / 0.23s, matching their times with `allow_method_load_attr = false` `loops_compiled`, `bridges_compiled` and `loops_aborted` on all four match the `allow_method_load_attr = false` counters. `inline_subwalk_mutating_residual` keeps the widening's gain at 13.0x pypy3 (dynasm) / 18.3x (cranelift), so its gate moves 200 -> 40, and `inline_subwalk_property_mutates` 80 -> 50. check.py: dynasm 354/354, cranelift 354/354. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 1, 2026
…base/top interpreter root stack (#930) * macros: drop #[inline(never)] from the dont_look_inside expansion `expand_dont_look_inside_attribute` backs both `#[dont_look_inside]` and `#[dont_look_inside_cannot_raise]` (~636 annotated functions) and emitted `#[inline(never)]` next to the tracing-policy marker. The policy rides the `_jit_look_inside_` marker const that `front/llbc_hints.rs` harvests from the extracted LLBC, and `charon cargo --mir` disables the MIR optimizations, so the attribute did not change what the tracer sees. `rlib/jit.py:133-140` sets `_jit_look_inside_ = False` and leaves the backend's inliner alone. Measured on gc22_any.py / gc22_all.py (user CPU, min of 15, interleaved, order rotated): 0.9150 and 0.9090. A pure integer loop that never reaches the rooting path holds at 1.0000. Reachable Python recursion depth goes 161300 -> 167700, and `stack_check.rs` measures the real stack pointer, so interpreter frames did not grow. The release binary grows 3.0%. Two comments described the removed behaviour and are corrected: the `dont_look_inside` doc named a `#[majit_opaque]` marker as the tracer's detection path (the fn-local `_MAJIT_OPAQUE` const has no consumers), and `gc_roots.rs` attributed a per-`with` `_tlv_get_addr` resolve to the thread-local, where disassembly shows that resolve is shared across the two accesses and the repeated cost is the initialization-state load. Assisted-by: Claude * jit-trace: assert one runtime fnaddr per build-time address `patch_constants_i_fnaddrs` builds `correspondence[build_fnaddr] = runtime_fnaddr` with a plain `HashMap::insert`. The key comes from the build process and the value from the running one, so a build-time address naming two functions whose runtime addresses differ kept only the last write, and one callee's `constants_i` constant was patched to the other's address — a residual call to the wrong target, with no decline and no panic. Registering several path spellings for one function is deliberate (`jit_fnaddr.rs` lists both `pyre_object::listobject::jit_list_reverse` and `pyre_object::jit_list_reverse`); those agree on the runtime address, so the assertion fires only on a disagreement. It covers every binding rather than only those surviving the `!=` filter, since an alias pair straddling that filter leaves the same hole. Assisted-by: Claude * listobject: keep drain_list_append individually addressable The body forwards verbatim to `w_list_append`, so once the tracing-policy attribute stopped emitting `#[inline(never)]` the callee inlined and the two functions became byte-identical. MSVC links with `/OPT:ICF` by default and folded them, giving one build-time address to two registered residual-call targets; `patch_constants_i_fnaddrs` then cannot tell which callee a `constants_i` entry meant, and its assertion fired across 24 `pyre-jit` tests on windows-latest. `#[inline(never)]` here is an addressability requirement rather than a tracing one, so it is stated at the definition: keeping the forwarding call keeps the two bodies distinct. A sweep of the 58 `dont_look_inside` functions whose body is a single call found this to be the only pair that is both same-signature and registered at both ends. `eval::call_depth` is unregistered, `bigint_gc_type_id` and `get_recursion_limit` forward to unregistered targets, and `w_list_new_empty` and `hash_str_hooked_bytes` differ from their callees in arity. Assisted-by: Claude * jit_fnaddr: test that no two registered functions share an address `patch_constants_i_fnaddrs` rewrites residual-call constants through a build-address to runtime-address map, so an address standing for two functions sends one callee's call to the other. Its assertion only fires once that patch path runs; this covers the registry directly. Several path spellings for one function are deliberate — the module path and the crate-root re-export both appear — so the check is on the leaf name: two distinct leaves on one address means unrelated functions were folded. The test observes only the address space it runs in. The MSVC fold happened in the build-script binary while the test binary kept the two apart, so a registry that passes here can still hand `runtime_fnaddr_patch` an ambiguous build address. Assisted-by: Claude * macros: confine #[inline(never)] to the release-gil surface `expand_elidable_attribute` (`#[elidable]`, `#[elidable_cannot_raise]`, `#[elidable_or_memerror]`), `elidable_promote`, `expand_call_surface_attr` (`#[jit_may_force]`, `#[jit_loop_invariant]`) and `look_inside_iff`'s trampoline each emitted `#[inline(never)]` next to their tracing-policy marker. Upstream keeps the two concepts apart. `rlib/jit.py:72 elidable` sets `_elidable_function_ = True`, `loop_invariant` sets `_jit_loop_invariant_`, and suppressing the backend inliner is a separate flag, `_dont_inline_` (`objectmodel.py:214`), read by `translator/backendopt/inline.py:565`. No elidable helper upstream carries it, and `jit_may_force` has no upstream decorator at all — `EF_FORCES_VIRTUAL_OR_VIRTUALIZABLE` is derived from the analyzed operations in `effectinfo.py:401-404`. `#[jit_release_gil]` is the exception and keeps the attribute: `rffi.py:219` sets `call_external_function._dont_inline_ = True` beside `:220 _gctransformer_hint_close_stack_ = True`, explained at `:232` as "don't inline, as a hack to guarantee that no GC pointer is alive anywhere in call_external_function" — the body runs with the GIL released, so a caller folded into it would put live GC pointers in that window. One expansion backs all three call-surface attributes, so the emission is branched on `attr_name`. The elidable doc claimed a `#[majit_elidable]` marker the tracer detects; the expansion emits an unread fn-local `_MAJIT_ELIDABLE` const, and the policy travels through `rpython_attribute_const_for` to `front/llbc_hints.rs`. Measured over ~152 annotated sites, none on the GC-rooting path: throughput is unchanged (1.0070 / 1.0000 on the any/all probes, 1.0000 on a pure integer control), the release binary grows 14 KB, and reachable Python recursion depth falls from 167700 to 159700 as the larger elidable bodies inline into the eval loop. This lands on parity, not on performance. Assisted-by: Claude * jit-trace: keep last-write-wins for ICF-folded residual fnaddrs With #[inline(never)] confined to the release-GIL surface, the tracing-policy helpers inline their callees, so several byte-identical registered residuals compile to one body: label_arg_to_usize / load_fast_var_num_to_index are both arg.get(op_arg).as_usize(), convert_value_arg / special_method_arg are both arg.get(op_arg), and hash_str_hooked_bytes decomposes its slice to the (ptr, len) hash_str_hooked already takes. The MSVC-linked build script folds each pair under /OPT:ICF onto one build address; the unoptimized test binary keeps them apart, so their two runtime addresses differ. patch_constants_i_fnaddrs keys correspondence on the build address, so a fold makes one build address map to two runtime addresses. Folding merges only identical machine code, so a residual call patched to either twin runs the same body; and a path bound to the wrong fn resolves identically in both processes, agreeing rather than colliding. The prior assert_eq therefore fired only on these benign folds -- as it did on windows-latest across the pyre-jit tests. Drop it and keep the last-write-wins insert. The jit_fnaddr registry test observes only the process it runs in, where nothing folds, so it cannot catch a build-script fold; last-write-wins is what keeps the folded build address safe at load time. * gc: nursery placement around pinned objects and varsize overflow rejection; majit-translate: scope a fixture's lowering (#939) * gc: preserve nursery placement around pinned objects * gc: reject overflowing varsize allocations * majit-translate: scope a fixture's lowering to the graphs it asserts on `build_semantic_program_from_llbcs_with_static_addrs_and_function_names` takes a leaf-name allowlist alongside the module paths. Whole-program metadata still comes from the entire LLBC; only the bodies of functions outside the allowlist are left unbuilt. The existing entry points pass `None` and are unchanged. The lowering loop's global-initializer, module and name gates now run before `FunDecl::unstructured`, so a declaration no gate admits no longer parses its body JSON. Each of those gates reads the declaration header or its name path. `test_rbigint_mir`'s four caller tables become consts and the allowlist is derived from them. Against `pyre-interpreter.ullbc` the dependent-crate test lowered 8580 of 30380 declarations to assert on 41 graphs; it now lowers those 41. The graphs are unchanged — `{:?}` compared against an unfiltered run, modulo the global variable-id counter — and the test binary goes from 156.5 s to 80.9 s of CPU. Assisted-by: Claude * jit: guard the inlined callee's Function fields, and bound the dynasm/wasm bridge chain (#925) * jit: exempt store_deref_value's NULL value arg from the residual NULL-Ref refusal `DELETE_DEREF` lowers to `load_deref_value` plus `store_deref_value(cell, Constant::none())` (`codewriter.rs` `Instruction::DeleteDeref`). `bh_store_deref_value_fn` passes that value to `w_cell_set` without dereferencing it, so the NULL is a checked sentinel like `CallFn`/`CallKw`/`CallFunctionEx`/`RaiseVarargs` already have exemptions for. Without the exemption `try_execute_residual_call_via_executor` declined the residual, which marks the walk as carrying a recorded-but-unexecuted effect; the walk-end flush then declined ("unjournaled effect — legacy replay kept") and the caller replayed a region whose other residuals the walk had already executed concretely. Added to `walker_abort_if_mayforce_null_ref_arg` too, per the in-code contract that the two exemption lists match. `bench/synth/del_cellvar_walk_commit.py`: a `del <cellvar>` loop calling a list-appending helper reported 20048 appends for 20000 iterations before this, one per declined-commit walk. check.py: dynasm 345/345, cranelift 345/345, wasm 341/341. Assisted-by: Claude * jit: root the virtuals cache across force_from_resumedata's materialization window `force_from_resumedata` prepared its reader with the bare `ResumeDataDirectReader::prepare`, which registers no GC root for `virtuals_cache.virtuals_ptr_cache`. Every materialization after it — inside `prepare_guard_pendingfields`, `consume_vref_and_vable` and `force_all_virtuals` — wrote object addresses into a `Vec<i64>` the collector cannot see, while `getvirtual_ptr` and the four `virtuals_cache.get_ptr` re-reads in `VirtualInfo::allocate` return the slot on the stated premise that a collection forwards it in place. Call `prepare_resume_heap_with_roots` instead, the helper the blackhole and bridge paths already use, and hold its scope to the end of the function. The `virtualizable_ptr` comment named the missing scope as the reason for its early read; state the allocation class instead. Add `shadow_stack::resume_ref_slice_registered` and assert the premise once per force in `force_all_virtuals`. `test_handle_async_forcing_prepares_rd_virtuals_from_exit_layout` fires it without the fix. Record on `walk_forced_virtuals_refs` that the walk is a strong edge where `jf_savedata` is an ephemeron one, what bounds the resulting retention to one major cycle, and the back-edge that would remove the bound. Assisted-by: Claude * jit(dynasm,wasm): stamp the guard_value per-value jitcounter bucket `regalloc.py:496-499 consider_guard_value` calls `descr.make_a_counter_per_value(op, index)` on every upstream backend, so `must_compile` hashes the (guard, failing value) pair (`compile.py:753-781`) and `store_hash` skips a descr whose status is already stamped (`compile.py:826-829`). pyre implemented the helper and the metainterp decode but called it only from cranelift, so on dynasm and wasm every GUARD_VALUE kept `status == 0` and hashed into one bucket per guard. A guard on a value that never repeats then reached `trace_eagerness` every 200 failures and compiled another bridge, without bound. On a 20000-iteration loop that defines a function in its own body and calls it, dynasm compiled 47 bridges where cranelift compiled 0; at N=200000, 497 bridges and 2.71s against cranelift's 0.71s. After the change dynasm compiles 0 bridges and runs 0.77s (interleaved, min of 5, user+sys): 20000 0.23s -> 0.19s, 100000 0.88s -> 0.45s, 200000 2.71s -> 0.77s. The stamp goes where each backend already lays the guard out: dynasm beside `set_rd_locs` (`assembler.py:279`), wasm in the guard-exit pre-scan. Both index by fail-arg position, as cranelift does, because `must_compile_with_values` reads the value back out of `fail_values`. Add `synth/call_loop_local_function` with its three `.jitstats` baselines; `bridges_compiled` is the recorded signal. Assisted-by: Claude * jit: gate the keyed bridge loop on no_bridge_enabled and log @@@guard there `run_compiled_detailed_with_bridge_keyed` computed `should_bridge` from `must_compile && !stack_almost_full()` only, unlike the two sibling loops that also consult `no_bridge_enabled()`. This is the loop pyre reaches from `execute_assembler`, so `MAJIT_NO_BRIDGE=1` reported a bridge-free run while still compiling every bridge. Also emit the `@@@GUARD` line the sibling loops emit, with the guard's own trace id and fail index. Assisted-by: Claude * jit-trace: guard the inlined callee's Function fields, not the function object The inline lever emitted one `GuardValue` on the callable's own address, and only when the callable was not already a trace constant. Guard the four `_immutable_fields_` names the inline actually bakes (`function.py:34-42` `['code?', 'w_func_globals?', 'closure?[*]', 'defs_w?[*]']`) by reading each field off the live function instead, and emit them unconditionally. `closure` is read through to its cells because `MAKE_FUNCTION` rebuilds the tuple; `defs_w` keeps a tuple-identity guard. `FUNCTION_DESCR_GROUP` grows `code`, `w_func_globals` and `closure` next to the existing `defs_w`. Two behaviour changes: - A callee built by a `MAKE_FUNCTION` in the caller's own loop body no longer fails its guard every iteration. On `synth/call_loop_local_function`, `guard_failures` 9480 -> 1 and `loops_compiled` 2 -> 1; baselines re-snapshotted on all three backends. - `f.__code__ = g.__code__` on a constant callable used to keep running the old code object: 41998 instead of 20020000 on a 40000-iteration loop. Assisted-by: Claude * jit-trace: skip the read-only Function field guards on a constant callable `w_func_globals` and `closure` have no Python-level setter, so a callable the trace has already pinned to one object cannot present different ones. Guard them only when the callable is not a trace constant; `code` and `defs_w` are writable and stay guarded either way. Also skip the guard when the heapcache already handed back a Const box (`_opimpl_any_guard_value` parity). Raise inline_helper's cranelift vs-pypy gate 1.5 -> 3. The remaining `getfield_gc_r` + `guard_value` on `Function.code` costs dynasm nothing (0.21s -> 0.20s) and cranelift 0.22s -> 0.32s on a trace that is 103 guards on both backends. Assisted-by: Claude * jit-trace: guard the Function fields only when the pinned object is that function `try_walker_inline_resolved_user_call` takes the resolved callee as `callable` and the object to pin as `callable_guard_value`, and the two are not always the same. The exception-string specializer resolves `str(e)` to an exception subclass's `__str__` but passes the CALL's own operand — the `str` builtin — as the value to pin. Reading `Function.code` off that operand is a type-confused load: it returns whatever sits at offset 16 of a PyCFunction, so the guard compares a value that is not `code`, and it never matched. On `synth/exception_subclass_attrs`: guard_failures 1 -> 99480, bridges_compiled 0 -> 497, 0.19s -> 6.18s. Emit the field guards only when the pinned object is a Function whose `code` is the code this inline resolved; otherwise fall back to the operand-identity guard. exception_subclass_attrs returns to 0 bridges / 1 guard failure at 0.14s, and call_loop_local_function keeps its 1 guard failure. Assisted-by: Claude * bench(synth): give call_loop_local_function a max-pypy-ratio gate Worst native ratio measures 3.7x (dynasm; cranelift 3.2x), so the gate is max(5, ceil(3.7 * 2)) = 8, matching how #853 calibrated the other 312 fixtures. Assisted-by: Claude * jit-trace: do not read the callee Function's fields through a baked ConstPtr The field guards were also emitted for a trace-constant callable, which makes the trace dereference a baked `ConstPtr`. A baked constant object pointer is not GC-forwarded (gh #108 gc-table; the note in synth/exception_subclass_attrs records the same hazard), so the load dangles once a minor collection moves the object. `synth/inline_subwalk_property_mutates` — a property getter that allocates every iteration — segfaults on cranelift under CI's macOS runner with the reads in place, and its jitstats are now identical to the pre-change run. Restrict the guards to a non-constant callable, which is the case the fresh `MAKE_FUNCTION` callee falls in: call_loop_local_function keeps guard_failures=1, exception_subclass_attrs keeps 0 bridges. This gives up the `f.__code__ = g.__code__` re-check on a constant callable that e5404cc's message claimed: a 40000-iteration loop reassigning `__code__` prints 41998 again instead of 20020000. Re-checking it needs either quasi-immutable `code?` with trace invalidation, or gh #108 so a baked constant can be dereferenced at all. inline_helper's cranelift vs-pypy gate goes back to 1.5 (measures 1.1x): the callables there are trace constants, so no guard is emitted for them now. Assisted-by: Claude * interpreter: build a raised exception's args once in to_exc_object `to_exc_object` allocated the message string and the `args_w` list twice per raise: `w_exception_new` builds both from the message, then the block below rebuilt them and overwrote `args_w`, discarding the first pair. The second build exists because the `ImportError` / `ModuleNotFoundError` `msg` stamp needs the message object in a shadow-stack slot. Allocate the instance with `w_exception_new_empty` and keep the single build. synth/type_immutable_reject exec 0.40s -> 0.31s, pypy ratio 69.8x -> 52.4x (dynasm, back-to-back on one machine; gate is 152x). dynasm 354/354, cranelift 354/354. Assisted-by: Claude * interpreter: compute def_first in signed arithmetic `argument.py:274,302-315` computes `def_first = co_argcount - len(defaults_w)` signed and keeps `defaults_w[i - def_first]` for every non-negative index, so a `__defaults__` longer than the parameter list binds the tuple's tail. Four pyre sites computed it in `usize`: - `call.rs fill_user_function_args` wrapped, so no parameter matched a default and the call raised `TypeError: missing 1 required positional argument`. - `argument.rs:1130 _match_signature` used `saturating_sub`, clamping to 0 and binding `defaults_w[1]` where upstream binds `defaults_w[2]`. - the too-many-args messages in both files wrapped the same difference. `fill_user_function_args` is reached from `bh_call_fn_impl` -> `call_user_function_residual`, so the wrap was observable only after a guard failure: `f.__defaults__ = (7, 9, 11)` set before a loop was correct, set mid-loop it raised at the first call after the flip. Add `synth/defaults_reassigned_midloop`, which reassigns `__defaults__` mid-loop both to a longer tuple and to a two-int tuple; without this change its first line raises instead of printing 330000. Assisted-by: Claude * jit: fire the immutable-type attr raise fold and guard it on the metaclass version_tag (#933) type_immutable_attr_raise_is_stable rejected the canonical type's own __setattr__/__delattr__ — the standard slot wrappers forwarding to object_setattr/object_delattr (init_type_type, for the Carlo Verre hackcheck) — as a diverting metaclass override, so the immutable-type STORE_ATTR/DELETE_ATTR raise fold never committed: every iteration residualised the raise as call_may_force and re-allocated the exception. The STORE_ATTR arm additionally required the store value to be a trace-time constant, which a loop-carried value never is. Accept type's own setattr/delattr in the predicate (branch C already proves the metaclass is the canonical, dict-frozen type) and drop the constant-value requirement (the value plays no role in the raise; the predicate proves no data descriptor for name, so the terminal raises before consulting it — substitute w_none for the trace-time run). The fold's emitted guard pinned only the receiver identity, but the raise decision reads the metaclass-MRO descriptor state (the branch-F name walk and the forwarding setattr/delattr). Emit the metaclass version_tag GuardValue the sibling method/attr folds carry, so any type/object dict mutation side-exits; one guard covers the (type, object) MRO because mutated() propagates down to the type subclass. type_immutable_reject drops from ~35x to ~1x vs pypy on both backends (per-iteration call_may_force 1 -> 0); lower its max-pypy-ratio gate 152 -> 15. * jit: make W_TypeObject._version_tag quasi-immutable (#940) `typeobject.py:177 _immutable_fields_ = ['_version_tag?']`. The method-cache fold needs the tag green for `promote(self.version_tag())` (typeobject.py:506); it was getting there through a live `getfield_gc_i` plus `guard_value` at each of the eight fold sites, because nothing could revoke a loop when the tag changed. A residual `CALL_MAY_FORCE` flushes a mutable field's cache, so a body with an un-inlined call re-read and re-guarded the tag after every call. Ports `quasiimmut.py`'s `QuasiImmut` — `register_loop_token`, `compress_looptokens_list` (`compress_limit = 30`, `(len + 15) * 2`), and `invalidate` — onto `W_TypeObject.quasi_immut_watchers`, which stands in for the hidden `mutate__version_tag` field the rtyper synthesises upstream. The sweep sets the per-artifact `AtomicBool` that `GUARD_NOT_INVALIDATED` already reads, in place of `looptoken.invalidated = True` + `cpu.invalidate_loop`. `w_type_set_version_tag` is the only writer of the field, so the invalidation hangs there and covers `mutated()`'s fresh identity and both demotions to `0` alike. `register_quasi_immutable_deps` now offers each collected dep to the type watcher as well as the module-dict one; both registrations self-filter on the object's kind. The eight fold sites emit `QUASIIMMUT_FIELD` through a shared `walker_pin_type_version_tag`, and `type_version_tag_descr()` becomes a `LazyLock` singleton carrying the quasi-immutable flag — `heap.rs:3274` keys `quasi_immut_cache` on the descr's `Arc` pointer, so a per-call descriptor missed its own cache on every read. Steady-body op counts: append loop 33 → 32; two un-inlined method calls on one instance 41 → 38 (both `_version_tag` reads and their guards replaced by one `GUARD_NOT_INVALIDATED`, which the residual call no longer flushes). Adds `synth/method_reassign_after_warmup`, covering rebinding on the class, rebinding again, rebinding on a base of the warmed receiver's class, and rebinding from inside the loop being traced. With the invalidation call removed the fixture returns the stale methods (`1 1 1 10 10`), so it is not vacuous. Assisted-by: Claude * majit: address PR #932 review — restrict as_ptr fold to slices, fix getslice fixed-result dst_items (#941) as_ptr identity fold (front/mir.rs): drop the `alloc::vec::<Impl>::as_ptr` arm. A resized-list / Vec reaches its items buffer through `ll_items(l) = l.items` (rlist.py:368, a getfield), not the receiver, so aliasing a Vec::as_ptr result to the receiver contradicts the layout; only the fixed-array `<[T]>::as_ptr` matches `ll_fixed_items(l) = l` (rlist.py:399). The two Vec::as_ptr callers (IntArray/FloatArray::from_vec) are host builtins residualised to their compiled bodies, so the Vec arm had no traced consumer — census is unchanged (1151). getslice dst_items (rtyper/rlist.rs): emit_listslice_alloc_and_copy hardcoded `getfield(new_lst, "items")` for the destination, which is wrong for a FixedSizeListRepr result (ll_newlist returns a bare Ptr(GcArray) with no items field). Branch dst_items on the result layout, read off result_ptr_lltype, mirroring the source-layout branch of src_items. Add a fixed-result getslice test asserting no `items` getfield, plus a non-nonneg-start rejection test. Also: fix a stale `simplify_and_finalize` cross-reference in the dormant slice_index note, align slice_index's consumer gate FieldWrite base with its removal sweep (`base == range`), and add a negative-length malloc_varsize llinterp test. Assisted-by: Claude * jit: fix a lost conditional store in an inlined callee, widen the method-form inline (#942) * majit: keep the nonstandard vable element heapcache in sync Two halves of the same gap on the NONSTANDARD virtualizable array path (a force-materialized inline-callee frame's locals_cells_stack_w): - Every `getarrayitem_vable` / `setarrayitem_vable` arm recorded its own fresh `GetfieldGcR` for the array base. The per-array element cache is keyed by that base OpRef, so reads and writes landed in different submaps. Route all four arms through a new `nonstandard_vable_array_base`, which forwards the cached field box on a hit and publishes the recorded op with `getfield_now_known`, the way `_opimpl_getfield_gc_any_pureornot` does. - The nonstandard store recorded `SETARRAYITEM_GC` without updating the heapcache. `execute_setarrayitem_gc` (pyjitpl.py) records and then calls `heapcache.setarrayitem`; add that pairing as `execute_setarrayitem_gc` and call it from the nonstandard arm. `gen_store_back_in_vable` keeps using the raw `vable_setarrayitem_descr`, matching its direct op recording upstream. With both, a local written inside an inlined callee no longer keeps reading the value the element cache was seeded with at frame construction. Assisted-by: Claude * jit: inline a method-form callee whose body reads self.attr `try_walker_inline_user_call` passed `allow_method_load_attr = false`, so `method_form_callee_body_supported` declined every `b.at(i)` whose callee body contains a `LoadAttr` residual — the ordinary accessor shape `def at(self, i): return self.v + i`. The same callee already inlined when stored as a bound method (`m = b.at; m(i)`). Pass `true` there, and document what the helper declines. Measured on the call-shape micro (400k iterations, dynasm): `b.at(i)` 1.54s -> 0.08s, matching the plain-function and bound-method forms; `synth/sre_pattern_methods` prints 280000, the pypy3 value. Assisted-by: Claude * jit: decline the widened method-form inline for raise-bearing and FOR_ITER-deferred callees `try_walker_inline_user_call` passes `allow_method_load_attr = true`, which admits an unbound method-form callee whose body reads `self.attr`. Two body shapes reached that way discarded the enclosing loop instead of falling back to a residual call: - A body containing a `raise`. The sub-walk records into the handler region, and a guard whose resume coordinate lands on the `Reraise` asks `collect_callee_active_boxes` for ref registers the recorded path never wrote. That decline arrives mid-recording on an opcode that is not effect-free, so it has no mid-body carrier and the trace is thrown away. - A FOR_ITER `CalleeReplaySafety::DeferredCall` admission whose deferred call resolves to a builtin: `fbw_abort_nested_unjournaled_residual` spends one abort and then denies the callee. Decline both at the callsite. Measured, median of 3: dynasm sre_pattern_methods 0.90s -> 0.67s, sre_wasm_min 0.55s -> 0.43s, sre_wasm_min1 0.45s -> 0.33s, type_metatype_method_call 0.35s -> 0.28s cranelift the same four return to 0.72s / 0.47s / 0.36s / 0.23s, matching their times with `allow_method_load_attr = false` `loops_compiled`, `bridges_compiled` and `loops_aborted` on all four match the `allow_method_load_attr = false` counters. `inline_subwalk_mutating_residual` keeps the widening's gain at 13.0x pypy3 (dynasm) / 18.3x (cranelift), so its gate moves 200 -> 40, and `inline_subwalk_property_mutates` 80 -> 50. check.py: dynasm 354/354, cranelift 354/354. Assisted-by: Claude * call: translator-safe __call__-slot guard + descriptor tests; operator countOf/indexOf GC fix (#944) * call: guard __call__-slot self-dispatch with call-depth instead of black_box std::hint::black_box is not recognized by the majit translator (front/mir.rs only lowers core::hint::n and core::convert::identity), so in the call-dispatch graph it stayed an unregistered FunctionPath. Replace it at all three slot sites with an increment_call_depth() guard: its drop after the recursive call keeps the self-dispatch off the tail, so a self-referential A.__call__ = A() still recurses natively for stack_check, and it counts the dispatch level. synth: add a descriptor whose __get__ resolves to a separate callable (positional and keyword), covering the get() branch of resolve_user_call_slot. Assisted-by: Claude * operator: move countOf app-level, route indexOf through space.sequence_index Add `baseobjspace::sequence_index` mirroring `descroperation.py:538`, pinning the iterator and needle on the shadow stack so they survive a collection triggered by `__next__`/`__eq__`. Remove the hand-rolled `op_countof`/`op_indexof` loops that held those references in raw locals: `countOf` becomes an `app_operator.py` function (`moduledef.py` `app_names`) and `indexOf` delegates to `sequence_index` (`interp_operator.py:58`). Assisted-by: Claude * Close the PR #937 review queue: GC rooting fixes, os.kill unwrap_spec, and the frame root-walk floor (#946) * _multiprocessing: port W_SemLock's recursion state, release checks and classmethod _rebuild `SemLock` kept no per-lock state, so `_count()` returned 0, `_is_mine()` returned false, `acquire` had no `RECURSIVE_MUTEX` re-entry check and went straight to `sem_wait` (a second acquire on an RLock blocks on a value-0 semaphore), `release` neither checked ownership nor decremented a recursion depth, and `_after_fork` had no state to reset. `count` and `last_tid` (interp_semaphore.py:462,465) now live in the instance dict beside `kind`, `maxvalue`, `handle` and `name`, and `acquire`/`release` follow interp_semaphore.py:506-545. `_is_zero` reported whether the handle was null rather than whether the semaphore was available; it and the new `_get_value` follow interp_semaphore.py:443-455 and :431-441, taking the `sem_trywait` fallback where `sem_getvalue` is broken (darwin, :86-89). `release` gained the maxvalue check of :407-427. `__enter__` returned `self` after an unchecked `sem_wait`; it now returns `acquire()`'s boolean and `__exit__` goes through `release()` (:563-567). `__new__` accepted any `kind`; it now raises `ValueError("unrecognized kind")` (:574-575). `_rebuild` rejected `name=None` and always reopened by name, and allocated on `type_object()` rather than the class it was called through; it is now a classmethod (:606) that falls back to `handle_w` on a nameless semaphore (:550-557, :223-224). Assisted-by: Claude * mapdict: write _mapdict_pop_attribute's map through the rooted receiver Both arms of `_mapdict_pop_attribute` can trigger a collection — the unboxed arm through `_mapdict_write_storage`, the other through the `grow_instance_items_block` shrink — and the trailing `self.map = map` ran through the `&mut self` address captured before them. A moving collection in either arm left that write targeting the stale address, so the live instance kept its old map and its storage index into a block that no longer has the slot. The receiver is now published for the whole function and both the storage write and the `map` write go through the reloaded address, matching `_set_mapdict_increase_storage1` (mapdict.rs:2406) and `_set_mapdict_storage_and_map` (:2427). Assisted-by: Claude * mapdict: treat a null "dict" SPECIAL slot as absent and root the receiver across the three allocating dict paths `_obj_getdict` returned `instance_get_dict_slot`'s answer unfiltered, so a slot holding NULL was reported as the instance's `__dict__`. `mapdict.py:828-830` reads that as "no dict yet" — RPython's `read` answers `None` both for an absent slot and for one holding `None` — and rebuilds the view. Returning the null instead makes `getdict` answer empty, which is exactly the state `descr__setattr__` turns into `"'%T' object attribute '%s' is read-only"` (descroperation.py:58-67): the sole way `_obj_getdict` can produce null, and the reported failure of `self._loop = loop` on a fresh `asyncio.futures.Future` (`_loop` is a class variable, futures.py:53, so the descriptor lookup always hits and the read-only branch is always armed). The sibling `instance_get_weakref_slot` already carries the same filter. Three receivers were also captured before an allocation and used after it: - `_obj_getdict` — `w_dict_new_with` allocates, so both the erased `dstorage` back-pointer baked into the new view and the `instance_set_dict_slot` target were pre-move addresses. `walk_gc_refs` only forwards `dstorage` from the collection after the wrapper is reachable, so the birth allocation is not covered; the back-pointer is now restated from the post-allocation address. - `materialize_dict` — `node_materialize_dict` allocates boxed names, dict stores and carrier transitions before the transplant wrote through the pre-walk `inst`. - `_obj_setdict` — `_obj_getdict` and `mapdict_switch_to_object_strategy` both allocate ahead of `instance_set_dict_slot(self_ref, w_dict)`. Assisted-by: Claude * mapdict: cover the null "dict" SPECIAL slot rebuild with a unit test; rustfmt The test nulls the slot in place — the state a write that never landed leaves behind — and asserts `_obj_getdict` builds a fresh view backed by the instance. It fails without the `!w_dict.is_null()` guard ("a null SPECIAL slot must rebuild the view") and passes with it. Assisted-by: Claude * posix: unwrap os.kill/os.killpg arguments through c_int_w interp_posix.py:1386 and :1394 declare `@unwrap_spec(pid=c_int, signal=c_int)`; both arguments were read with `w_int_get_value`, a raw payload read with no type check. `is_int` compares `ob_type` for exact identity, so no non-`int` argument took a checked path: `os.kill(pid, None)`, `os.kill(pid, [30])` and `os.kill("x", 30)` passed whatever bit pattern sat at that offset to `kill(2)` instead of raising TypeError, and an out-of-range value was truncated rather than reported. Assisted-by: Claude * baseobjspace: include the operand type in space_int's unwrap error baseobjspace.py:323 raises through `_typed_unwrap_error(space, "integer")`, whose body at :316-318 is `"expected %s, got %T object"`. The message here was `"expected integer"`, dropping the operand type. Assisted-by: Claude * mapdict: resolve the unboxed storage slot without allocating `boxed_storage_indices` built a `vec![false; len]` and a result `Vec` on every call. `instance_walk_boxed_storage` calls it from inside `object_object_custom_trace`, so the collector allocated twice per traced instance while marking, and `_mapdict_write_storage` called it through `storage_index_is_boxed` on every attribute store to answer a single-slot question. `_compute_storageindex_listindex` (mapdict.py:549-562) breaks at the first `UnboxedPlainAttribute` ancestor and reuses its `storageindex`, setting `firstunwrapped` only when the walk finds none, so a chain owns at most one unboxed slot. `unboxed_storage_index` returns that slot without allocating; the index list is now derived from it and remains only for `_set_mapdict_storage_and_map`, which needs the list itself. Assisted-by: Claude * posix: build the sysconf_names value before reading the rooted dict Call arguments evaluate left to right, so `w_dict_setitem_str` read the dict out of the shadow stack before `w_int_new` allocated the value: a collection in that window moved the dict and the store wrote through the pre-move address. The slot index is now taken once after `pin_root` instead of being respelled as `shadow_stack_len() - 1` inside the loop. Assisted-by: Claude * eval: floor the frame root walk at the locals/cells prefix Both walks over `locals_cells_stack_w` took `valuestackdepth.min(len)` as the walk length. `valuestackdepth` is an absolute index that starts at `stack_base()` (pyframe.rs:2136) and `pop` refuses to go below it, so for a running frame that already covers the locals/cells prefix — `PyFrame::descr_clear` is the exception: it rebinds every cell slot to a fresh `w_cell_new` and then sets `valuestackdepth = 0`. Both walks then scanned zero slots for a cleared frame, so cells reachable only through the array were neither kept nor forwarded while the array still pointed at them. The length now floors at `stack_base()`, which only ever widens the walk. The SAFETY comment claiming the walk covers the full fixed-length array described neither the previous behaviour nor this one. Assisted-by: Claude * _multiprocessing, thread: reload the receiver past getdict_native; drop a repeated fork-child reinit `semlock_instance` pinned `obj` and then passed the pre-pin copy to `getdict_native`, which materialises the instance dict and can therefore collect and move it; the receiver is now read back from its slot, as the stores in the same function already do. `after_fork_child` called `setobject::set_locks_after_fork_child()` twice. Assisted-by: Claude * gc_roots, executioncontext, posix, _socket: root operands held across collecting calls `pin_roots` did not carry `pin_root`'s debug-only `assert_shadow_stack_not_walking`, so a caller moving from one to the other lost the guard against publishing roots mid-walk. `_call_finalizer` read `__del__` out of the type dict and passed the pre-call pointer to `report_error` after running arbitrary Python. `posix_spawn`'s env loop kept the mapping and the key vector in plain locals across `getitem` and two `fsencode_bytes_w` calls. `sendmsg` pinned each data item and then iterated the unrooted vector, handing pre-pin copies to `simple_buffer_bytes`, which looks up `__buffer__` and builds a memoryview. Assisted-by: Claude * mapdict: scan the whole chain for unboxed slots instead of the first hit The allocation-free rewrite returned at the first `firstunwrapped` node, resting on `_compute_storageindex_listindex` (mapdict.py:549-562) giving a chain at most one. The collector is the wrong place to depend on that: under-reporting one unboxed slot hands the marker a raw `Vec<i64>` pointer. `storage_index_is_unboxed` now scans the chain for a matching storageindex, which restores the exact set the pre-rewrite `boxed_storage_indices` computed while keeping both allocations out of `object_object_custom_trace`. Assisted-by: Claude * types: complete the UnionType slot surface; functools function exports; check.py startup median (#936) * types: expose UnionType rich comparison slots * types: complete UnionType identity and iteration slots * functools: export reduce and cmp_to_key as functions * check.py: take the empty-program startup median over five samples The measured startup is subtracted from every timed run, so it is the divisor for short benches. One high sample collapses that denominator: a macOS runner measured pypy startup at 0.031s where the same job had measured 0.013s, and three benches failed their gates in that run while their pyre exec times had gone down. Assisted-by: Claude * jit,interp: lower DELETE_NAME/GLOBAL, drop invalidated celldict watchers (#935) * interp: decline only a metatype data descriptor in the classmethod fold `classmethod_on_type_fast_path` declined every name the metatype defines. `typeobject.py:813-823 descr_getattribute` orders the two lookups the other way: the metatype entry preempts the class's own MRO only when it is a data descriptor, and otherwise `self.lookup(name)` is what gets selected. Since the fold already requires the class MRO lookup to answer a classmethod, the class-MRO-miss arm is unreachable, so a non-data metatype attribute is provably never the selected value. Swap the check to `type_lookup_is_data_descr`, which is the existing `space.is_data_descr(space.lookup(...))` analogue in the same module. The check was dead on main until the metatype pin landed: it read `&TYPE_TYPE` as an object pointer, whose first field is an `AtomicI64`, so `lookup_in_type`'s `is_type` gate rejected it for every name. Pinning the real metatype made it live, and with it the divergence. Reach is one name: the walker declines every dunder up front, and `mro` is the only non-dunder `type` defines. `Cls.mro()` with `mro` a classmethod now folds, matching pypy3 and python3. Assisted-by: Claude * comments: name the real blockers behind three declined ports No behaviour change. `w_pytraceback_get_lineno` justified the eager `offset2lineno` stamp with "PyFrame is not a GC-traced W_Root, the frame may already be freed". That stopped being true: `pytraceback_object_custom_trace` forwards the `frame` edge, and `w_code` — which is `frame.pycode` — is forwarded unconditionally, so a lazy resolve off `w_code` and `lasti` would be safe. What actually blocks the port is the JIT fold, which reads the slot and declines on the sentinel, so every fresh node would decline on its first read. Record that, and the two `tb_lineno` answers the eager stamp diverges on: `tb.tb_lasti = N` before the first read, and a sentinel written back through `TracebackType` or the setter. The `TbLineno` fold named one sentinel producer; add the other two. `FBW_FORITER_DEFERRED_DENY` lacked the per-thread paragraph its sibling `FBW_HAZARDOUS_INLINE_DENY` carries, so nothing said that it is only a memo and that the live abort is what keeps a deferred body honest. Assisted-by: Claude * builtins: read the pinned iterator back from its slot `collect_iterator`, `builtin_any` and `builtin_all` pinned the iterator and then ran the dict-view type check and the backing-dict read off the raw local. `pin_root` normalizes through `try_gc_current_object_address` and writes the result into the shadow-stack slot only, so the slot — not the local — is the authoritative value once it returns. The loop below already reloaded per call; these three lines were the outliers, and the sibling `min_max_sequence` in the same file already reads the slot for exactly this. No live path reaches the difference today: every builtin iterator is `allocate_stable` and a dict-view iterator is off-GC `malloc_typed`, so neither moves, and nothing collects between `iter(...)` and the pin at any of the three sites. Assisted-by: Claude * interp: read the frame builtin through get_builtin in LOAD_GLOBAL `_load_global` (pyopcode.py) reaches the builtins fallback through `self.get_builtin()`. `load_global_value` and the `jit_load_name_from_namespace` extern read the `w_builtin` slot directly instead. `pyframe.py` assigns `self.builtin` only under `honor__builtins__`, which `baseobjspace::HONOR_BUILTINS` leaves off, so `get_builtin` answering `space.builtin` for an unset slot is the only route that resolves a builtin at all for a frame nobody wrote the slot on -- among them the inlined-callee frames the tracer emits, which follow the same constructor and leave it at the allocation's zero-fill. Before this, forcing one of those frames made every builtin name raise NameError. Both frame emitters' trailing comments now list `w_builtin` alongside the other class-level defaults they deliberately do not store. Assisted-by: Claude * dynasm: take getenv and String building off the entry path `execute_token` read `PYRE_DYNASM_EXEC_DIAG` once and `PYRE_GC_FREELIST_DIAG` twice per compiled-trace entry, each an uncached `std::env::var_os` that takes the global env lock and walks the env table. It also built the `format!("before/after trace {id}")` site strings unconditionally, so a diagnostic that is off still cost two heap allocations per entry. `dynasm_debug_validate_oldgen_freeblocks` paid the same getenv plus one `format!` per residual call. Cache both flags in `LazyLock` bools next to `majit_log_enabled`, and take `std::fmt::Arguments` in `debug_validate_oldgen_freeblocks` so the site name is only materialized once the flag is on. Sampling `synth/gc_bug_bridge_flavor_traceback_names` (5s, dynasm): samples under `run_compiled_detailed_with_values_at_dispatch_key` 244 -> 113, with the `__findenv_locked` and `debug_validate_oldgen_freeblocks` frames gone from the entry path. Assisted-by: Claude * jit: lower DELETE_NAME and DELETE_GLOBAL to residual calls Both opcodes were emitted as a static `abort_permanent`, which discards the enclosing loop trace permanently. A module-level `except X as e:` compiles its implicit cleanup to `e = None; del e`, and at module scope `del e` is DELETE_NAME, so any module loop containing an except-as clause could never be traced. Function scope uses DELETE_FAST, which was already lowered. pyopcode.py DELETE_NAME is `space.delitem(w_locals, w_varname)` and DELETE_GLOBAL the same against `w_globals`; neither is an unsupported opcode upstream. Adds the `delete_name` / `delete_global` HLOps in the frame-receiver call shape of `store_name`, minus the value operand: two Ref operands, void result, zero stack effect. They lower to `residual_call_r_v` with `PyreHelperKind::None` (there is no fold for delete) and are registered in `graph_op_can_raise` — DELETE_NAME raises NameError when the binding is absent. `bh_delete_name_fn` / `bh_delete_global_fn` delegate to the existing `OpcodeStepExecutor` methods on PyFrame and publish into both exception cells on error, matching `bh_store_name_fn`. SetupAnnotations stays on `abort_permanent`. loops_aborted -> 0 on the three benches whose hot module loops reached the op: exception_reraise_tb_depth_hot (30 -> 0, bridges_compiled 0 -> 9), exception_reraise_tb_depth_jitstress (30 -> 0), and exception_reentry_guard_finally_residual (5 -> 0). exception_reraise_tb_depth_hot: 692ms -> 267ms CPU, 6.5x -> 2.5x of pypy. check.py dynasm 354/354, cranelift 354/354. Assisted-by: Claude * celldict: drop version watchers once they are invalidated `sweep_version_watchers` used `Vec::retain` and returned true for every watcher it could still upgrade, so a live loop flag stayed in the list after being flipped. `QuasiImmut.invalidate` (quasiimmut.py) instead takes the list and empties it — `wrefs = self.looptokens_wrefs; self.looptokens_wrefs = []` — so a loop is invalidated exactly once and then drops out. Retaining them is redundant and unbounded: the flag is already true, so re-storing it changes nothing, and the list grows by one entry per compiled loop that folded a module-global. A module-level `except X as e:` runs `del e` every iteration and `delitem` calls `mutated()`, so each iteration walked every loop ever compiled in that module. Dropping is safe because registration is per compiled artifact: `last_compiled_artifact_invalidation_flag` registers a fresh flag on every compile, and a flipped flag is permanently invalid. On synth/exception_metadata_hot at N=250000 this was the largest single non-interpreter profile leaf (629 samples, ahead of eval_loop_jit) and is absent from the profile afterwards; 13244ms -> 12019ms CPU, 8.31x -> 7.54x of pypy. It does not change loops_compiled (309 either way) — the remaining gap is the version churn itself, not the walk. Also corrects the `_setitem_str_cell_known` doc comment, which claimed `write_cell` was stubbed to identity; it ports the MutableCell family and absorbs a same-cell rewrite without touching the version. cargo test pyre-object + pyre-interpreter green, check.py cranelift 354/354, dynasm 353/354 with synth/jit_reg_const_pool_256_slot_decline a gate-boundary flake (passes 3/3 solo; both binaries measure 984ms vs 988ms on it). Assisted-by: Claude * codewriter: reformat two delete-opcode let bindings The DeleteName / DeleteGlobal arms sit at a shallower indentation after the rebase onto main, so rustfmt joins both `Constant::string` bindings onto one line. Whitespace only; `cargo fmt --all --check` is clean again. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 1, 2026
…nline moved `#942` passes `allow_method_load_attr = true` at `try_walker_inline_user_call`, so an unbound method-form callee whose body reads `self.attr` inlines into the caller's loop instead of running as a residual call. Three fixtures carry that shape (`c.add(i & 7)`, `self.process_word()`) and lose the callee's separately compiled loop: class_attrs_methods loops_compiled 2 -> 1 (dynasm, cranelift) foriter_inplace_immutable loops_compiled 2 -> 1 (cranelift) inline_subwalk_mutating_residual loops_compiled 4 -> 3 (cranelift) Every counter that moves with them falls: `class_attrs_methods` cranelift `guard_failures` 202 -> 1 and `bridges_compiled` 1 -> 0; `inline_subwalk_mutating_residual` cranelift `guard_failures` 12316 -> 603 and `loops_aborted` 1 -> 0, wasm `loops_aborted` 4 -> 2. The measured ratio for that fixture is 13.1x dynasm / 17.2x cranelift, matching the 13.0x / 18.3x `#942` records. `method_reassign_after_warmup` arrived with `#940` and had no baseline, which the missing-baseline gate reported on all three backends. check.py: dynasm 359/359, cranelift 359/359, wasm 355/355. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 1, 2026
….py: arm the jit-stats floor across the synthetic suite (#947) * jit: count the deduped set in intern_liveness' liveness record header `intern_liveness` keyed its dedup table on the sorted, deduped form of its three `&[u8]` arguments but took the record's count bytes and encoded bitsets from the raw slices. `encode_liveness` dedups, so a repeated register index made the count exceed the bitset's cardinality. `LivenessIterator` is driven by that count, so decoding such a record read past its own bitset into the following record's bytes and swallowed it. Read the counts and the bytes off the canonical form already computed for the key, matching `majit-translate`'s producer, which counts the `VecSet` it encodes from (codewriter/assembler.rs:1060). Assisted-by: Claude * majit: read the diagnostic env gates once, and build their messages behind them `execute_token` brackets every entry into compiled code with `debug_validate_oldgen_freeblocks`, which read `PYRE_GC_FREELIST_DIAG` with `std::env::var_os` on each call, and it read `PYRE_DYNASM_EXEC_DIAG` the same way — three `getenv`s per trace entry, each taking the environment lock and scanning the array. The two bracket calls also built their `site` string with `format!` before the gate, so a disabled diagnostic still allocated and formatted one per trace entry, and `dynasm_debug_validate_oldgen_freeblocks` one per residual call site. Cache both gates in a `LazyLock<bool>` as `majit_log_enabled` does, take `site` as `Arguments` so the string is materialised only past the gate, and do the same for `MAJIT_LOG` in `do_collect_nursery` / `finish_incremental_cycle` and `PYRE_GC_FREELIST_DIAG` in `dynasm_nursery_slowpath`. On a 20x-scaled `recursion_memo_branch` the startup-subtracted user-CPU exec time goes 0.7609s to 0.6566s (0.863x, min of 7 interleaved rounds, same host), output unchanged; `__findenv_locked` and `std::env::__var_os` fall from 43 of 388 working samples to 0 of 581. Also correct the comment above the `should_disable` application: it cited `trace_opcode.rs:3044-3049` as a second site applying `disable_noninlinable_function`, where no such symbol exists. Assisted-by: Claude * jit: emit the CALL's null_or_self slot in the outer-call stack overrides `collect_call_stack_overrides` had no source for the `null_or_self` operand a `PUSH_NULL` leaves under a `CALL`: the walk holds no box for that slot, no live color names it, and the virtualizable shadow reads back the same NULL an unmirrored slot does. The slot stayed absent, so the outer-call flush that follows a `LoopBearingCalleeInlineUnsupported` abort declined on "stack override missing for a live slot" and fell back to the legacy rollback+replay, where `fbw_foriter_inflight_take` refuses in-flight FOR_ITER delivery and the whole iteration is dropped. Name the slot from the CALL's own operand layout — `[callable, null_or_self, arg0 .. arg_{argc-1}]` ends at the caller's stack top, so the sentinel sits `argc + 1` below it — and push an explicit null override. A coordinate that does not invert to a plain `CALL` keeps the existing decline. The added `bench/synth/foriter_call_resume_drops_iteration.py` printed `491088 8999` before this change and `491130 9000` after, matching CPython, PyPy and `PYRE_NO_JIT=1`. Assisted-by: Claude * check.py: gate guard_failures on a bounded rise against the jit-stats baseline _jit_stats_regression_floor now iterates JITSTATS_RISE_BOUNDED_FIELDS in addition to JITSTATS_BADNESS_FIELDS. A badness counter still fails on any rise above its baseline; a rise-bounded counter fails only past `base + max(base // 4, 2)`. A field absent from the baseline keeps reading as 0 for the badness counters, whose healthy value is 0, but leaves a rise-bounded counter unpinned: the wasm [jit-stats] line reports no guard_failures, so every *.wasm.jitstats baseline omits it. guard_failures moves out of the informational snapshot surface into that gate. Assisted-by: Claude * check.py: track the synthetic .jitstats baselines pyre/.gitignore ignored /bench/synth/*.jitstats with three `!` negations, so the regression floor read a committed baseline for 3 of 340 synthetic fixtures and skipped the other 337. Recorded with `pyre/check.py --snapshot` (dynasm 355/355, cranelift 355/355, wasm 351/351): 340 dynasm, 340 cranelift, 339 wasm — getframe_caller_locals_nested_compiled_callee carries skip-backends=wasm. Of the 1019 synthetic baselines, 872 record all-zero badness counters, 147 record a nonzero loops_aborted, and none record a nonzero descr_set_* or internal_compile_panics. Assisted-by: Claude * check.py: record the synthetic baselines the rebase left missing or stale defaults_reassigned_midloop and del_cellvar_walk_commit arrived from origin/main with no .jitstats baseline, so the regression floor skipped them. type_metatype_method_call moves loops_compiled 4 -> 2, bridges_compiled 47 -> 0 and guard_failures 9480 -> 1 on the native backends, the effect #925 records in its own message. comprehension_object_append_hot.dynasm moves guard_failures 3611 -> 3612. Assisted-by: Claude * check.py: gate loops_compiled on a fall, and fail a benchmark with no baseline JITSTATS_FALL_FIELDS gates a counter whose defect direction is down. loops_compiled is its first member: a pre-trace decline aborts nothing, so loops_aborted holds, and it removes the compiled guards that used to fail, so guard_failures falls — both existing gates read the loss as an improvement while the hot loop runs interpreted. The fall is gated exactly rather than through a band. Diffing the dynasm and cranelift baselines across the 341 synthetic fixtures, the two code generators agree on loops_compiled for 340 of them (99.7%), against 97.4% for guard_failures. A benchmark that emits a [jit-stats] line but has no committed baseline now fails instead of passing. Every gate above compares against nothing when the file is absent, so "never recorded" and "recorded and clean" print the same PASS — which is how the floor came to cover 3 of 340 synthetic fixtures. Assisted-by: Claude * check.py: re-record the jit-stats baselines the widened method-form inline moved `#942` passes `allow_method_load_attr = true` at `try_walker_inline_user_call`, so an unbound method-form callee whose body reads `self.attr` inlines into the caller's loop instead of running as a residual call. Three fixtures carry that shape (`c.add(i & 7)`, `self.process_word()`) and lose the callee's separately compiled loop: class_attrs_methods loops_compiled 2 -> 1 (dynasm, cranelift) foriter_inplace_immutable loops_compiled 2 -> 1 (cranelift) inline_subwalk_mutating_residual loops_compiled 4 -> 3 (cranelift) Every counter that moves with them falls: `class_attrs_methods` cranelift `guard_failures` 202 -> 1 and `bridges_compiled` 1 -> 0; `inline_subwalk_mutating_residual` cranelift `guard_failures` 12316 -> 603 and `loops_aborted` 1 -> 0, wasm `loops_aborted` 4 -> 2. The measured ratio for that fixture is 13.1x dynasm / 17.2x cranelift, matching the 13.0x / 18.3x `#942` records. `method_reassign_after_warmup` arrived with `#940` and had no baseline, which the missing-baseline gate reported on all three backends. check.py: dynasm 359/359, cranelift 359/359, wasm 355/355. Assisted-by: Claude * codex-review: withhold generated .jitstats baselines from the changed-file list The workflow appends `git diff --name-only upstream/main` to the review prompt as the authoritative definition of "this patch". On a PR that re-records check.py's jit-stats baselines in bulk that list reached 1027 entries and Codex returned no report at all (exit 1); CodeRabbit skipped the same PR for being 927 files over its own 100-file limit. `.jitstats` files are generated golden data with no RPython/PyPy counterpart, so no parity finding can cite one. Exclude them from the list, report the withheld count in the prompt header and in the empty-diff message, and put the same exclusion in the prompt's own scope-discipline instruction, which is what the local `/codex-review` path re-derives from. On this branch the list goes from 1132 to 112 files. Assisted-by: Claude * check.py: re-record the exception-reraise baselines main's trace shape moved `origin/main` alone reproduces the counters exactly, so this is not our branch: exception_reraise_tb_depth_hot 40 loops / 0 guards / 30 aborts / 0 bridges -> 4 loops / 1803 guards / 0 aborts / 9 bridges exception_reraise_tb_depth_jitstress 900 / 0 / 30 / 0 -> 805 / 1798 / 0 / 1 Both gated directions fired at once (`loops_compiled` down, `guard_failures` up) and neither is a defect on the native backends. Interleaved user+sys CPU time, min of 5, `e1e97e61d32` vs `origin/main`, same host: `_hot` 0.750s -> 0.370s (2.03x faster), `_jitstress` 0.490s -> 0.490s. The 40 specialized loops with 30 aborts were the worse shape. wasm moves the other way on the same two fixtures: `loops_aborted` 30 -> 44 and 30 -> 1198. Recording those pins the higher value, so the rise is stated here rather than absorbed silently. check.py, per fixture: dynasm 1/1, cranelift 1/1, wasm 1/1. Assisted-by: Claude
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.
Four commits. The performance table in the original description of this PR was wrong; it has been replaced — see "Correction" at the bottom.
majit: keep the nonstandard vable element heapcache in syncWrong-code fix, and the reason this branch exists. A force-materialized inline-callee frame's
locals_cells_stack_wtakes the nonstandard-virtualizable path, whose element heapcache (a) keyed reads and writes off differentGetfieldGcRarray-baseOpRefs and (b) recorded aSETARRAYITEM_GCwithout updating the cache. Reads therefore kept returning the value seeded at frame construction.Repro —
i += 1writesW_Int(2189)to slot 1, and the immediately followingLOAD_FAST ipushes the staleW_Int(2188):origin/mainprintsBAD len=3001; this branch printsOK.trace_ctx.rsgainsnonstandard_vable_array_base(forwards the cached field box on a hit, publishes withgetfield_now_known) andTraceCtx::execute_setarrayitem_gc(records, then callsheapcache.setarrayitem).gen_store_back_in_vablekeeps the rawvable_setarrayitem_descr.This commit is worth 0.98x–1.00x — it is a correctness fix with no measurable performance effect.
jit: inline a method-form callee whose body reads self.attrtry_walker_inline_user_callpassedallow_method_load_attr = false, somethod_form_callee_body_supporteddeclined everyb.at(i)whose callee body contains aLoadAttrresidual — the ordinary accessor shapedef at(self, i): return self.v + i. The same callee already inlined when stored as a bound method (m = b.at; m(i)).jit: decline the widened method-form inline for raise-bearing and FOR_ITER-deferred calleesThe widening on its own discarded the enclosing loop for two body shapes:
raise. The sub-walk records into the handler region, and a guard whose resume coordinate lands on theReraiseaskscollect_callee_active_boxesfor ref registers the recorded path never wrote. The decline arrives mid-recording on an opcode that is not effect-free, so it has no mid-body carrier.CalleeReplaySafety::DeferredCalladmission whose deferred call resolves to a builtin:fbw_abort_nested_unjournaled_residualspends one abort and then denies the callee.jit: key the raise decline on widened_method_form, not allow_method_load_attrThe commit above keyed the raise decline on
allow_method_load_attr. Five entries pass that flag, and four of them — thetype.__call____init__fold, the exception__str__/__repr__override, and the property getter and setter — passed it before the widening, so the decline also withdrew inlines that already happened. A method-form body with no attribute read hasmethod_form_callee_body_supported == true, hencewidened_method_form == false, and is now admitted again.None of the 357 synthetic benches covered this shape.
Measurements
Against a binary built from
origin/main, min of 7 interleaved runs, user+sys CPU with empty-program startup subtracted. The two binaries were checked apart by the wrong-code repro above (main→BAD len=3001, branch →OK).synth/inline_subwalk_mutating_residualsynth/sre_pattern_methodssynth/sre_wasm_minsynth/sre_wasm_min1synth/type_metatype_method_callproperty_mutates/radd_consumed/user_iterator/getframe_multiframeSplitting the win by commit on
inline_subwalk_mutating_residual(min of 9 interleaved runs): the heapcache fix alone is 0.98x (dynasm) / 1.00x (cranelift); the widening plus its declines is 3.89x / 3.82x. The entire measured gain comes from the widening, and the entire correctness gain from the fix.inline_subwalk_mutating_residual's gate moves 200 → 40 andinline_subwalk_property_mutates's 80 → 50.Verification
check.py --backend dynasm357/357check.py --backend cranelift357/357cargo test --all --no-default-features7324 passed, 0 failed (run before the last commit)Correction
The first version of this description claimed
sre_pattern_methods0.90s → 0.67s,sre_wasm_min0.55s → 0.43s and similar gains on four benches. Those numbers were real but did not mean what the description said: the "before" arm carried the heapcache fix and differed from the "after" arm only by the widening, so the table measured the widening's own regression being repaired, not an improvement overmain. Againstmainthose four benches are flat. A second error compounded it —check.pyrunscargo builditself (pyre/check.py:993), so an A/B done by swapping prebuilt binaries intotarget/releaseruns the same build on both arms.Note on #935
#935 carries
0d8413d3c4and3f560c4ca0, which are patch-id-identical to this PR's first two commits (b2a00ef0…andc36e7c6e…), plus five unrelated commits. It does not carry the two decline commits, so it is expected to reproduce both the loop-discard on the sre benches and the 46x/20x raise-body regression. This repository squash-merges, so after this PR lands the squashed commit's patch-id will match neither original and rebase dedup will not drop them — they have to be dropped from #935 explicitly.— authored by Claude