Repository navigation
jit: nsvable multi-frame resume + callee-locals shadow + exc-edge bridge + portal-runner descr (gated); gc: register deque iterators; vstack NONE-hole - #643
Conversation
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (27)
✨ Finishing Touches🧪 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 |
![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: 2d80918ee7
ℹ️ 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".
| // `NONE` means the result was produced in an unboxed bank; | ||
| // leave an intentional hole so the capture overlay around | ||
| // stack_sync omits the slot and resume rematerializes it. | ||
| ctx.vstack_boxes[new_depth - 1] = ctx.vstack_last_ref; |
There was a problem hiding this comment.
When a ResultToTos opcode produces no Ref (vstack_last_ref == OpRef::NONE) during a non-inline root/bridge walk, this assignment creates a hole but the generic hole-fill just below immediately calls reseed_vstack_from_shadow because hole is true and skip_shadow_fill is false. On bridge walks the virtualizable shadow can still contain the previous Ref for that stack slot (pop clearing is deliberately not mirrored there), so the NONE can be replaced before stack_sync and the guard snapshot republishes the stale box this change is trying to suppress; please skip shadow refill for this TOS or explicitly keep the vable slot omitted/null in this case.
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 90d7921). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patch
2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
youknowone
changed the title
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/18818e1f0d3b56cca19bc9fb1cb230124208132a/pyre-jit-trace/src/jitcode_dispatch.rs#L13567
In the new multi-frame success path, this only overwrites the resume data on the most recent guard recorded by the vable helper. When a helper records more than one guard before this call (for example an array-item vable access can run _nonstandard_virtualizable and then promote a non-constant index), any earlier GuardValue keeps the placeholder snapshot from record_guard_with_snapshot (jitcode 0 with empty boxes). Because this branch now returns Ok instead of the old sentinel abort, such a trace can be optimized/compiled with a guard that will mis-resume if it survives and fails; the dispatch layer needs to attach the multi-frame snapshot to each newly recorded helper guard or capture at each promote site.
ℹ️ 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".
b566eda to
30d842c
Compare
youknowone
changed the title
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/30d842c1c99156a44fed6b49645c24213ff6a563/pyre-jit-trace/src/jitcode_dispatch/mod.rs#L2106-L2107
When PYRE_EXC_EDGE_BRIDGE=1 and an except handler contains a branch with both a return arm and a loop-rejoin arm, this reachability test accepts the handler as soon as it sees the rejoin path. That bypasses the ExcEdgeCrossFrameReturnUnsupported decline even though the concrete walk can still take the *_return arm and record the cross-frame Finish path that the comment above says pyre cannot reconstruct. Please decline handlers with any reachable return path, or make the decision follow the concrete branch path before routing.
ℹ️ 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".
youknowone
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a962737f13
ℹ️ 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".
| if op.key.starts_with("jit_merge_point") { | ||
| return true; |
There was a problem hiding this comment.
When PYRE_EXC_EDGE_BRIDGE=1 and an except handler has mixed control flow (for example one branch returns from the function while another falls back to the loop), this predicate returns true as soon as it finds the rejoin path. That lets dispatch_via_miframe route the bridge instead of declining with ExcEdgeCrossFrameReturnUnsupported; if the current exception takes the return branch, the walk still records the unsupported Finish/DoneWithThisFrame path that the surrounding comment says cannot be reconstructed. The check needs to prove that no reachable handler path returns/escapes, not just that some path reaches a jit_merge_point.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/1498660dd61ac760033cd6e1c1aed85d9cd9cee7/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs#L362
When PYRE_EXC_EDGE_BRIDGE is enabled and this synthetic GUARD_EXCEPTION later fails (for example the same residual call raises a different exception class), walker_capture_snapshot_for_last_guard treats every GuardException as after_residual_call and advances the supplied pc to semantic_fallthrough_pc before publishing last_instr/stack depth. Here position is already the bridge resume/live coordinate rather than the original residual-call pc, so the guard's resume data skips the first handler/fallthrough opcode and deopts to the wrong frame state; capture this bridge-entry guard without the after-residual advance, or pass the original call coordinate instead.
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a6fbb885a
ℹ️ 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".
| let (instruction, op_arg) = arg_state.get(unit); | ||
| if let Instruction::LoadGlobal { namei } = instruction { | ||
| let name_idx = (namei.get(op_arg) as usize) >> 1; | ||
| if code.names.get(name_idx).map(|n| n.as_ref()) == Some(own_name) { |
There was a problem hiding this comment.
When an inlined callee recurses through a global alias or a mutual call, this check returns false because the loaded name is not code.obj_name. The new fbw_abort_nested_unjournaled_residual relies on code_is_self_recursive to keep recursive nested residuals on the old decline path; if the CALL_ASSEMBLER fold also declines (for example non-int args or an unsupported frame shape), the recursive residual is now allowed even though the comment in fbw_inline_callee_hazardous says that moving-nursery callee frames cannot survive that trampoline. This needs to compare the prospective callee/code identity or keep the blanket decline for unresolved recursive CallFn cases, not just match LOAD_GLOBAL <own name>.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6334b9ce1c
ℹ️ 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".
| let exc_class_const = wc.trace_ctx.const_int(exc_edge_class); | ||
| wc.trace_ctx | ||
| .record_guard(OpCode::GuardException, &[exc_class_const], 0); | ||
| walker_capture_snapshot_for_last_guard(&mut wc, position)?; |
There was a problem hiding this comment.
With PYRE_EXC_EDGE_BRIDGE=1, this synthetic bridge-entry GuardException is recorded after the original failing call has already resumed at position (the no-exception fallthrough). Calling walker_capture_snapshot_for_last_guard here makes the helper classify it as after_residual_call because the last guard opcode is GuardException, so it advances from position to semantic_fallthrough_pc(position) when building resume data. If the compiled routed bridge later sees a different exception class and this guard fails, blackhole resume skips the real fallthrough/handler dispatch by one opcode; capture this guard without the after-residual adjustment, or pass the original raising call pc instead.
Useful? React with 👍 / 👎.
ResultToTos always replaces TOS with the opcode result. When the result lives in an unboxed bank, vstack_last_ref is NONE; skipping the write can leave a stale consumed Ref box in the mirrored TOS slot for guard capture. Write the slot whenever the new depth is non-zero so unboxed results become intentional NONE holes that the stack_sync overlay omits and resume rematerializes. Assisted-by: Claude
Assisted-by: Claude
…multi-frame resume chain The `_nonstandard_virtualizable` PTR_EQ promote GuardValue emitted inside an inline sub-walk was captured by `walker_capture_inline_nonstandard_vable_guard` with a hardcoded sentinel resume word (`NO_JITCODE_PC`), which `resolve_resume_pc_with_jitcode_pc` rejects, so every such emit aborted with `GuardResumeCoordinateUnavailable`. Route the guard through `walker_capture_multi_frame_inline_snapshot` when the paused-caller chain covers the full inline depth, publishing the callee's own coordinate plus each paused caller (the resume RPython builds at pyjitpl.py:1916/2582/2610), stamping the last guard op since `emit_force_virtualizable` records ops after the promote. Add the op-stamping multi-frame snapshot method to majit history and a `stamp_last_guard_op` parameter to the publisher. Keep the single-frame sentinel as the fallback for a partial chain. Gated `PYRE_FBW_NSVABLE_MULTIFRAME` (default-on). Assisted-by: Claude
… for branching (try_multiframe) inlines The forward-inline sub-walk seeds `CalleeLocalsShadow.set_concrete` (the per-inline-level MIFrame-register analog whose concretes survive a may-force heapcache clear) from the callee's param boxes only on the `!try_multiframe` path. A branching callee (try_multiframe) inlined with a materialized virtual frame relied instead on the heapcache forward that `emit_new_pyframe_inline_with_params` registers for its locals array. That forward is cleared by any in-callee may-force op (`reset_keep_likely_virtuals` empties heap_cache/heap_array_cache), so a LOAD_FAST re-read of a param after a may-force missed the heapcache and produced a non-concrete value, surfacing `GotoIfNotValueNotConcrete` at the next branch inside the callee. Seed `set_concrete(callee_portal_frame_reg, slot, concrete)` for the try_multiframe params too, so the post-may-force `getarrayitem_vable` read fallback recovers the param concrete. The register-to-register fold (`fold_frame_reg` + `set_opref`) stays gated to `!try_multiframe` (it bypasses the real virtual frame). STORE_FAST already re-seeds the shadow via the `setarrayitem_vable` handler. Under PYRE_FBW_NESTED_RESID_ABORT=0 (decline lifted) the walker no longer aborts inline_subwalk_property_mutates / nested_callee_chain_mutation_abort with GotoIfNotValueNotConcrete; both trace to the correct result ((7176000, 60000) / (4848000, 112000)). The census's remaining classes (ConstEncodingOverflow / NestedBreakBridgeResume / NoRecipes) are unrelated later walls. Default path unchanged: cranelift 225/225, wasm 224/224, and both witnesses bit-exact on dynasm. Assisted-by: Claude
…arded op-table The `getarrayitem_vable` fallback recovered a callee local's concrete from the per-inline-level `CalleeLocalsShadow.concrete` map, which stores a raw `majit_ir::Value` copy. The trace-ref walker does not visit `callee_shadow` (it lives in `WalkContext`, not `TraceCtx`), so a `Value::Ref` copy held there is not forwarded when a minor collection moves a nursery object. A Ref param allocated during the walk and seeded into the shadow could therefore dangle if an in-callee may-force residual triggered a minor GC before the post-call LOAD_FAST re-read — the fallback would then recover and stamp a stale pointer. Seeding the shadow for the try_multiframe path (prior commit) widened this to the forward inline callee params. The read fallback now prefers re-resolving the slot's SSA OpRef through `concrete_of_opref` (the op-table value channel, which IS GC-forwarded) over the raw `concrete` copy, falling back to the copy only when the OpRef carries no live concrete (ints/floats are value copies of immutable payloads, always safe). To keep the two maps consistent so the preferred OpRef is never stale: - Seed `set_opref` for the try_multiframe param path too (previously `!try_multiframe`-only); the fold consumer stays gated by `fold_frame_reg` (kept `!try_multiframe`), so this is inert for folding. - The `setarrayitem_vable` non-fold re-seed now updates `set_opref` alongside `set_concrete` on every store, matching the strict-fold store path. The OpRef is consulted only when a frame-matched `concrete` entry exists, so the per-frame isolation the `frame_reg` filter gives `concrete` also bounds the re-resolution. No behavior change on the default path or the witnesses (their int params are immortal small ints); this closes the Ref-staleness window for a general nursery Ref param. Assisted-by: Claude
…PYRE_EXC_EDGE_BRIDGE, default-off) Add `exc_edge_bridge_enabled()` (PYRE_EXC_EDGE_BRIDGE, default-off). When set, an exception-guard bridge whose failing guard is caught in-frame routes the walk to the in-frame `except` handler instead of declining to the blackhole. - jitcode_dispatch.rs: `find_catch_before_resume_live` selects this opcode's catch (backward op scan bounded by the call's post-call `-live-`); `exc_handler_rejoins_loop` proves the handler reaches a `jit_merge_point` (route) vs terminates at a `*_return` (decline). dispatch_via_miframe emits the SAVE_EXC_CLASS / SAVE_EXCEPTION / RESTORE_EXCEPTION + GUARD_EXCEPTION resumption at bridge-entry, stamps the restored-exception concrete onto the SAVE_EXCEPTION box, enters the handler-entry vstack, and walks from the catch target. New `DispatchError::ExcEdgeCrossFrameReturnUnsupported` aborts a cross-frame-return handler before any recording. - call_jit.rs: `route_exc_edge` (caught_in_frame && !is_multiframe_resume && enabled) publishes the grabbed exception into BH_LAST_EXC_VALUE and skips the pending-exc decline; clears a stale BH_LAST_EXC_VALUE on exception-free bridges; skips the legacy call-site exception prologue when routing is on. Assisted-by: Claude
…ume descr blackhole_resume_via_rd_numb wired the traced function's own frame-based residual-call descr (bh.jitcode.calldescr, arg_classes "r") into BhJitDriverSd.mainjitcode_calldescr. get_portal_runner returns that descr paired with bh_portal_runner, and the recursive-portal resume (bhimpl_recursive_call_r -> bh_call_r) verifies the merged green+red argument banks against it. bh_portal_runner consumes the warmspot portalfunc_ARGS: greens [next_instr:i, is_being_profiled:i, pycode:r] + reds [frame:r, ec:r] = 2 int + 3 ref. Set arg_classes to "iirrr" and result_type to 'r' so the descr matches the runner it is paired with. Assisted-by: Claude
…variant_name arm rustfmt collapses the match arm (it fits within the width limit); the exception-edge bridge commit left it wrapped across three lines, failing `cargo fmt --all -- --check` on CI. Assisted-by: Claude
bhimpl_recursive_call_r calls the portal runner through get_portal_runner
-> bh_call_r, which invokes it as a C function laid out by
mainjitcode_calldescr ("iirrr": next_instr, is_being_profiled, pycode,
frame, ec — five integer-register arguments). portal_runner_ptr held
bh_portal_runner, whose fn(&[i64], &[i64], &[i64]) -> i64 signature takes
three slice fat pointers instead, so the C-ABI call filled those six
registers with the five scalars: all_i became {ptr: next_instr,
len: is_being_profiled} (an empty slice, so next_instr read as 0),
all_r.first() dereferenced pycode as a slice base, and frame.pycode was
overwritten with that garbage, faulting in unsupported_jit_shape while
iterating a bogus constant pool.
Split bh_portal_runner's body into extern "C" fn bh_portal_runner_c taking
the five scalars directly and point portal_runner_ptr at it; the slice-form
bh_portal_runner now extracts and delegates. Change the field type to
Option<extern "C" fn(i64, i64, i64, i64, i64) -> i64>.
The path is reached when a non-bottommost blackhole frame reaches its own
jit_merge_point (blackhole.py:1066 recursive-call level), e.g. a
loop-bearing inlined callee resumed under PYRE_FBW_NESTED_RESID_ABORT=0.
Assisted-by: Claude
…ecursive callees `fbw_abort_nested_unjournaled_residual` declined every non-side-effect-free residual reached inside an inline sub-walk (framestack non-empty), blocking the depth-≥2 mutating-callee inline (#73). With the portal-runner ABI corrected, a straight-line mutating callee inlines bit-exact; only two callee shapes still miscompile and were masked by the blanket decline: * loop-bearing (its CodeObject has a FOR_ITER): the FOR_ITER-exempt double-advance when a kept-stack guard abort refuses the Option-C item delivery for a `for..break` frame parked past the loop header (foriter_exempt_shared_generator / foriter_exempt_nested_foriter). * self-recursive (it LOAD_GLOBALs its own name): the hot CALL_ASSEMBLER recursion bridge whose moving-nursery callee frame cannot survive the residual trampoline; on the wasm always-portal path the inlined body also type-confuses the optimizer (setintbound: got Ref) — wasm_ca_trampoline_decline. Decline only those two classes (`fbw_inline_callee_hazardous`), inlining every other nested residual. Both are properties of the framestack callee knowable at the residual decline point, so the whole trace aborts before the hazardous body commits. Adds `code_has_for_iter` / `code_is_self_recursive` to pyopcode. dynasm 241/241, cranelift 241/241, wasm 239/240 (only the pre-existing selfrec_tail_exception_unwind int+ZeroDivisionError). `PYRE_FBW_NESTED_RESID_ABORT=0` still disables the decline for A/B. Assisted-by: Claude
… live frame `try_walker_load_global_cell_fold`'s builtins-fallback arm required a materialised frame (`frame.get_builtin()`) and declined when the frame operand was unseeded — the case for an inlined callee's `portal_frame_reg`. #670 stopped seeding builtins into fresh module globals (`fresh_module_globals`, `exec_code_module` parity; also removes 141 builtins from `globals()`), so a builtin `LOAD_GLOBAL` inside an inlined callee (`len(bodies)` in nbody's `advance`) now misses the module-dict cell fold and falls to the `bh_load_global_fn` residual, whose unseeded frame operand is NO_CONCRETE. The residual declines and the whole inline aborts (`LoopBearingCalleeInlineUnsupported`), dropping nbody from 0.18s to 2.3s. When the frame is absent (`frame_ptr == 0`) derive the builtin module from the concrete globals' `__builtins__` cell — the same object `pick_builtin` resolves (`baseobjspace.rs:9716`) and the one the interpreter fallback rebuilds for the resumed callee frame — instead of requiring `frame.get_builtin()`. `__builtins__` is a stable module cell #670 keeps in every module dict, `ns_ptr` is the callee's authoritative namespace field, and the existing globals + builtins `version` QUASIIMMUT guards already fail the loop on a later `__builtins__` rebind or shadowing-name insert. The `frame_ptr != 0` path is unchanged. nbody: loops_aborted 5->0, 2.3s->0.18s, bit-exact -0.035117363568587606; `globals()` stays builtin-free (Python 3.14 parity). check.py dynasm 241/241, cranelift 241/241, wasm 240/240. Assisted-by: Claude
The committed pyre/bench/<name>.<backend>.jitstats baselines were only compared under --snapshot-diff, which the CI invocation (`pyre/check.py` bare) never passes, so the baselines were never enforced: a run whose loops_aborted rose from the baseline 0 passed both locally and in CI. Add a regression floor in _apply_snapshot_gate that runs on every non-record invocation whenever a committed .jitstats baseline exists. It gates only the sign-stable badness counters (loops_aborted, internal_compile_panics), whose healthy value is 0 on every platform, so a rise reddens the default run without --snapshot-diff and without the cross-runner-determinism risk of the count-valued fields (guard_failures/loops_compiled/bridges_compiled), which stay informational in the existing full-exact diff path. Update the Linux "JIT structural stats" CI step comment to record that the badness subset is now hard-gated by the main check.py step while that step remains informational for the count-valued fields. Assisted-by: Claude
… floor
The wasm runner emits its own [jit-stats] schema (compiles/executes/jit_calls +
memory sizes), which carries none of the native badness counters, so check.py's
regression floor had nothing to gate the wasm backend on and no wasm baseline
was recordable.
Export pyre_jit_loops_aborted / pyre_jit_internal_compile_panics from the guest
(reading the same driver_pair().0.get_stats() the native [jit-stats] reports),
and have the runner emit a "[jit-stats] loops_aborted={} internal_compile_panics={}"
line under MAJIT_STATS — the env check.py already sets for every backend. Under
check.py this is the single [jit-stats] line the snapshot picks up, so the floor
gates wasm on the two sign-stable counters exactly as it gates the native
backends. Exported rather than imported so reading them cannot perturb the JIT's
table/function indices.
Record the wasm jitstats baselines for the ten benches that run to completion on
wasm; all are healthy (loops_aborted=0 internal_compile_panics=0).
Assisted-by: Claude
nbody had no pypy-relative ceiling (d_vs_py / c_vs_py were None) and a loose 3x cpython ceiling. Set the native ratios to 1x cpython and 5x pypy so a wall-clock blowup is bounded alongside the structural jit-stats floor. Measured margins: dynasm 0.14x cpython / 1.5x pypy, cranelift 0.23x cpython / 2.7x pypy. Assisted-by: Claude
Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90d7921b80
ℹ️ 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".
| /// through `collect_call_args` + `bh_call_i_dispatch`, i.e. as a raw C function | ||
| /// whose five arguments arrive in integer registers per the calldescr — so the | ||
| /// runner must take those five scalars directly. (A Rust `fn(&[i64], …)` would | ||
| /// receive slice fat pointers there instead, corrupting every argument.) |
There was a problem hiding this comment.
This fixes the get_portal_runner/recursive_call path, but the per-JitCode path still stamps every JitCode.fnaddr from CallControl::get_jitcode_calldescr as bh_portal_runner with a one-ref calldescr (pyre/pyre-jit/src/jit/call.rs:365-371). When a blackhole resume executes a canonical inline_call_* descriptor, blackhole.rs calls cpu.bh_call_* with that fnaddr/calldescr, so it still invokes the slice-ABI fn(&[i64], ...) as a raw C function and corrupts the portal arguments; update that calldescr/fnaddr to use bh_portal_runner_c and the matching iirrr layout too.
Useful? React with 👍 / 👎.
…ne (#1106) * jit(wasm): drop the wasm32 arm of the self-recursive root-bridge inline `bridge_rec_root_selfrec` (inline_call.rs) was `cfg!(not(target_arch = "wasm32"))`, so a guard-failure bridge reaching a self-recursive callee's CALL took the root-bridge admission on the native backends and residualized on wasm. The cfg's stated reason — "the wasm always-portal path type-confuses the self-recursive inline (`setintbound: got Ref`)" — is phrasing carried over from #643 `920367da965`, where it belonged to `fbw_inline_callee_hazardous`: a decline that is not cfg-gated and rests on the CALL_ASSEMBLER trampoline frame. #749 `71726847a10` introduced the cfg and reported dynasm + cranelift 294/294 for that slice with no wasm number. `always-portal` names nothing in the tree. Neither mechanism that would make such a confusion wasm-specific holds: `W_IntObject.intval` is i64 on every target, so there is no word-size Int→Ref promotion, and general wasm CALL_ASSEMBLER support landed in #564 `c89254a6211` on 2026-07-15, before the claim. Add `synth/selfrec_bridge_nontail_promote`, covering the admission in the form `bridge_recursion_overflow` does not: non-tail recursion, so the guard's resume stream is multi-frame; a Ref local live across the recursive CALL; and an accumulator crossing 2**63 at level 13 of 24, so a long — a Ref — reaches the int operations. wasm reads loops/bridges/aborted/gf 2/4/1/606 with the decline and 2/6/0/806 without it, byte-identical to dynasm and cranelift. wasm jit-stats re-recorded for the five fixtures the admission moves. All five now carry the native values except `recursion_memo_branch`, one guard failure apart, previously twenty-one. check.py dynasm 405/406, cranelift 405/406, wasm 401/402. The one failure, `synth/pypy_type_surface`, reports the same `bridges_compiled 5 -> 102, guard_failures 1011 -> 20497` with this change's two files restored to origin/main, and it fails identically on all three backends while the change is a no-op wherever the cfg already read true. The fixture also holds no self-recursive function for `code_is_self_recursive` to answer. wasm user+sys CPU, min/median over 31 interleaved samples, decline removed vs kept: `wasm_ca_trampoline_decline` -20.1%/-20.7%, `selfrec_bridge_nontail_promote` -12.9%/-11.9%, `recursion_memo_branch` -1.2%/-1.6%. Two get slower: `ca_bridge_multiframe_resume_double_call` +1.2%/+2.7% and `foriter_call_resume_drops_iteration` +2.4%/+4.0%. Both now report what the dynasm baseline records — 16 bridges / 0 aborts and 27 bridges / 1 abort — where the decline left them at 16/1 and 26/2, so the cost is wasm taking the native decision and its compile toll. Assisted-by: Claude * jit: decline the `Cls.__name__` fold when its metaclass guard cannot hold `try_walker_specialize_load_type_name_attr` takes the metaclass from `baseobjspace::type_name_obj_fast_path`, which asks `typedef::type`. That falls back to `gettypefor(ob_type)` when the receiver's `w_class` slot is null (typedef.rs:234-243; the objects it covers sit in RODATA, so writing the slot would SIGBUS). The fold then guards the metaclass by reading the raw `w_class` field, so for such a receiver it emits `guard_value(NULL, type)` — a guard that fails on every execution and that nothing can discharge, because no later write fills the slot. One class reaches it. The `getset_descriptor` type object is built lazily from inside the init loop (typedef.rs:9501) as a builder for the descriptors other typedefs install, so it never enters the registry the post-loop sweep walks to stamp `w_class = type` (typedef.rs:1546-1553). `typedef::type` still answers `type` through the fallback, so `type(x)` and `x.__class__` are correct and the null slot is invisible from Python. `synth/pypy_type_surface` reads `type(type.__dict__[name]).__name__`, so it paid one guard failure per iteration on every backend: dynasm, cranelift and wasm all reported `bridges_compiled 5 -> 102, guard_failures 1011 -> 20497` against the figures #999 `dad2a722907` recorded — 19486 excess failures over 20000 iterations, and 19486/200 = 97 excess bridges, one per `trace_eagerness` bucket. #1097 `d51ea7f32a0`, which added the fold, merged 83 seconds before #999, and the recorded figures are the ones this decline reproduces. Localized by taking the fixture apart: `check_descriptor_kinds` alone carries all 97 bridges; the receiver's own value is irrelevant (a loop-invariant object reads the same), the `FOR_ITER` is irrelevant (removing it reads the same), and `int`, `str`, `list`, `NoneType`, `object`, `type`, `method_descriptor`, `wrapper_descriptor`, `builtin_function_or_method` and a user class are all clean. `getattr(C, "__name__")` — same semantics, but the fold declines because the site's name is not in the code's name table — is clean too. check.py dynasm 406/406, cranelift 406/406, wasm 402/402, all green. `synth/pypy_type_surface` reads its recorded 11/5/0/1011 exactly and `synth/type_name_attr_fold` still reads its own 5/0/1/4, so the fold keeps applying wherever its guard holds. The null slot itself is left alone. Stamping `w_class` on the `getset_descriptor` type object would restore the invariant the sweep's comment states and let the fold apply there as well, but it moves recorded baselines and belongs in its own change. Assisted-by: Claude * parity_tests: gate os_utime_pathconf_truncate's POSIX-only halves on Windows Two parts of the script do not run there: - the `times=` keyword check passed a time before the epoch, which the block above it already excludes Windows from, and the pyre backends answer it with `ValueError: utime: timestamp out of range`; - `pathconf` and the `pathconf_names` table it resolves through are registered under `#[cfg(unix)]`, and CPython's `os` carries neither on Windows, so both runtimes raise `AttributeError`. `pyre/check.py (windows-latest)` reports `os_utime_pathconf_truncate.py cpython=FAIL dynasm=FAIL cranelift=FAIL` at the merge-base d936eb4. The keyword spelling is now checked with a time the platform holds, and the pathconf section runs where the names table exists. Assisted-by: Claude * jit: read an aborted inline's Ref operand list at its argcode offset `reconstructed_all_ref_call_stack` read the residual op's Ref var-list at operand offset 1. That offset holds for the Ref-only residual shape (`iRd>r`) but not for the mixed one (`iIRd>r`), where the leading Int list is variable-width and `dispatch_residual_call_iIRd_kind` reads the Ref list at `1 + i_width`. On the mixed shape the read landed on the Int list's length byte and resolved its register indices through the Ref bank. The resulting list has an unrelated length, so the operand stack the abort-flush composes from it (kept vstack prefix + this list) still matched `depth_at_py_pc` at the CALL and was committed. For `p[0]` inside a `for p in ...` body the committed stack was `[iterator, p, iterator]`, and the interpreter re-executed the subscript with the loop's iterator as the index: `re.compile("|".join("%d" % x for x in range(2000)))` raised `TypeError: list indices must be integers or slices, not list_iterator` from `_compiler.py:504 _get_charset_prefix`, failing `test.test_re` in the vendored CPython suite. Derive the offset from the op's argcodes instead, walking the widths of `blackhole.py:112-157`; an op with no Ref list declines. Covered by `jitcode_dispatch::tests::ref_var_list_offset_follows_the_argcodes_not_a_fixed_byte` and `parity_tests/foriter_body_call_abort_operand_stack.py`. Assisted-by: Claude (cherry picked from commit 624d547)
Eight changes stacked on
nbody(rebased onto currentmain).1. Leave NONE holes for unboxed vstack TOS results
reconcile_vstack_at_boundary'sVstackOpClass::ResultToTosarm previouslykept the prior
vstack_last_refwhen the result was produced in an unboxedbank (
vstack_last_ref == OpRef::NONE), publishing a stale box into thecapture overlay. The arm now writes
vstack_last_refunconditionally whennew_depth > 0; aNONEresult leaves an intentional hole so the guard-captureoverlay omits the slot and resume rematerializes it.
Strictly narrowing: a real
Refresult is unchanged; only the unboxed case(previously a wrong-box publish) now defers. Local-safe subset of the Ref-bank
unboxed-int rematerialize work; the full interior-coordinate fix stays on the
walker Layer-3 frontier.
2. Retire 10 dead gate rows in gate-triage re-audit
Doc-only.
pyre/gate-triage.mdre-audited onnbody: 10 gates listed aslive/kept/deferred had already had their env readers deleted by closed epics
(0
.rsread sites remain — verified). Added §1c recording the retirement withper-gate PR/commit attribution, struck the stale rows at every location, and
recounted the summary buckets. Still-live gates (
PYRE_PROBE_AUTHORITATIVE,PYRE_P2_DRAIN,PYRE_FBW_VABLE_SCALAR_CA) left intact.3. Publish the nonstandard-virtualizable promote guard through the multi-frame resume chain
The
_nonstandard_virtualizablePTR_EQ promoteGuardValueemitted inside aninline sub-walk was captured by
walker_capture_inline_nonstandard_vable_guardwith a hardcoded sentinel resume word (
NO_JITCODE_PC), whichresolve_resume_pc_with_jitcode_pcrejects — so every such emit aborted withGuardResumeCoordinateUnavailable, collapsing the guard to a single outerframe. RPython gives this guard a plain full-framestack resume at its own
orgpc(pyjitpl.py:1120_nonstandard_virtualizable→1916implement_guard_value(eqbox, pc)→2582generate_guard(resumepc=orgpc)→2610capture_resumedatawalking the wholeMIFramechain,opencoder.py:819).When the paused-caller chain covers the full inline depth (the same gate as the
standard multi-frame guard path), route the guard through
walker_capture_multi_frame_inline_snapshot, publishing the callee's owncoordinate plus each paused caller. Because
emit_force_virtualizablerecordsGETFIELD_GC/PTR_NE/COND_CALLafter the promote, stamp the last guardop: add
capture_snapshot_for_last_guard_op_multi_frame_with_vable_vreftomajit history and a
stamp_last_guard_opparameter to the publisher. Keep thesingle-frame sentinel as the fallback for a partial chain. Gated
PYRE_FBW_NSVABLE_MULTIFRAME(default-on,=0restores the sentinel decline).Infrastructure step toward lifting the depth-≥2 mutating-callee decline (#73):
this guard is a prerequisite for that decline-lift, which stays gated behind
PYRE_FBW_NESTED_RESID_ABORTuntil every downstream wall is cleared. Thepromote guard is a runtime tautology (a fresh helper-allocated callee frame is
never pointer-equal to the live caller frame), so its resume is unreachable at
runtime; a malformed snapshot is a loud trace-time encoder abort, never a silent
wrong result.
4. Seed inline-callee param concretes into the callee-locals shadow for branching (try_multiframe) inlines
Next wall past change 3 under the decline-lift: a branching callee inlined with
a materialized virtual frame (
emit_new_pyframe_inline_with_params) hitGotoIfNotValueNotConcreteat its own first in-body branch after a may-force.CalleeLocalsShadowis the walker's per-inline-level MIFrame-register analog —recording-time concretes that survive a may-force heapcache clear. The
forward-inline sub-walk seeded it from the callee's param boxes only on the
!try_multiframe(strict fresh-frame fold) path. Atry_multiframecalleeinstead relied on the heapcache forward that the virtual-frame build registers
for its locals array — which any in-callee may-force op wipes wholesale via
reset_keep_likely_virtuals(heapcache.py:183; parity-faithful — RPythonwipes the same for
CALL_MAY_FORCE,heapcache.py:376). The post-callLOAD_FAST re-read of a param then missed the heapcache and produced a
non-concrete value.
The fix seeds
set_concrete(callee_portal_frame_reg, slot, concrete)for thetry_multiframeparams too, so the post-callgetarrayitem_vablereadfallback (
getarrayitem_vable_via_metainterp, written for exactly "aloop-carried local read after a may-force op") recovers the param concrete —
the analog of the callee MIFrame register box RPython reads
box.getint()off(registers survive a residual; the heapcache does not). The register-to-register
fold (
fold_frame_reg+set_opref) stays gated to!try_multiframe(itbypasses the real virtual frame, so the recorded op stream is byte-identical —
only the Void-fallback gains recovery).
STORE_FASTalready re-seeds the shadowvia the
setarrayitem_vablehandler; the bridge-carrier subwalk already doesthe identical seeding for reconstructed callees — the forward multiframe inline
was the one unseeded path. The inline virtual frame has no concrete allocation,
so no residual can reach it and no escape revalidation is needed.
5. Re-resolve the callee-locals shadow reads through the GC-forwarded op-table
Hardening follow-up to change 4.
CalleeLocalsShadow.concretestores a rawmajit_ir::Valuecopy, and the trace-ref walker does not visitcallee_shadow(it lives in
WalkContext, notTraceCtx), so aValue::Refcopy held thereis not forwarded when a minor collection moves a nursery object. A Ref param
allocated in-walk and seeded into the shadow could dangle if an in-callee
may-force residual triggered a minor GC before the post-call LOAD_FAST re-read;
change 4 widened this from the strict-fold/carrier paths to the forward inline
params.
The
getarrayitem_vableread fallback now prefers re-resolving the slot's SSAOpRef through
concrete_of_opref(the op-table value channel, which ISGC-forwarded) over the raw
concretecopy, falling back to the copy only whenthe OpRef carries no live concrete (ints/floats are value copies of immutable
payloads, always safe). To keep the two maps consistent so the preferred OpRef
is never stale:
set_oprefis now seeded on the try_multiframe param path too(the fold consumer stays gated by
fold_frame_reg, so it is inert for folding),and the
setarrayitem_vablenon-fold re-seed updatesset_oprefalongsideset_concreteon every store (matching the strict-fold store path). The OpRefis consulted only when a frame-matched
concreteentry exists, so the per-frameisolation the
frame_regfilter givesconcretealso bounds the re-resolution.6. Exception-edge bridge routing to the in-frame except handler (gated, default-off)
Adds
exc_edge_bridge_enabled()(PYRE_EXC_EDGE_BRIDGE, default-off). When set,an exception-guard bridge whose failing guard is caught in-frame routes the walk
to the in-frame
excepthandler instead of declining to the blackhole — theparity companion to the callee-inline decline-lift (changes 3–5), which needs an
exception edge to deliver a raise across an inlined call boundary.
jitcode_dispatch:find_catch_before_resume_liveselects the opcode's catch(backward op scan bounded by the call's post-call
-live-);exc_handler_rejoins_loopproves the handler reaches ajit_merge_point(route) vs terminates at a
*_return(decline).dispatch_via_miframeemitsthe
SAVE_EXC_CLASS/SAVE_EXCEPTION/RESTORE_EXCEPTION+GUARD_EXCEPTIONresumption at bridge-entry, stamps the restored-exception concrete onto the
SAVE_EXCEPTIONbox, enters the handler-entry vstack, and walks from the catchtarget. New
DispatchError::ExcEdgeCrossFrameReturnUnsupportedaborts across-frame-return handler before any recording.
call_jit:route_exc_edge(caught_in_frame && !is_multiframe_resume && enabled) publishes the grabbed exception intoBH_LAST_EXC_VALUEand skipsthe pending-exc decline; clears a stale
BH_LAST_EXC_VALUEon exception-freebridges; skips the legacy call-site exception prologue when routing is on.
Default-off: the OFF path is byte-identical to
main+ changes 1–5; no corpusbehavior changes until the gate is flipped as part of the multi-frame exc-edge
work.
7. Describe the portal runner in the blackhole recursive-portal resume descr
blackhole_resume_via_rd_numbwired the traced function's own frame-basedresidual-call descr (
bh.jitcode.calldescr,arg_classes"r") intoBhJitDriverSd.mainjitcode_calldescr.get_portal_runner(
blackhole.py:1095-1099) returns that descr paired withbh_portal_runner,and the recursive-portal resume (
bhimpl_recursive_call_r→bh_call_r)verifies the merged green+red argument banks against it — so it must describe
the runner, not the traced function.
bh_portal_runnerconsumes the warmspotportalfunc_ARGS(warmspot.py:972-975): greens[next_instr:i, is_being_profiled:i, pycode:r]+ reds[frame:r, ec:r](
interp_jit.py:67-68) = 2 int + 3 ref. Setarg_classesto "iirrr" andresult_typeto 'r' so the descr matches the runner it is paired with.Latent on
main— no corpus test reaches the blackhole recursive-portal resumepath (the
PYRE_FBW_NESTED_RESID_ABORTdecline keeps every nested-residualinline off it). It is a prerequisite of the depth-≥2 decline-lift (changes 3–5):
under
PYRE_FBW_NESTED_RESID_ABORT=0the traced-descr mismatch panickedBhCallDescr(arg_classes "r"has 0 int slots vs 2 merged int args); with therunner descr the recursive-portal resume verifies the banks correctly.
8. Register the deque iterator
#[pyre_class]types inbuild_gcRebasing onto current
mainsurfaced a pre-existing native breakage from #682(stdlib compat): it added
_collections._deque_iterator/_deque_reverse_iterator(W_DequeIter/W_DequeRevIter) as#[pyre_class]types, each holding an inline
deque: PyObjectRefchild, but never wired theminto
build_gc. Their resolved tid stayedUNASSIGNED, so malloc stamped tid 0(
object: empty offsets) and the marker forwarded none of their children — thesource deque swept while the iterator is live. The GC-root completeness oracle
(
eval.rs,cfg(not(wasm32))) flagged both on every native JIT init, so everydynasm/cranelift corpus test panicked (wasm PASS: oracle is native-only).
Register each via
register_pyre_classat the auto-id tail, mirroringW_Deque(#635's "immortal iterators + deque backing list" registration, same bug class),
and re-export the payloads from the
_collectionsmodule sobuild_gccan nametheir descriptors. Not part of the #73 JIT work — an independent
main-is-broken fix folded in to unblock this branch's native verification.
Validation
python pyre/check.py→ dynasm 241/241, cranelift 241/241 ALL PASSED; wasm239 passed, 1 skipped, 1 pre-existing fail (
synth/selfrec_tail_exception_unwind,see below). Rebased onto current
main(#674 gh#343 depth-2 recursive inline +PYPY_GC_MAX, #647 EXTENDED_ARG reset, #682 stdlib compat, #676 loop-perf).
Changes 3, 4, 6 and 7 are default-invisible for the depth-2 witnesses (change 6
only with
PYRE_EXC_EDGE_BRIDGE=1; changes 3/4/7 only with the decline lifted).Native was broken on
mainbefore change 8 (#682's unregistered dequeiterators panicked every dynasm/cranelift init); change 8 restores it — this
branch is 241/241 native where
mainis 0/241.Pre-existing wasm failure, not introduced here.
synth/selfrec_tail_exception_unwind(added by #647) fails on wasm with
TypeError: unsupported operand type(s) for +: 'int' and 'ZeroDivisionError'— the self-recursiveCALL_ASSEMBLERexception unwind consumes the raised value into the recursive-call return slot.
Confirmed pre-existing by a 3-way A/B: it fails identically (a) with all 8
commits, (b) with change 7's descr reverted, and (c) on pristine
origin/mainwith none of these commits. It reproduces only on wasm, where
PYRE_FBW_REC_MULTIFRAME(#674, default-on) is hard-on because the guest cannotread env vars; native (env-readable, same default-on) passes. Root cause is a
main-side wasm resume miscompile, out of scope for this branch.Rebase note: the only content conflict was change 3's
fbw_nsvable_multiframe_enabledgate landing next to #674'sfbw_rec_multiframe_enableddefault-ON flip infbw_state.rs— resolved as ablend (both gates kept, #674's default-ON doc preserved). Change 7's descr edit
and #674's per-frame
virtualizable_ptrbinding coexist inblackhole_resume_via_rd_numbwith no collision.— authored by Claude