wasm32 JIT: self-recursive CALL_ASSEMBLER on GC JitFrames + bridge-chaining groundwork - #312
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
WalkthroughAdds CA-aware wasm code generation and bridge execution support, expands loop/bridge metadata and diagnostics, and wires wasm host/runtime entry points plus runner toggles for bridges, CA, fuel, and stats. ChangesWasm CA In-Module Recursive Codegen and Bridge Infra
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 2822339). 1. 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
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dcbd52039
ℹ️ 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 constants: majit_ir::VecMap<u32, i64> = majit_ir::VecMap::new(); | ||
| let (bytes, guards, _, _) = codegen::build_wasm_module( | ||
| let (bytes, guards, _, _, _) = codegen::build_wasm_module( |
There was a problem hiding this comment.
This updated call site, and the other direct build_wasm_module calls in this test file, still pass only the old argument list even though the function now requires the trailing ca: CaParams parameter. The tuple destructuring was updated for the new return value, but the missing argument means majit-backend-wasm's test target will fail to compile with an arity error until these calls pass codegen::CaParams::default() or use a wrapper.
Useful? React with 👍 / 👎.
| // call_indirect(table_index, type_index): table 0, type for arity n. | ||
| sink.call_indirect(0, base + nargs as u32); |
There was a problem hiding this comment.
When a residual target's actual wasm signature is not exactly the synthetic (i64...)->i64 type, this direct call_indirect traps instead of using the existing host trampoline that reflects the target's real ValTypes and catches target traps (pyre-wasm-runner/src/main.rs:788-823). The JIT CallDescr only tells us logical Int/Ref types, not the callee's wasm ABI, so any eligible CallI/CallR helper with an i32 pointer ABI or a trapping residual target can now abort the whole wasm run rather than being marshalled/caught through jit_call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
majit/majit-backend-wasm/tests/codegen_test.rs (1)
45-58: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winAdd the trailing
codegen::CaParams::default()argument to everybuild_wasm_modulecall in this file.build_wasm_modulenow takes a required finalCaParams, so these invocations stop short and won’t type-check as written.🤖 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-backend-wasm/tests/codegen_test.rs` around lines 45 - 58, The build_wasm_module call in this test is missing the required trailing CaParams argument, so update every build_wasm_module invocation in codegen_test.rs to pass codegen::CaParams::default() as the final parameter. Use the existing build_wasm_module call site and its surrounding test helpers to locate all affected invocations and make them match the current function signature exactly.
🤖 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/src/call_jit.rs`:
- Around line 2742-2745: The guard-exit path in call_jit.rs is using a synthetic
green key via unwrap_or(0), which can pick the wrong compiled-exit layout when a
fail descr has no owning token. Update the logic around green_key and
mi.build_exit_layout_for_descr so it does not default to 0; instead, recover the
key from the owning compiled loop through majit_backend::descr_owning_jct when
possible, or fail loudly if no owning token exists. Keep the fix localized to
the guard exit handling in this section.
In `@pyre/pyre-wasm-runner/src/main.rs`:
- Around line 200-202: The PYRE_WASM_FUEL parsing in main.rs currently treats
any invalid value as None, which silently disables fuel. Update the fuel_limit
setup so invalid values for PYRE_WASM_FUEL are rejected explicitly instead of
falling back to no fuel, and preserve the existing Option<u64> behavior only for
a missing variable. Use the existing fuel_limit handling in main to surface a
clear error for bad input rather than calling parse().ok().
- Around line 242-254: Treat the wasm feature toggles in main as boolean flags
instead of mere presence checks: the PYRE_WASM_ENABLE_BRIDGES and PYRE_WASM_CA
branches should only enable their exports when the environment variable is set
to a non-disabled value, so "0" does not turn them on. Update the logic around
the env checks and the calls to instance.get_typed_func and f.call in main so
both toggles honor disabled values consistently, while still allowing missing
exports to remain no-ops.
---
Outside diff comments:
In `@majit/majit-backend-wasm/tests/codegen_test.rs`:
- Around line 45-58: The build_wasm_module call in this test is missing the
required trailing CaParams argument, so update every build_wasm_module
invocation in codegen_test.rs to pass codegen::CaParams::default() as the final
parameter. Use the existing build_wasm_module call site and its surrounding test
helpers to locate all affected invocations and make them match the current
function signature exactly.
🪄 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
Run ID: 8fbd4f78-24f0-4c88-8e3f-699f6351cb37
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
majit/majit-backend-wasm/src/codegen.rsmajit/majit-backend-wasm/src/failguard.rsmajit/majit-backend-wasm/src/lib.rsmajit/majit-backend-wasm/tests/codegen_test.rsmajit/majit-backend/src/lib.rsmajit/majit-metainterp/src/lib.rsmajit/majit-metainterp/src/pyjitpl.rspyre/pyre-jit/src/call_jit.rspyre/pyre-jit/src/eval.rspyre/pyre-wasm-runner/src/main.rspyre/pyre-wasm/Cargo.tomlpyre/pyre-wasm/src/lib.rs
| let green_key = majit_backend::descr_owning_jct(descr) | ||
| .map(|jct| jct.green_key) | ||
| .unwrap_or(0); | ||
| let exit_layout = mi.build_exit_layout_for_descr(green_key, descr); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not resume CA deopts with a synthetic green key.
unwrap_or(0) can route a guard exit through the wrong compiled-exit layout when the fail descr has no owning token. This path should fail loudly or recover the key from the owning compiled loop instead of blackhole-resuming with key 0.
Suggested fix
- let green_key = majit_backend::descr_owning_jct(descr)
- .map(|jct| jct.green_key)
- .unwrap_or(0);
+ let green_key = majit_backend::descr_owning_jct(descr)
+ .expect("CA deopt: fail descr has no owning JitCellToken")
+ .green_key;📝 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.
| let green_key = majit_backend::descr_owning_jct(descr) | |
| .map(|jct| jct.green_key) | |
| .unwrap_or(0); | |
| let exit_layout = mi.build_exit_layout_for_descr(green_key, descr); | |
| let green_key = majit_backend::descr_owning_jct(descr) | |
| .expect("CA deopt: fail descr has no owning JitCellToken") | |
| .green_key; | |
| let exit_layout = mi.build_exit_layout_for_descr(green_key, descr); |
🤖 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/call_jit.rs` around lines 2742 - 2745, The guard-exit path
in call_jit.rs is using a synthetic green key via unwrap_or(0), which can pick
the wrong compiled-exit layout when a fail descr has no owning token. Update the
logic around green_key and mi.build_exit_layout_for_descr so it does not default
to 0; instead, recover the key from the owning compiled loop through
majit_backend::descr_owning_jct when possible, or fail loudly if no owning token
exists. Keep the fix localized to the guard exit handling in this section.
| let fuel_limit: Option<u64> = std::env::var("PYRE_WASM_FUEL") | ||
| .ok() | ||
| .and_then(|s| s.parse().ok()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject invalid PYRE_WASM_FUEL instead of disabling fuel silently.
A typo like PYRE_WASM_FUEL=10s currently parses to None, disables fuel, and can hang the diagnostic livelock run this option is meant to stop.
Suggested fix
- let fuel_limit: Option<u64> = std::env::var("PYRE_WASM_FUEL")
- .ok()
- .and_then(|s| s.parse().ok());
+ let fuel_limit: Option<u64> = match std::env::var("PYRE_WASM_FUEL") {
+ Ok(s) => Some(
+ s.parse()
+ .with_context(|| format!("parse PYRE_WASM_FUEL={s:?} as u64"))?,
+ ),
+ Err(std::env::VarError::NotPresent) => None,
+ Err(e) => return Err(e).context("read PYRE_WASM_FUEL"),
+ };📝 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.
| let fuel_limit: Option<u64> = std::env::var("PYRE_WASM_FUEL") | |
| .ok() | |
| .and_then(|s| s.parse().ok()); | |
| let fuel_limit: Option<u64> = match std::env::var("PYRE_WASM_FUEL") { | |
| Ok(s) => Some( | |
| s.parse() | |
| .with_context(|| format!("parse PYRE_WASM_FUEL={s:?} as u64"))?, | |
| ), | |
| Err(std::env::VarError::NotPresent) => None, | |
| Err(e) => return Err(e).context("read PYRE_WASM_FUEL"), | |
| }; |
🤖 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-wasm-runner/src/main.rs` around lines 200 - 202, The PYRE_WASM_FUEL
parsing in main.rs currently treats any invalid value as None, which silently
disables fuel. Update the fuel_limit setup so invalid values for PYRE_WASM_FUEL
are rejected explicitly instead of falling back to no fuel, and preserve the
existing Option<u64> behavior only for a missing variable. Use the existing
fuel_limit handling in main to surface a clear error for bad input rather than
calling parse().ok().
| if std::env::var_os("PYRE_WASM_ENABLE_BRIDGES").is_some() { | ||
| if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_enable_bridges") | ||
| { | ||
| f.call(&mut store, 1)?; | ||
| } | ||
| } | ||
|
|
||
| // Enable the self-recursive CALL_ASSEMBLER guest→guest `call_indirect` arm | ||
| // (`PYRE_WASM_CA`). The guest has no environment, so the flag is plumbed | ||
| // through this export. No-op if the export is absent (older modules). | ||
| if std::env::var_os("PYRE_WASM_CA").is_some() { | ||
| if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_wasm_ca") { | ||
| f.call(&mut store, 1)?; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Treat "0" as disabled for wasm feature toggles.
var_os(...).is_some() enables bridges/CA even for PYRE_WASM_ENABLE_BRIDGES=0 or PYRE_WASM_CA=0, which can accidentally turn on dormant experimental paths.
Suggested fix
+ let env_flag_enabled = |name: &str| {
+ std::env::var_os(name)
+ .map(|v| {
+ let v = v.to_string_lossy();
+ !v.is_empty() && v != "0"
+ })
+ .unwrap_or(false)
+ };
+
- if std::env::var_os("PYRE_WASM_ENABLE_BRIDGES").is_some() {
+ if env_flag_enabled("PYRE_WASM_ENABLE_BRIDGES") {
if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_enable_bridges")
{
f.call(&mut store, 1)?;
}
}
@@
- if std::env::var_os("PYRE_WASM_CA").is_some() {
+ if env_flag_enabled("PYRE_WASM_CA") {
if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_wasm_ca") {
f.call(&mut store, 1)?;
}
}📝 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.
| if std::env::var_os("PYRE_WASM_ENABLE_BRIDGES").is_some() { | |
| if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_enable_bridges") | |
| { | |
| f.call(&mut store, 1)?; | |
| } | |
| } | |
| // Enable the self-recursive CALL_ASSEMBLER guest→guest `call_indirect` arm | |
| // (`PYRE_WASM_CA`). The guest has no environment, so the flag is plumbed | |
| // through this export. No-op if the export is absent (older modules). | |
| if std::env::var_os("PYRE_WASM_CA").is_some() { | |
| if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_wasm_ca") { | |
| f.call(&mut store, 1)?; | |
| let env_flag_enabled = |name: &str| { | |
| std::env::var_os(name) | |
| .map(|v| { | |
| let v = v.to_string_lossy(); | |
| !v.is_empty() && v != "0" | |
| }) | |
| .unwrap_or(false) | |
| }; | |
| if env_flag_enabled("PYRE_WASM_ENABLE_BRIDGES") { | |
| if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_enable_bridges") | |
| { | |
| f.call(&mut store, 1)?; | |
| } | |
| } | |
| // Enable the self-recursive CALL_ASSEMBLER guest→guest `call_indirect` arm | |
| // (`PYRE_WASM_CA`). The guest has no environment, so the flag is plumbed | |
| // through this export. No-op if the export is absent (older modules). | |
| if env_flag_enabled("PYRE_WASM_CA") { | |
| if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_wasm_ca") { | |
| f.call(&mut store, 1)?; | |
| } | |
| } |
🤖 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-wasm-runner/src/main.rs` around lines 242 - 254, Treat the wasm
feature toggles in main as boolean flags instead of mere presence checks: the
PYRE_WASM_ENABLE_BRIDGES and PYRE_WASM_CA branches should only enable their
exports when the environment variable is set to a non-disabled value, so "0"
does not turn them on. Update the logic around the env checks and the calls to
instance.get_typed_func and f.call in main so both toggles honor disabled values
consistently, while still allowing missing exports to remain no-ops.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a73838c1fc
ℹ️ 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 std::env::var_os("PYRE_WASM_CA").is_some() { | ||
| if let Ok(f) = instance.get_typed_func::<u32, ()>(&mut store, "pyre_jit_set_wasm_ca") { | ||
| f.call(&mut store, 1)?; |
There was a problem hiding this comment.
When a user sets only PYRE_WASM_CA, this block flips the backend CA codegen flag but leaves the wasm bridge tracer disabled; trace_and_compile_from_bridge still returns early while WASM_BRIDGES_ENABLED is false, so compile_bridge never runs and the self-recursive CALL_ASSEMBLER bridge/CA arm is never emitted. Either set pyre_jit_set_enable_bridges as part of this flag path or make the documented CA flag require the bridge flag, otherwise the advertised PYRE_WASM_CA mode remains a no-op unless users know to also set PYRE_WASM_ENABLE_BRIDGES.
Useful? React with 👍 / 👎.
…e parity Lower an eligible residual CALL (CallI/R, CallPureI/R, CallMayForceI/R, CallLoopinvariantI/R whose calldescr arg and result types are all Int/Ref) to a direct in-module call_indirect into the callee's __indirect_function_table slot with a static (i64xN)->i64 type, instead of the jit_call host trampoline. The residual ABI is uniformly i64 for Int/Ref args and result, so args and result move on the wasm stack with no marshalling and no call-area traffic. Gated by WASM_DIRECT_RESIDUAL_CALL; the type section declares one (i64xN)->i64 type per residual arity. call_indirect takes (table_index, type_index): table 0, type for the arity. Also carries the bridge-decline parity fixes: - Backend::bridge_decline_is_terminal() scopes declined_bridge_guards to wasm structural declines; native backends keep transient-retry semantics for Unsupported. - wasm bridge guard store_hash; decline bridges exceeding source frame output slots; RAII-own bridge cells instead of leaking. dynasm 160/160, wasm 150/160 (0 wrong; remaining fails are pre-existing heavy TIMEOUTs and list JIT-PANICs). Assisted-by: Claude
A loop-closing bridge (terminal JUMP, no LABEL) re-enters the source loop through its table-slot function entry. For a peeled loop that re-runs the preamble (the unrolled first iteration) instead of resuming at the LABEL, so the induction variable never advances; compile_bridge declined such bridges. Add a resume-at-LABEL preamble-skip dispatch for the single-label peeled shape (codegen::is_single_label_peeled), so a loop-closing bridge re-enters at the LABEL in-module instead of declining to a host round-trip: - A reserved frame slot DISPATCH_KEY_OFS (= MIN_FRAME_BYTES, between the call area and the Ref-home region; HOME_SLOT_BASE shifts +8). 0 = preamble/host entry (the execute_token frame is zeroed, +1 slot); 1 = resume at LABEL, written by the loop-closing JUMP arm before return_call_indirect. - build_function wraps such a loop in three blocks ($exit / $past_loader / $skip_preamble) and an entry br_if on the key: key 0 runs the entry inputarg loader + preamble then branches over the resume loader to the loop; key 1 skips both and runs the resume loader, which loads each label arg from its frame slot and refreshes its Ref home. The entry inputarg loader runs on the key-0 path only, so a resuming bridge never scatters frame values into the function inputargs' home slots. Preamble guards br to depth 2, body to 1. - CompiledWasmLoop.is_single_label_peeled (shared predicate, set in compile_loop) keys both the wrapper and the narrowed decline: compile_bridge now declines a loop-closing bridge only into a MULTI-label peeled loop (the br_table form is a follow-up). Also fix codegen_test's stale 4-tuple build_wasm_module destructures (the return is a 5-tuple) and add test_single_label_peeled_loop_validates, which validates the new control-flow nesting and br depths with wasmparser. dynasm 160/160, wasm 151/160 (0 wrong; remaining fails are the pre-existing heavy TIMEOUTs and list JIT-PANICs). The single-label-peeled shape is not what blocks the heavy benches' chaining, so their timings are unchanged. Assisted-by: Claude
Add two static-counter diagnostics, read out via guest exports at PYRE_WASM_JIT_STATS time, to trace why wasm guard exits do not chain into in-module bridges. Static counters + exports (not host imports, which would shift the module's function-index space and break the JIT's baked `fn as usize` table indices). - majit-backend-wasm: `BRIDGE_DIAG[8]` bumped at each `compile_bridge` outcome (entered / decline-CALL_ASSEMBLER / decline-multi-label-peeled / decline-not-direct-guard / decline-ref-home / compiled / loop-closing shape / source-has-preamble), read by `bridge_diag`. - majit-metainterp: `MC_DIAG[8]` bumped along the guard-failure → bridge-trace gate in `must_compile_with_values` (entered / declined-short-circuit / descr_addr==0 / status-busy / jitcounter-fired) and `start_retrace_from_guard` (entered / source-loop-evicted bail), read by `mc_diag`. - pyre-wasm: `pyre_jit_bridge_diag` / `pyre_jit_mc_diag` exports forward to those readers (direct path deps on majit-backend-wasm and majit-metainterp, both default-features=false, under the wasm-host feature). - pyre-wasm-runner: prints `[jit-stats] bridge_diag …` / `mc_diag …`. These measured that the wasm bridge tracer never reaches the backend: `must_compile` fires (FIRED>0) but `start_retrace_from_guard` is never entered, because `trace_and_compile_from_bridge` returns early under `#[cfg(target_arch = "wasm32")]`. Assisted-by: Claude
Extend the resume-at-LABEL preamble-skip wrapper from single-label peeled loops to any peeled loop, and accept a loop-closing bridge into a multi-label peeled source when it re-enters at the source's last LABEL (the one carrying the `loop`). - codegen.rs: add `is_resumable_peeled` (a peeled loop, single- or multi-label) and gate the wrapper on it instead of `is_single_label_peeled`; keep `is_single_label_peeled` as its single-label subset. The wrapper resumes at the last label, so its block nesting and br depths are byte-identical to the single-label case. - lib.rs compile_loop: stamp each LABEL's loop-target descr with its ordinal (`set_label_block_id`, skipping a descr without an `AtomicU32` slot) — pure metadata, no wasm bytes. Record `last_label_block_id` and `last_label_num_args` on `CompiledWasmLoop`; derive `has_preamble` through the shared predicate. - lib.rs compile_bridge: replace the blanket multi-label decline with a last-label check — accept when the closing JUMP's recovered target ordinal (read off its descr, shared by Arc identity with the source LABEL) equals the source's last label and the arity matches; decline (blackhole fallback) otherwise. The bridge_diag tally gains a sub-breakdown of the decline reason (stripped descr / non-last label / arity mismatch). - failguard.rs: `last_label_block_id`, `last_label_num_args` fields; updated `has_preamble` / `is_single_label_peeled` docs. - codegen_test.rs: a multi-label peeled loop validates with the same wrapper, and the per-LABEL registration stamps an ordinal readable from a JUMP that shares the LABEL's descr. The wasm bridge tracer remains dormant (call_jit.rs), so this is inert at runtime until chaining is enabled; with the tracer enabled a raise_catch loop-closing bridge (last-label target) is accepted, while nbody's bridges target non-last labels and stay declined pending the br_table form. Assisted-by: Claude
Replace the unconditional wasm `return false` in trace_and_compile_from_bridge with a check of a `WASM_BRIDGES_ENABLED` flag (default off, so behaviour is unchanged), settable at runtime through a new `pyre_jit_set_enable_bridges` guest export. The runner calls it when PYRE_WASM_ENABLE_BRIDGES is set, so inter-trace chaining can be enabled and measured without rebuilding the module. - pyre-jit call_jit.rs: `WASM_BRIDGES_ENABLED` AtomicBool + `set_wasm_bridges_enabled`; the dormancy early-return is now gated on the flag. - pyre-wasm: `pyre_jit_set_enable_bridges(u32)` export forwarding to the setter (export, not import, to keep the function-index space stable). - pyre-wasm-runner: call the export when PYRE_WASM_ENABLE_BRIDGES is set. Assisted-by: Claude
Add an optional instruction budget (PYRE_WASM_FUEL=N enables consume_fuel and sets the store fuel) so a diagnostic run that livelocks — e.g. a buggy loop-closing bridge — traps after N fuel units instead of hanging forever. Restructure the run so the PYRE_WASM_JIT_STATS readout happens regardless of whether the guest returned or trapped (the JIT counters are populated at compile time), refilling the store fuel first so the diagnostic export calls do not themselves trap on the exhausted budget. Assisted-by: Claude
…thmetic A loop-closing bridge re-enters the source loop through its terminal JUMP args and tail-calls the loop to iterate again. When none of those args is the result of an induction-advancing arithmetic op — every loop-carried value is a verbatim input reload, a fresh allocation, or a baked constant — the bridge re-presents byte-identical loop state on every pass, so the loop's exit guard never flips and the loop⇄bridge cycle spins at constant stack depth and heap state (a control- flow livelock; observed on fannkuch with the bridge tracer enabled: flat RSS, 100% CPU, no output). Add `is_inductive_arith` (the IntAdd/.. and float-arithmetic block, the overflow variants, and unary IntNeg/IntInvert; excludes copies, casts, comparisons, and allocations) and, in compile_bridge, decline any loop-closing bridge whose terminal JUMP has no arg produced by such an op. The guard then falls back to blackhole resume and declined_bridge_guards stops the metainterp re-tracing it. A bridge that advances a loop-carried value (an `i += 1` counter feeding a JUMP arg) still compiles and chains. BRIDGE_DIAG slot 11 (decl_noadvance) tallies the decline; the runner labels it. With PYRE_WASM_ENABLE_BRIDGES=1: fannkuch declines 10 such bridges and completes correctly; raise_catch's advancing bridge is still accepted (BRIDGE_OK=1). Assisted-by: Claude
Add the PYRE_WASM_CA arm: lower a self-recursive CallAssemblerR into an in-module call_indirect into the source loop instead of a host round-trip. Each callee frame is allocated as a GC-managed old-gen JitFrame via the wasm_jit_ca_alloc_frame / wasm_jit_ca_pop_frame trampolines (push_jf-rooted, traced by a per-frame gcmap over its input + home Ref slots); the source loop runs on frame_base + FIRST_ITEM_OFFSET so its local-0-relative slot accesses are unchanged. A guard deopt hands the callee frame to wasm_ca_resume_deopt, which blackhole-resumes it on the host. Run the wasm host entry frame F0 and the non-CA loops on real JitFrames too: push the jitframe GC type id into the wasm backend from eval.rs (set_wasm_jitframe_tid, the sole registration authority), size F0 for the larger of the loop's and any chained CA bridge's ref homes (CompiledWasmLoop::ca_bridge_ref_homes), and gate the path behind the runtime PYRE_WASM_CA flag (pyre_jit_set_wasm_ca export, set by the runner). build_exit_layout_for_descr (metainterp) and wasm_ca_resume_deopt (call_jit) reconstruct the deopt exit layout for a callee frame an in-guest call_indirect already ran to a guard exit. Assisted-by: Claude
… no-op residual_call_i64_arity: note CallMayForce* is direct-call eligible because the wasm backend has no force tokens / guard_not_forced — a forceable residual call needs no pre-call jf_descr (a guard inside it deopts through fail_index; the caller's live Refs survive via the JitFrame gcmap). CondCallGcWb arm: note the COND_CALL_GC_WB / COND_CALL_GC_WB_ARRAY ops are no-ops because the wasm backend emits the write barrier inline at each ref-store (write_barrier_base + emit_write_barrier) rather than consuming a standalone barrier op. Assisted-by: Claude
residual_call_i64_arity no longer accepts CallMayForceI/R, so a forceable
residual call keeps the jit_call trampoline instead of a direct call_indirect.
jf_descr (the force descr guard_not_forced reads) is set immediately before
CALL_MAY_FORCE, and the direct call_indirect does not reproduce that store. The
wasm backend has no force tokens, so the direct lowering would be sound, but the
fast-path stays parity-conservative and covers only the non-forceable residual
calls (Call{,Pure,Loopinvariant}{I,R}).
Assisted-by: Claude
build_wasm_module gained the trailing CaParams argument with the self-recursive CALL_ASSEMBLER arm; the codegen_test call sites still passed 11 arguments and failed to compile (E0061). Pass CaParams::default() (emit_ca off) at each call. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c1273aaf1
ℹ️ 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 descr.is_finish() { | ||
| Outcome::Finished(backend.get_ref_value(&frame, 0).as_usize() as i64) |
There was a problem hiding this comment.
When a self-recursive wasm CA callee exits via ExitFrameWithExceptionDescrRef, the helper reaches this descr.is_finish() branch and returns slot 0 as an ordinary Ref result without publishing the exception. The normal compiled outcome path treats is_exit_frame_with_exception() specially and raises it, and handle_blackhole_result only gets a chance to stash the exception on the non-finish deopt path; with this shortcut, recursive calls that raise under PYRE_WASM_CA can be observed as a normal object return instead of propagating the Python exception.
Useful? React with 👍 / 👎.
#262
Summary
wasm32 JIT backend work layered on
main(1b45bbd712); 8 commits, all wasm backend / runner. Headline is the self-recursiveCALL_ASSEMBLERfast path on GC-managedJitFrames; the rest is inter-trace bridge-chaining groundwork (default-off) plus runner diagnostics.Self-recursive
CALL_ASSEMBLERon GC-managedJitFrames (PYRE_WASM_CA, default-off)CallAssemblerRinto an in-modulecall_indirectinto the source loop instead of a host round-trip.JitFrame(wasm_jit_ca_alloc_frame/wasm_jit_ca_pop_frametrampolines,push_jf-rooted, traced by a per-frame gcmap over its input + home Ref slots); the source loop runs onframe_base + FIRST_ITEM_OFFSET, so its local-0-relative slot accesses are unchanged.F0and the non-CA loops also run on realJitFrames (jitframe GC type id pushed into the wasm backend fromeval.rs, the sole registration authority);F0is sized for the larger of the loop's and any chained CA bridge's ref homes.wasm_ca_resume_deopt, which blackhole-resumes it on the host.fib(20..30)correct, andfib(24)under a tiny nursery (PYPY_GC_NURSERY=131072, forcing mid-recursion collections) correct.Inter-trace bridge-chaining groundwork (bridge tracer default-off via
PYRE_WASM_ENABLE_BRIDGES)pyre_jit_set_enable_bridgesexport — export, not host import, to keep the baked function-index space stable).call_indirectfor eligible residual Int/Ref calls.Runner / diagnostics
PYRE_WASM_FUEL=Ninstruction-budget interrupt so a diagnostic livelock traps after N units instead of hanging; thePYRE_WASM_JIT_STATSreadout now runs on trap as well as on clean return.Self-review
This patch is AI-assisted (every commit carries an
Assisted-by: Claudetrailer).A local Codex parity review (gpt-5.5, the prompt below) was run, but note the template's caveat that review should be done in a separate session — an independent review pass is still recommended before merge. For reference, that run reported section 1 (parity regressions introduced by this patch) = None; the only flagged mismatches were in
model_ssa.rs, which belongs to the base (#309, already onmain) and is outside this PR's diff.Assisted-by🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests