Repository navigation
jit: force the caller's frame on its own vable_token, through the real allocator, and reuse what the force materialized - #899
Conversation
…ugh the registered allocator `force_pyframe` required `ptr::eq(frame, MetaInterp::vable_ptr)` in addition to an armed token. `vable_ptr` is written only by `sync_before`, which a callee's own compiled-loop entry runs from inside the caller's running loop, and nothing restores it when that activation returns; the caller therefore stopped matching for the rest of its own activation. Test the frame's own `vable_token` instead, as `virtualizable.py:214-217 force_virtualizable_if_necessary` does. The force then ran `MetaInterp::force_virtualizable_token`, which hardcoded `NullAllocator`, whose `BlackholeAllocator` default bodies return 0. A vable slot whose resume value is a virtual was written as null, and `fast2locals` renders a null slot as an absent name. Route the force through `JitDriver::force_virtualizable_token`, which supplies the registered blackhole allocator, and drop the `NullAllocator` wrapper; `force_pyframe_vref` already took that path. Together these were a JIT-only wrong answer: a callee reading its caller's frame through `sys._getframe(1)` saw the caller's locals frozen at the value they held when the caller's loop was compiled. Added `synth/getframe_caller_locals_nested_compiled_callee.py`, which returned 104995 instead of 105005 on both native backends before this change. check.py: dynasm 341/342, cranelift 341/342, wasm 338/339 — the single failure on each is the pre-existing `synth/str_search_index_bounds` red on main. 327-fixture JIT-vs-interpreter differential unchanged on both native backends. Assisted-by: Claude
`force_virtualizable_token_with_allocator` discarded what `handle_async_forcing_with_allocator` returned, so the GUARD_NOT_FORCED failure that follows a force decoded the resume data again through a fresh `ResumeDataDirectReader`. Probing `write_from_resume_data_partial` with `reader.resume_after_guard_not_forced` on `bench/synth/getframe_caller_locals_nested_compiled_callee.py`, the forced frame took 5 `ragnf=1` writes from the forces and 5 `ragnf=0` writes from the re-decode, and the second pass installed a different pointer for a slot the first pass had materialized. After: 5 `ragnf=1`, 0 `ragnf=0`, each force consumed once. `force_virtualizable_token_with_allocator` and `JitDriver::force_virtualizable_token` now return the cache. `force_pyframe` stores it on `MetaInterp::forced_virtuals`, keyed by the forced `PyFrame`, and `blackhole_from_resumedata` takes an `all_virtuals` parameter that reaches `ResumeDataDirectReader::new` — which already implemented the `resume_after_guard_not_forced == 2` branch that skips the vable section and the vref pairs. On that branch `_prepare` is skipped, so `rd_virtuals` and `rd_guard_pendingfields` are not passed to `prepare_resume_heap_with_roots` (resume.py:1368-1375). The consume side is gated on `Descr::is_guard_forced()`, which `invent_fail_descr_for_op` sets for GUARD_NOT_FORCED / GUARD_NOT_FORCED_2, so no other guard reads the cache — compile.py:950-958 is the only `handle_fail` that calls `get_savedata_ref`. A key miss falls back to a full decode rather than compile.py:959-960's empty `VirtualCache`, because a deadframe-local `jf_savedata` slot cannot miss and a frame-keyed lookup can. Two upstream mechanisms are unavailable here. `jf_savedata` cannot hold the cache: the GC traces it as an object reference (`majit-backend/src/jitframe.rs:278`). `FailDescr::force_token_slots` cannot key it: only cranelift calls `set_force_token_slots`, so on dynasm the failing exit does not name its own force token. dynasm and cranelift: `bench/synth` JIT vs `PYRE_NO_JIT=1` 328/328 identical, `check.py` 343/343 each, `majit-metainterp --lib` 1415 passed. Assisted-by: Claude
`run_bench` and `run_selfcheck` already took `skip_backends`; the synthetic suite discovers its fixtures by glob and had no equivalent, so a fixture that exercises a mechanism one backend does not implement could only be red there. `synth_skip_backends` reads `# pyre-check: skip-backends=<names>` from the first 20 lines the way `synth_perf_gate` reads `max-pypy-ratio=`, and `run_synthetic_bench` prints `skip` for those backends. An unrecognized backend name raises rather than exempting nothing. `getframe_caller_locals_nested_compiled_callee.py` uses it for wasm. That backend reaches neither defect the fixture guards: `OpCode::ForceToken` lowers to a literal `0` sentinel (`majit-backend-wasm/src/codegen.rs:4225`), so the compiled loop stores 0 into `vable_token` and `force_virtualizable_if_necessary` never forces, and the backend does not override `force()` (`majit-backend/src/lib.rs:2170` returns None) so it has no deadframe to decode. It printed 104995 against 105005 on the native backends and CPython. The fixture header records that chain. check.py: dynasm 343/343, cranelift 343/343, wasm 339/339. Assisted-by: Claude
…ult the fixture exemption covers `compile.py:706-709` was cited three times for the GUARD_NOT_FORCED savedata fetch. Those lines are the must_compile branch of the base `ResumeGuardDescr.handle_fail` (`_trace_and_compile_from_bridge` / `finally: done_compiling()`); the fetch is `compile.py:956-958` inside `ResumeGuardForcedDescr.handle_fail`, which overrides `handle_fail`, so a GUARD_NOT_FORCED failure never reaches line 706 at all. `virtualizable.py:214-217` was cited for `force_virtualizable_if_necessary`; those lines are `cast_gcref_to_vtype` and the function is at `virtualizable.py:282-284`. The wasm exemption on `bench/synth/getframe_caller_locals_nested_compiled_callee.py` said the backend "reaches neither defect". It reaches the first one's symptom: wasm reads 3691 on all nine `f_locals['acc']` reads while the value walks 104967 -> 105000, and prints 104995 where dynasm, cranelift, `PYRE_NO_JIT=1` and CPython print 105005. It is not a blanket failure — the same read is correct on wasm when the callee does not take a compiled entry from inside the caller's running loop — so the shape this fixture builds is one the wasm backend's stated invariant does not hold for, the invariant it cites three times to lower `GuardNotForced` to a no-op (`majit-backend-wasm/src/codegen.rs:859`, `:908`, `:950`, guard at `:2175`). The header records that, and the directive moved to line 1 so it cannot drift out of the 20-line window. `synth_skip_backends`' docstring now states that a directive below the window makes the fixture run everywhere and go red rather than being honored silently, and that the exemption deliberately does not cover the three baseline-failure paths, where a broken fixture must stay loud on every backend. Assisted-by: Claude
WalkthroughThe change captures forced virtualizable state by owner, threads it through guard-failure blackhole resume paths, and avoids duplicate virtual materialization. A synthetic benchmark and per-benchmark backend skip directives are added for regression coverage. ChangesForced virtual capture and resume
Synthetic benchmark handling
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 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 97a0d22). 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: 97a0d2293e
ℹ️ 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".
| /// (`jitframe.rs:278`) — so the cache is held here, keyed by the | ||
| /// virtualizable that was forced. One entry per frame, overwritten on | ||
| /// re-force, exactly like the single `jf_savedata` word. | ||
| pub(crate) forced_virtuals: Vec<(u64, Vec<i64>, Vec<i64>)>, |
There was a problem hiding this comment.
When a residual helper forces a frame and then allocates before returning to GUARD_NOT_FORCED, this field is the only owner of some materialized virtuals, but its pointer half is stored as raw i64 values and none of the registered MetaInterp root walkers visits it. A major collection in that interval can therefore reclaim cache-only old-generation objects, after which the guard resume installs dangling pointers from all_virtuals; the upstream jf_savedata representation avoided this specifically by being GC-traced, so the replacement cache needs an equivalent root walker for its lifetime.
AGENTS.md reference: AGENTS.md:L153-L155
Useful? React with 👍 / 👎.
| /// failure. In particular, a `jit.virtual_ref` frame must not be decoded | ||
| /// through `NullAllocator`, or its `forced` writeback remains null. | ||
| pub fn force_virtualizable_token(&mut self, token: u64) { | ||
| pub fn force_virtualizable_token(&mut self, token: u64) -> Option<(Vec<i64>, Vec<i64>)> { |
There was a problem hiding this comment.
When frame-chain access forces an active JitVirtualRef, force_pyframe_vref at eval.rs:3893 still calls this newly value-returning method as a statement, unlike force_pyframe, which saves the returned cache. The following forced-guard lookup consequently misses and performs a full second decode, so the frame exposed through vref.forced and the resumed registers can receive distinct materializations of the same virtual object—the identity defect this cache was added to prevent. The virtual-ref caller must persist the returned cache against the corresponding forced frame/deadframe too.
AGENTS.md reference: AGENTS.md:L194-L195
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
majit/majit-metainterp/src/resume.rs (1)
5140-5202: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new
all_virtuals = Some(...)branch.The existing test only exercises
all_virtuals = None(unchanged behavior). Since this PR's core fix is theSome(...)skip-prepare path, a focused unit test here (assertingprepare_resume_heap_with_rootsis not re-run withrd_virtuals/rd_guard_pendingfieldswhenall_virtualsisSome) would directly validate the mechanism instead of relying solely on the end-to-end synthetic bench.🤖 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 `@majit/majit-metainterp/src/resume.rs` around lines 5140 - 5202, Add a focused unit test alongside blackhole_from_resumedata_accepts_runtime_jitcode_without_canonical_pair that passes all_virtuals as Some(...). Verify the resume path skips prepare_resume_heap_with_roots for rd_virtuals and rd_guard_pendingfields while preserving the successful resumed blackhole behavior and existing assertions.pyre/pyre-jit/src/eval.rs (1)
3887-3894: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCache forced virtuals in
force_pyframe_vrefusing the owner that will resume with this cache.
force_pyframesavesdriver.force_virtualizable_token(token)under the frame pointer thatforce_pyframeforced.force_pyframe_vrefcurrently discards the returned caches, so any laterGUARD_NOT_FORCEDresume that expects this materialization misses and can rebuild/replay the virtuals through the ordinary decode path. Preserve the same cache only when the owner used by guard resume matches the forced v-frame; otherwise don’t store it here unless the resume keying is updated to the same owner.🤖 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/src/eval.rs` around lines 3887 - 3894, The force_virtual flow in force_pyframe_vref must retain the caches returned by driver.force_virtualizable_token for later GUARD_NOT_FORCED resumes. Store them under the same owner/key used by guard resume, matching force_pyframe’s behavior, and only cache them when that owner is the forced v-frame; otherwise leave caching unchanged unless resume keying is updated consistently.
🤖 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 `@majit/majit-metainterp/src/jitdriver.rs`:
- Line 3766: The async forcing paths in handle_async_forcing callers at
majit/majit-metainterp/src/jitdriver.rs:3766-3766 and 6209-6209 must preserve
the returned StorageGuardForced::all_virtuals cache. Unwrap or otherwise obtain
the all_virtuals result and pass it instead of None into
blackhole_from_resumedata so GUARD_NOT_FORCED resumption reuses existing
virtuals rather than rebuilding them.
---
Outside diff comments:
In `@majit/majit-metainterp/src/resume.rs`:
- Around line 5140-5202: Add a focused unit test alongside
blackhole_from_resumedata_accepts_runtime_jitcode_without_canonical_pair that
passes all_virtuals as Some(...). Verify the resume path skips
prepare_resume_heap_with_roots for rd_virtuals and rd_guard_pendingfields while
preserving the successful resumed blackhole behavior and existing assertions.
In `@pyre/pyre-jit/src/eval.rs`:
- Around line 3887-3894: The force_virtual flow in force_pyframe_vref must
retain the caches returned by driver.force_virtualizable_token for later
GUARD_NOT_FORCED resumes. Store them under the same owner/key used by guard
resume, matching force_pyframe’s behavior, and only cache them when that owner
is the forced v-frame; otherwise leave caching unchanged unless resume keying is
updated consistently.
🪄 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: e142551d-6d34-456d-84bb-f9daa39565cc
📒 Files selected for processing (8)
majit/majit-metainterp/src/blackhole.rsmajit/majit-metainterp/src/jitdriver.rsmajit/majit-metainterp/src/pyjitpl.rsmajit/majit-metainterp/src/resume.rspyre/bench/synth/getframe_caller_locals_nested_compiled_callee.pypyre/check.pypyre/pyre-jit/src/call_jit.rspyre/pyre-jit/src/eval.rs
| .map(|a| a.as_ref() as &dyn crate::resume::VirtualizableInfo), | ||
| None, // ginfo | ||
| vable_identity_override, | ||
| None, // all_virtuals |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'majit/majit-metainterp/src/jitdriver\.rs$' || true
echo "== outline around relevant symbols =="
ast-grep outline majit/majit-metainterp/src/jitdriver.rs --match back_edge_internal --view expanded || true
ast-grep outline majit/majit-metainterp/src/jitdriver.rs --match run_back_edge_generic --view expanded || true
echo "== context around reported lines =="
sed -n '3720,3790p' majit/majit-metainterp/src/jitdriver.rs
sed -n '6170,6240p' majit/majit-metainterp/src/jitdriver.rs
echo "== all "all_virtuals" occurrences =="
rg -n "all_virtuals|GUARD_NOT_FORCED|back_edge_internal|run_back_edge_generic" majit/majit-metainterp/src/jitdriver.rsRepository: youknowone/pyre
Length of output: 10028
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== function signatures and call sites =="
sed -n '2030,2085p' majit/majit-metainterp/src/jitdriver.rs
sed -n '3190,3460p' majit/majit-metainterp/src/jitdriver.rs
sed -n '3460,3870p' majit/majit-metainterp/src/jitdriver.rs
sed -n '5800,6285p' majit/majit-metainterp/src/jitdriver.rs
echo "== all_virtuals declarations/usages across repo (case-sensitive) =="
rg -n "all_virtuals|rd_virtuals|virtuals_cache|GuardForced|GUARD_NOT_FORCED|GUARD_NOT_FORCED" -S .Repository: youknowone/pyre
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate resume module files =="
git ls-files majit/majit-metainterp/src | rg 'resume|virtual|guard|compiled' || true
echo "== blackhole_from_resumedata definitions/usages =="
rg -n "pub fn blackhole_from_resumedata|fn blackhole_from_resumedata|blackhole_from_resumedata\\(" -S majit/majit-metainterp/src -C 3
echo "== all_virtuals references with context =="
rg -n "all_virtuals" -S majit/majit-metainterp/src -C 5 || true
echo "== compiled exit layout fields relevant to virtuals =="
rg -n "struct CompiledExitLayout|rd_virtuals|virtuals|PendingFields|GuardDescr|Guard" -S majit/majit-metainterp/src -C 2 | head -n 240Repository: youknowone/pyre
Length of output: 40669
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pyjitpl force/guard-failure section =="
sed -n '11600,11735p' majit/majit-metainterp/src/pyjitpl.rs
echo "== handle_fail resume sections with force/cache storage =="
sed -n '11690,11725p' majit/majit-metainterp/src/pyjitpl.rs
sed -n '2030,2095p' majit/majit-metainterp/src/jitdriver.rs
sed -n '3190,3255p' majit/majit-metainterp/src/jitdriver.rs
echo "== all handle_async_forcing calls/usages =="
rg -n "handle_async_forcing|force_now|force_all_virtuals|all_virtuals_ptr|all_virtuals_int" -S majit/majit-metainterp/src -C 3Repository: youknowone/pyre
Length of output: 43861
Store the forced virtual cache so GUARD_NOT_FORCED resumption can reuse it.
handle_async_forcing returns (all_virtuals_ptr, all_virtuals_int), but these sites only call guard failure/resume through blackhole_from_resumedata(..., None, ...) and never carry that cache back into the failure handler. Pass/unwrap the StorageGuardForced::all_virtuals result from the async force and thread it through to the resume path so the already-forced virtuals are reused instead of rebuilt.
📍 Affects 1 file
majit/majit-metainterp/src/jitdriver.rs#L3766-L3766(this comment)majit/majit-metainterp/src/jitdriver.rs#L6209-L6209
🤖 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 `@majit/majit-metainterp/src/jitdriver.rs` at line 3766, The async forcing
paths in handle_async_forcing callers at
majit/majit-metainterp/src/jitdriver.rs:3766-3766 and 6209-6209 must preserve
the returned StorageGuardForced::all_virtuals cache. Unwrap or otherwise obtain
the all_virtuals result and pass it instead of None into
blackhole_from_resumedata so GUARD_NOT_FORCED resumption reuses existing
virtuals rather than rebuilding them.
Three stacked defects in one code path — a callee reading its caller's frame
through
sys._getframe(1)saw the caller's locals frozen at the value they heldwhen the caller's loop was compiled. Silent wrong answer on both native backends,
no crash, no abort.
Regression fixture:
bench/synth/getframe_caller_locals_nested_compiled_callee.py—
104995before,105005after, matchingPYRE_NO_JIT=1and CPython.L1 — the force was skipped (
force_pyframe)sys._getframe(depth)does force the frame it hands out, soforce_pyframewasreached with the caller frame. It then additionally required
ptr::eq(frame, MetaInterp::vable_ptr). That cell is written only bysync_before, and the callee's own compiled-loop entry runssync_beforefrominside the caller's still-running loop, re-pointing it at the callee frame. Nothing
restores it —
sync_before(8 sites) andsync_after(7 sites) are not paired1:1 — so the caller stopped matching for the rest of its activation.
Fixed by testing the frame's own
vable_token, which is whatvirtualizable.py:282-284 force_virtualizable_if_necessarydoes. Safe becausePyFrameis built withvable_token: 0(pyframe.rs:3790,:4096) andmaterialization never writes the slot.
L2 — the force then wrote NULL over a live local
With L1 fixed the fixture raised
KeyError: 'acc'. The force went throughMetaInterp::force_virtualizable_token, hardcoded toNullAllocator, whoseBlackholeAllocatordefault method bodies return0. A vable slot whose resumevalue is a virtual therefore materialized as
0, andfast2localsrenders anull slot as an absent name.
Fixed by routing through
JitDriver::force_virtualizable_token, which suppliesthe registered blackhole allocator, and deleting the
NullAllocatorwrapper sothe footgun cannot be re-picked.
compile.py:966-1000 force_nowhas noallocator-free form.
An earlier hypothesis — that the resume image did not cover every vable array
slot — was refuted:
consume_vable_infoassertsget_total_size(virtualizable) == vable_size - 1, so a short vector would panicrather than silently null.
L3 — the async-forcing virtual cache was discarded
force_virtualizable_token_with_allocatordropped whathandle_async_forcing_with_allocatorreturned, so theGUARD_NOT_FORCEDfailurethat follows a force decoded the resume data again through a fresh
ResumeDataDirectReader.Measured by probing
write_from_resume_data_partialwithreader.resume_after_guard_not_forced(ragnf):ragnf=1(the forces) + 5×ragnf=0(the re-decode); the second pass installed a different pointer for a slot the first had materialized (item[1] 0x…9140 → 0x…91c0)ragnf=1, 0×ragnf=0; each force consumed exactly once, nothing left pendingResumeDataDirectReader::newalready implemented theall_virtuals is Some→(2, from_caches(..))branch (resume.rs:6134) — it had simply never beenpassed a
Some. Only the producer, the store and the consumer were missing.With
ragnf == 2,consume_vref_and_vablejumps the vable section and the vrefpairs, and
_prepareis skipped entirely, so neither_prepare_virtualsnor_prepare_pendingfieldsruns (resume.py:1368-1375).Two upstream mechanisms are unavailable here, each forcing a documented choice:
jf_savedatacannot hold the cache. The GC traces it as a real objectreference (
majit-backend/src/jitframe.rs:120is ausize,:278trace_callback(&mut (*obj_addr).jf_savedata)), so a leaked Rust value therewould be walked as a heap object. The cache lives on
MetaInterp::forced_virtualsinstead, one entry per frame, overwritten onre-force exactly like the single
jf_savedataword.FailDescr::force_token_slotscannot key it. Only cranelift callsset_force_token_slots; dynasm leaves it empty. Keying on the jitframe addresssilently never hit on dynasm — that version was built and measured, and the
ragnf=0re-write was still there. The working key is the identity both endsshare: the forced
PyFrame.The consume side is gated on
Descr::is_guard_forced(), the flaginvent_fail_descr_for_opsets forGUARD_NOT_FORCED/GUARD_NOT_FORCED_2, sono other guard reads the cache —
compile.py:950-963is the onlyhandle_failthat calls
get_savedata_ref. A key miss falls back to a full decode ratherthan
compile.py:959-960's emptyVirtualCache, because a deadframe-local slotcannot miss and a frame-keyed lookup can; an empty cache plus a skipped vable
section would leave the resumed virtuals unbound.
Five call sites need the new parameter, and a native
cargo checksees only four— the fifth is the wasm CALL_ASSEMBLER deopt path (
call_jit.rs:3819).check.pycaught it.check.py:
# pyre-check: skip-backends=run_benchandrun_selfcheckalready tookskip_backends; the synthetic suitediscovers fixtures by glob and had no equivalent.
synth_skip_backendsreads thedirective from the first 20 lines the way
synth_perf_gatereadsmax-pypy-ratio=. An unrecognized backend name raises rather than exemptingnothing, and a directive below the window makes the fixture run everywhere and go
red rather than being honored silently. The exemption deliberately does not
cover the three baseline-failure paths (cpython crash, pypy crash, cpython/pypy
mismatch) — a broken fixture must stay loud on every backend.
Known wasm defect this exempts (not fixed here)
wasm still gets the fixture wrong, and the exemption is there so the guard stays
meaningful on the backends it guards — not because wasm is right.
Measured: wasm reads
3691on all ninef_locals['acc']reads while the valuewalks 104967 → 105000, printing
104995; dynasm, cranelift,PYRE_NO_JIT=1andCPython print
105005. It is not a blanket wasmf_localsfailure — the sameread is correct on wasm when the callee does not take a compiled entry of its own
from inside the caller's running loop. That shape breaks the invariant the wasm
backend states three times to justify lowering
GuardNotForcedto a no-op:"the wasm virtualizable is always materialized"
(
majit-backend-wasm/src/codegen.rs:859,:908,:950; the no-op guard at:2175).Neither fix above can reach it:
OpCode::ForceTokenlowers to a literal0sentinel (
codegen.rs:4225), so the compiled loop stores 0 intovable_tokenandforce_virtualizable_if_necessarynever forces; and wasm does not overrideforce()(majit-backend/src/lib.rs:2170returnsNone), so there would be nodeadframe to decode. wasm holds loop state in wasm locals rather than a heap
jitframe, so forcing mid-execution is a wasm backend epic of its own.
Also corrected
Four upstream citations that were wrong.
compile.py:706-709was cited threetimes for the
GUARD_NOT_FORCEDsavedata fetch; those lines are themust_compilebranch of the baseResumeGuardDescr.handle_fail, and becauseResumeGuardForcedDescroverrideshandle_fail, a forced guard never reachesline 706 at all — the fetch is
compile.py:956-958.virtualizable.py:214-217was cited for
force_virtualizable_if_necessary; those lines arecast_gcref_to_vtypeand the function is at:282-284.Not addressed
force_pyframe_vrefdiscards the cacheforce_virtualizable_tokennow returns,keeping the double-decode on the vref path. Its forced-frame identity is murkier
than the direct
force_pyframecase.An earlier note claiming pyre lacked upstream's "failures of a GUARD_NOT_FORCED
are never compiled, but always just blackholed" rule was wrong: pyre
implements it as descr dispatch, not an opcode check —
call_jit.rs:2861 if descr_arc.is_guard_forced() { return BridgeResolution::ResumeBlackhole; }.Verification
On base
d873f1396e(#742):check.py— dynasm 344/344, cranelift 344/344, wasm 340/340bench/synthJIT vsPYRE_NO_JIT=1differential — 329/329 identical onboth native backends
cargo test --release -p majit-metainterp --features dynasm --lib— 1416passed, 0 failed
105005on dynasm, cranelift,PYRE_NO_JIT=1and CPython🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Testing