Skip to content

jit: decline wasm bridges with CallMayForce re-entry (real fix for the #404 workaround)#407

Merged
youknowone merged 1 commit into
mainfrom
wasm-callmayforce-bridge-decline
Jul 8, 2026
Merged

jit: decline wasm bridges with CallMayForce re-entry (real fix for the #404 workaround)#407
youknowone merged 1 commit into
mainfrom
wasm-callmayforce-bridge-decline

Conversation

@youknowone

@youknowone youknowone commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

Real fix for the wasm float_div_zero_caught_loop failure that #404 worked around by gating the #366 non-branch direct-pc carry off on wasm.

Root cause. Under the m366 carry, the checked-float-truediv deopt (float_eq(rhs,0.0) -> guard_false, every 5th iteration) gets a guard side-trace (bridge) compiled for it on wasm only. That bridge does NewWithVtable + SetfieldGc + CallMayForceR(...1.5-float-const..., v80, 5) — it re-enters the plain interpreter through a may-force residual call. On wasm the reconstructed interpreter value stack has the 1.5 dividend as NULL, so the re-entered interpreter runs truediv(NULL, int)TypeError: unsupported operand type(s) for /: '' and 'int'. Native never compiles this bridge — it blackholes the deopt and prints 37500.0.

The earlier "wasm liveness twin ≠ raw -live- decode" (#404) and a later "wasm32 32-bit object-layout corruption" hypothesis were both wrong; the latter was a stale-wasm-module measurement artifact. The blackhole path runs byte-identically on wasm and native — the divergence is native-blackhole vs wasm-bridge.

Fix

  1. majit-backend-wasm compile_bridge — decline any bridge whose ops contain a may-force call, so the deopt routes back through the blackhole interpreter, matching native.
  2. jitcode_dispatch.rs — with the deopt handled, the m366 carry resumes correctly on wasm, so the wasm-only gate in m366_nonbranch_pc_enabled (added in jit: disable the #366 non-branch direct-pc carry on wasm #404) and its stale doc-comment are removed. m366 is now ON on all backends (parity).

dynasm/cranelift codegen is unaffected by construction: the lib.rs change is wasm-backend-only, and the removed jitcode gate branch was never taken on native.

Verification

  • wasm synth/float_div_zero_caught_loop37500.0
  • wasm synthetic suite 173/173
  • dynasm synthetic suite 173/173

Summary by CodeRabbit

  • Bug Fixes

    • Improved bridge handling to avoid a runtime path that could surface division-related interpreter errors, matching native behavior more closely.
    • Fixed feature flag behavior so the non-branch PC setting now respects the environment variable consistently across platforms.
  • Documentation

    • Updated feature notes to clarify the default behavior and how to disable it.

…gate

A wasm-compiled guard side-trace containing a may-force residual call
reconstructs the interpreter value stack from its inputargs before the
call; a float-const dividend materialises as NULL there, so the
re-entered interpreter raises `unsupported operand type(s) for /`.
Native blackholes that deopt instead. Decline such bridges in the wasm
backend so the deopt blackholes, matching native.

With the deopt handled this way, the #366 non-branch pc carry resumes
correctly on wasm, so remove the wasm-only gate in
`m366_nonbranch_pc_enabled` (added in #404) and its stale doc-comment.
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 440ec057-5b12-42d6-b296-70a37d717532

📥 Commits

Reviewing files that changed from the base of the PR and between 4bf9ac9 and bcdd1be.

📒 Files selected for processing (2)
  • majit/majit-backend-wasm/src/lib.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs

Walkthrough

The wasm JIT backend now declines compiling bridges containing a CallMayForce opcode, returning BackendError::Unsupported so execution routes through the blackhole interpreter instead. Separately, m366_nonbranch_pc_enabled() removes its wasm32-specific hard-disable and instead derives its cached state from the PYRE_M366_NONBRANCH_PC environment variable.

Changes

Wasm Bridge CallMayForce Guard

Layer / File(s) Summary
Decline bridges with CallMayForce
majit/majit-backend-wasm/src/lib.rs
compile_bridge scans ops for CallMayForce and returns BackendError::Unsupported with a specific reason if found, avoiding a spurious interpreter re-entry path.

m366 Nonbranch PC Flag Fix

Layer / File(s) Summary
Env-driven flag computation
pyre/pyre-jit-trace/src/jitcode_dispatch.rs
m366_nonbranch_pc_enabled() no longer hard-disables on wasm32; it now caches its value via OnceLock derived from PYRE_M366_NONBRANCH_PC, with updated documentation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • youknowone/pyre#347: Both PRs modify compile_bridge in majit/majit-backend-wasm/src/lib.rs, overlapping in the same bridge compilation control logic.

Poem

A bridge that forces calls to spin,
the wasm hare says "not letting you in!"
Unsupported, deopt, off you go,
to the blackhole interpreter's slow glow. 🐇
And m366's flag, freed from wasm's cage,
now reads the env at any stage. ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wasm-callmayforce-bridge-decline

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@youknowone youknowone marked this pull request as ready for review July 8, 2026 03:46
@youknowone youknowone merged commit 03a1b1f into main Jul 8, 2026
30 of 31 checks passed
@youknowone youknowone deleted the wasm-callmayforce-bridge-decline branch July 8, 2026 03:47
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit cd24acc).

1. Regressions to PyPy parity introduced by this patch

  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs:7259 ↔ rpython/jit/metainterp/pyjitpl.py:177: current m366_nonbranch_pc_enabled() no longer disables wasm, so wasm now reaches the direct-jitcode_pc non-branch snapshot path at pyre/pyre-jit-trace/src/jitcode_dispatch.rs:10255. PyPy encodes active boxes from the frame’s current pc via get_list_of_active_boxes(...); Pyre’s wasm blackhole resolver in majit/majit-metainterp/src/jitdriver.rs:2717 ignores _carried_jitcode_pc, so encoder/decoder coordinates can diverge. This regresses the upstream/main wasm opt-out that avoided the mismatch.

  • majit/majit-backend-wasm/src/lib.rs:1391 ↔ rpython/jit/backend/x86/regalloc.py:913: the patch rejects every wasm bridge containing CallMayForce* with Unsupported, while PyPy lowers CALL_MAY_FORCE in bridges via _consider_call(op, guard_not_forced=True) and x86 emits it at rpython/jit/backend/x86/assembler.py:2239. This is a new backend-level decline that upstream/main did not have.

2. Other mismatches introduced by this patch

None.

3. Pre-existing mismatches (already present before this patch)

  • majit/majit-metainterp/src/jitdriver.rs:2717 ↔ rpython/jit/metainterp/resume.py:1338: the resume resolver accepts (jitcode_index, pc, _carried_jitcode_pc) but discards _carried_jitcode_pc, then calls ResolvedJitCode::new(..., pc as usize). PyPy’s blackhole_from_resumedata stores exactly the executable pc it later passes to setposition; Pyre’s snapshot format can carry a separate direct JitCode coordinate, so ignoring it is a pre-existing asymmetry exposed by this patch.

4. Structural adaptations

  • majit/majit-ir/src/resumedata.rs:528 ↔ rpython/jit/metainterp/resume.py:249: Pyre’s resume frame header stores a third jitcode_pc word after (jitcode_index, pc), while PyPy appends only jitcode_index and pc. This is a structural adaptation for Pyre’s Python-PC-to-JitCode-PC mapping and is acceptable only if all readers use it symmetrically.

@youknowone

Copy link
Copy Markdown
Owner Author

⚠️ Perf regression found — do not merge as-is

A/B/C benchmark of the wasm synthetic suite (WASM column total, per-bench noisy but big ratios reproduce across two independent builds):

config bool_arithmetic is_op string_ops suite total
A = origin/main #404 (m366 OFF, no decline) 2.73s 0.20s 1.25s 125.25s
B = this PR (m366 ON + decline) 15.84s 1.81s 3.72s 139.19s (+11%)
C = m366 OFF + decline 16.80s 2.06s 3.81s 144.72s

Since C ≈ B and both ≫ A, the regression is caused by the bridge decline, not m366 (m366 ON/OFF on wasm is noise-level). bool_arithmetic/is_op/string_ops compile legitimate, fast CallMayForce bridges on wasm; declining all of them kills those speedups. Only the float-truediv deopt bridge is actually broken.

So #404's targeted m366-off was correct and had zero perf cost, while this blanket decline is a net regression.

Real root cause / proper fix direction: the wasm bridge inputarg/resume path materialises the float const 1.5 as NULL. Lead: emit_resolve bakes constants.get(&opref.raw()).unwrap_or(0) — a float ConstPtr missing from the bridge resume constants table falls back to 0 (NULL). Fixing float-const carriage in the bridge resume data would keep all bridges enabled (perf preserved) and correct — no decline, no m366 gate needed.

Holding this draft pending a decision: (1) real codegen fix [recommended], (2) close and keep #404, or (3) narrow the decline to only the float-const-rebox shape.

youknowone added a commit that referenced this pull request Jul 8, 2026
…on) (#416)

#407 declined every CallMayForce bridge on wasm, which also dropped the
non-raising loop-closing bridges wasm relies on for speed (bool_arithmetic
~6x slower, ~+11% on the synthetic suite).

Only exception-resume bridges are actually broken: a bridge containing
`GuardException` resumes into the exception handler by re-entering the
interpreter at the raising bytecode, reading the pre-call operand stack.
Constant stack entries (e.g. a float dividend) live only in the guard's
`rd_consts` resume data, not in a spilled frame slot, and the compiled
bridge — reconstructing state from inputargs alone — cannot see
`rd_consts`, so the constant materialises as NULL and the re-entered
interpreter runs e.g. `truediv(NULL, int)`.

Narrow the decline from `is_call_may_force()` to `GuardException`: the
broken shape blackholes (rematerialising constants from `rd_consts`, the
native path) while `GuardNoException` loop-closing bridges keep their
compiled fast path. wasm synthetic suite 173/173 (float_div correct), no
CallMayForce-bridge regression, and faster overall than both #407 and the
#404 workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant