From 37cebf6519ba03485d7537a7534e185fd19d89c7 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 5 Aug 2026 07:48:46 +0900 Subject: [PATCH 1/2] jit: pointer-width exception-class reads, the wasm exception triple, and the exception-edge bridge on every backend `PyObject` is `{ ob_type, w_class }`, two machine words. Four sites read the one-word `typeptr` at offset 0 through `*const i64`, so on a 32-bit target the value carried the adjacent `w_class` in its high half and could never compare equal to the pending-exception cell, which `jit_exc_raise` publishes at pointer width. All four now read at pointer width; the reads are byte-identical on 64-bit targets. The site that reaches generated code is `bridge_subwalk::dispatch_via_miframe`, whose `exc_edge_class` becomes the bridge-entry GUARD_EXCEPTION constant. On wasm that guard executed 218889 times and passed 0 times, measured by baking a `BRIDGE_DIAG` slot address into the emitted guard and storing both compare operands: runtime `JIT_EXC_TYPE` 0x0000_0000_0083_4658 against a baked 0x00E0_EA40_0083_4658. The three `pyjitpl.rs` sites are the same read; the third fires once per raising iteration. The wasm backend no-opped SAVE_EXCEPTION / SAVE_EXC_CLASS / RESTORE_EXCEPTION. SAVE_EXCEPTION produces a value, and the explicit arm bypassed the value-producing decline in the `_` fallback, so the local stayed null and the resumed handler compared against it. Lowered all three against `x86/assembler.py` genop_save_exc_class, genop_save_exception and _restore_exception. With both fixed, `exc_edge_bridge_enabled()` returns `true` for every backend instead of `cfg!(not(target_arch = "wasm32"))`, and the wasm guard-failure counts land on the native ones: fixture wasm before -> after dynasm type_name_surrogate_reject 9464 -> 202 201 exc_caught_in_callee_return_loop 23748 -> 613 611 inline_subwalk_property_mutates 23748 -> 613 611 inline_subwalk_mutating_residual 23950 -> 815 813 named_reraise_sibling_hot 2649 -> 1712 1712 sre_pattern_methods 2536 -> 1671 1670 handler_reraise_second_exc 1285 -> 804 804 `loops_aborted` goes to 0 on all seven; `type_name_surrogate_reject` also drops `jit_calls` 103624 -> 2248 and `compile_ms` 76 -> 7.8. The seven `.wasm.jitstats` baselines are re-recorded. `BRIDGE_DIAG` grows to 30 slots: `cell_set` / `cell_missing` / `cell_rebridge` separate "the backend accepted a bridge" from "the source guard's dispatch cell can reach it", which `BRIDGE_OK` alone does not. The `if !exc_edge_bridge_enabled()` legacy prologue branch in `call_jit.rs` is now unreachable and is left in place. Assisted-by: Claude --- majit/majit-backend-wasm/src/codegen.rs | 42 ++++++++++++++++++- majit/majit-backend-wasm/src/lib.rs | 29 ++++++++++++- majit/majit-metainterp/src/pyjitpl.rs | 13 ++++-- ...caught_in_callee_return_loop.wasm.jitstats | 6 +-- .../handler_reraise_second_exc.wasm.jitstats | 6 +-- ...ne_subwalk_mutating_residual.wasm.jitstats | 6 +-- ...ine_subwalk_property_mutates.wasm.jitstats | 6 +-- .../named_reraise_sibling_hot.wasm.jitstats | 6 +-- .../synth/sre_pattern_methods.wasm.jitstats | 6 +-- .../type_name_surrogate_reject.wasm.jitstats | 6 +-- .../src/jitcode_dispatch/bridge_subwalk.rs | 6 ++- .../src/jitcode_dispatch/mod.rs | 23 ++++++++-- pyre/pyre-wasm-runner/src/main.rs | 3 ++ 13 files changed, 125 insertions(+), 33 deletions(-) diff --git a/majit/majit-backend-wasm/src/codegen.rs b/majit/majit-backend-wasm/src/codegen.rs index 16b73049a64..f24da1c6173 100644 --- a/majit/majit-backend-wasm/src/codegen.rs +++ b/majit/majit-backend-wasm/src/codegen.rs @@ -3068,8 +3068,46 @@ fn build_function( } // ── Exception handling ── - OpCode::SaveException | OpCode::SaveExcClass | OpCode::RestoreException => { - // No-op in wasm MVP — exception state is managed by the host. + OpCode::SaveException => { + // x86/assembler.py:1820-1821 genop_save_exception: + // _store_and_reset_exception → resloc = [pos_exc_value]; + // [pos_exception] = 0; [pos_exc_value] = 0. + // The result is the caught exception the resumed handler reads, + // so it must be written even though the slots themselves are + // shared with the host: skipping the op leaves the local null. + let vi = op.pos.get().raw(); + if !OpRef::raw_is_constant(vi) { + sink.i32_const(crate::jit_exc_value_addr() as i32); + sink.i64_load(mem64(0)); + sink.local_set(1 + vi); + } + sink.i32_const(crate::jit_exc_type_addr() as i32); + sink.i64_const(0); + sink.i64_store(mem64(0)); + sink.i32_const(crate::jit_exc_value_addr() as i32); + sink.i64_const(0); + sink.i64_store(mem64(0)); + } + OpCode::SaveExcClass => { + // x86/assembler.py:1817-1818 genop_save_exc_class: + // MOV resloc, [pos_exception] + let vi = op.pos.get().raw(); + if !OpRef::raw_is_constant(vi) { + sink.i32_const(crate::jit_exc_type_addr() as i32); + sink.i64_load(mem64(0)); + sink.local_set(1 + vi); + } + } + OpCode::RestoreException => { + // x86/assembler.py:1845-1850 _restore_exception: + // MOV [pos_exc_value], excvalloc + // MOV [pos_exception], exctploc + sink.i32_const(crate::jit_exc_value_addr() as i32); + emit_resolve(&mut sink, constants, value_types, op.arg(1).to_opref()); + sink.i64_store(mem64(0)); + sink.i32_const(crate::jit_exc_type_addr() as i32); + emit_resolve(&mut sink, constants, value_types, op.arg(0).to_opref()); + sink.i64_store(mem64(0)); } // ── Conditional calls ── diff --git a/majit/majit-backend-wasm/src/lib.rs b/majit/majit-backend-wasm/src/lib.rs index 9e58c350a6e..b108230dcb6 100644 --- a/majit/majit-backend-wasm/src/lib.rs +++ b/majit/majit-backend-wasm/src/lib.rs @@ -48,10 +48,25 @@ use std::sync::atomic::{AtomicI64, AtomicU64, Ordering}; /// `wasm_unsupported_trace_reason` (the #62 loop-callee CALL_ASSEMBLER gap), /// 26 = the wasm host rejected the emitted module (`func_handle == 0`). /// Indices 2 and 4 double as compile_loop's other two declines. -pub static BRIDGE_DIAG: [AtomicU64; 27] = { +/// +/// 27-28 ask 12-13's question of EVERY accepted bridge rather than only the +/// CALL_ASSEMBLER ones: 27 = the source guard's dispatch cell was written, so +/// the loop epilogue now tail-calls this bridge in-module; 28 = it was not, +/// because the owning trace reserved no cell array, so the guard keeps +/// round-tripping to the host and the bridge is compiled but unreachable. +/// `BRIDGE_OK` (5) only says the backend accepted a bridge, which is strictly +/// weaker than "the guard can reach it"; without this split the two are +/// indistinguishable from outside the guest. +/// +/// 29 = the cell written at 27 was ALREADY non-zero, i.e. this guard had a +/// reachable bridge and kept failing anyway. One of those is ordinary (a guard +/// re-bridged after its first bridge was outgrown); a count that tracks +/// `BRIDGE_OK` says the epilogue dispatch is not taking the cell at all and +/// every bridge after the first is dead weight. +pub static BRIDGE_DIAG: [AtomicU64; 30] = { const Z: AtomicU64 = AtomicU64::new(0); [ - Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, + Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, Z, ] }; @@ -2842,11 +2857,21 @@ impl majit_backend::Backend for WasmBackend { diag_bump(13); } } + // The same question for every accepted bridge (slots 27/28): a bridge + // whose source guard has no cell is compiled and then unreachable. + if source_cells_base != 0 && bridge_slot != 0 { + diag_bump(27); + } else { + diag_bump(28); + } #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] if source_cells_base != 0 && bridge_slot != 0 { // cells[source_fail_index] = bridge_slot — the loop epilogue now // tails into this bridge instead of returning to the host. let cell = (source_cells_base as usize + source_fail_index as usize * 4) as *mut u32; + if unsafe { core::ptr::read(cell) } != 0 { + diag_bump(29); // this guard already had a reachable bridge + } unsafe { core::ptr::write(cell, bridge_slot); } diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 8ef8225a748..d445605fdc3 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -9468,7 +9468,12 @@ impl MetaInterp { let exc_class = if result.exception_value.is_null() { 0 } else { - unsafe { *(result.exception_value.0 as *const i64) } + // `typeptr` is one machine word at offset 0, so read it at + // pointer width: an i64 read on a 32-bit target would pull the + // adjacent header word into the high half and the class value + // would never compare equal to a pointer-width one + // (`Cpu::cls_of_gcref`, `jit_exc_raise`). + unsafe { *(result.exception_value.0 as *const usize) as i64 } }; let exception = ExceptionState { exc_class, @@ -9638,7 +9643,8 @@ impl MetaInterp { let exc_class = if exc_value_ref.is_null() { 0 } else { - unsafe { *(exc_value_ref.0 as *const i64) } + // Pointer-width read — see the `result.exception_value` site. + unsafe { *(exc_value_ref.0 as *const usize) as i64 } }; let exception = ExceptionState { exc_class, @@ -9816,7 +9822,8 @@ impl MetaInterp { let exc_class = if exc_value_ref.is_null() { 0 } else { - unsafe { *(exc_value_ref.0 as *const i64) } + // Pointer-width read — see the `result.exception_value` site. + unsafe { *(exc_value_ref.0 as *const usize) as i64 } }; let exception = ExceptionState { exc_class, diff --git a/pyre/bench/synth/exc_caught_in_callee_return_loop.wasm.jitstats b/pyre/bench/synth/exc_caught_in_callee_return_loop.wasm.jitstats index ff5f5dcbfdb..2e09ab735c1 100644 --- a/pyre/bench/synth/exc_caught_in_callee_return_loop.wasm.jitstats +++ b/pyre/bench/synth/exc_caught_in_callee_return_loop.wasm.jitstats @@ -1,8 +1,8 @@ -bridges_compiled=2 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -guard_failures=23748 +guard_failures=613 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/handler_reraise_second_exc.wasm.jitstats b/pyre/bench/synth/handler_reraise_second_exc.wasm.jitstats index 72e050d9d85..fa8ddf54e8d 100644 --- a/pyre/bench/synth/handler_reraise_second_exc.wasm.jitstats +++ b/pyre/bench/synth/handler_reraise_second_exc.wasm.jitstats @@ -1,8 +1,8 @@ -bridges_compiled=2 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -guard_failures=1285 +guard_failures=804 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/inline_subwalk_mutating_residual.wasm.jitstats b/pyre/bench/synth/inline_subwalk_mutating_residual.wasm.jitstats index 916d9282c3c..febceef7a1e 100644 --- a/pyre/bench/synth/inline_subwalk_mutating_residual.wasm.jitstats +++ b/pyre/bench/synth/inline_subwalk_mutating_residual.wasm.jitstats @@ -1,8 +1,8 @@ -bridges_compiled=3 +bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -guard_failures=23950 +guard_failures=815 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/inline_subwalk_property_mutates.wasm.jitstats b/pyre/bench/synth/inline_subwalk_property_mutates.wasm.jitstats index ff5f5dcbfdb..2e09ab735c1 100644 --- a/pyre/bench/synth/inline_subwalk_property_mutates.wasm.jitstats +++ b/pyre/bench/synth/inline_subwalk_property_mutates.wasm.jitstats @@ -1,8 +1,8 @@ -bridges_compiled=2 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -guard_failures=23748 +guard_failures=613 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/named_reraise_sibling_hot.wasm.jitstats b/pyre/bench/synth/named_reraise_sibling_hot.wasm.jitstats index dc76392c8ea..6b129e7fe85 100644 --- a/pyre/bench/synth/named_reraise_sibling_hot.wasm.jitstats +++ b/pyre/bench/synth/named_reraise_sibling_hot.wasm.jitstats @@ -1,8 +1,8 @@ -bridges_compiled=4 +bridges_compiled=7 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -guard_failures=2649 +guard_failures=1712 internal_compile_panics=0 -loops_aborted=3 +loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/sre_pattern_methods.wasm.jitstats b/pyre/bench/synth/sre_pattern_methods.wasm.jitstats index e48d4140427..2088140402c 100644 --- a/pyre/bench/synth/sre_pattern_methods.wasm.jitstats +++ b/pyre/bench/synth/sre_pattern_methods.wasm.jitstats @@ -1,8 +1,8 @@ -bridges_compiled=7 +bridges_compiled=8 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -guard_failures=2536 +guard_failures=1671 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/type_name_surrogate_reject.wasm.jitstats b/pyre/bench/synth/type_name_surrogate_reject.wasm.jitstats index 7a2e59f9a03..1ddd2244f0c 100644 --- a/pyre/bench/synth/type_name_surrogate_reject.wasm.jitstats +++ b/pyre/bench/synth/type_name_surrogate_reject.wasm.jitstats @@ -1,8 +1,8 @@ -bridges_compiled=0 +bridges_compiled=1 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -guard_failures=9464 +guard_failures=202 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=2 diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs index 848856120f7..0c0b034b8f3 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs @@ -341,7 +341,11 @@ pub fn dispatch_via_miframe( // typeptr at offset 0 (`_store_exception` invariant): the expected class the // bridge-entry GUARD_EXCEPTION checks the restored pending exception against. let exc_edge_class = if exc_edge_catch_target.is_some() && !exc_edge_concrete.is_null() { - unsafe { *(exc_edge_concrete as *const i64) } + // One machine word, so read it at pointer width: an i64 read on a + // 32-bit target pulls the adjacent header word into the high half, and + // the guard then compares against a class value the pending-exception + // cell (`jit_exc_raise`, pointer-width) can never hold. + unsafe { *(exc_edge_concrete as *const usize) as i64 } } else { 0 }; diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index dc5a093ad31..f90e5bd7e19 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -3133,11 +3133,26 @@ pub(crate) fn try_catch_exception_at(code: &[u8], position: usize) -> Option 202 +/// guard failures against dynasm's 201, `inline_subwalk_property_mutates` +/// 23748 -> 613 against 611, `handler_reraise_second_exc` and +/// `named_reraise_sibling_hot` exactly on dynasm's 804 and 1712, and +/// `loops_aborted` 1 -> 0 on all of them. pub fn exc_edge_bridge_enabled() -> bool { - cfg!(not(target_arch = "wasm32")) + true } /// `PYRE_CARRIER_EXC_RESUME=1` enables the multi-frame (carrier) exception diff --git a/pyre/pyre-wasm-runner/src/main.rs b/pyre/pyre-wasm-runner/src/main.rs index e349ffd97a0..c45a15cb1dc 100644 --- a/pyre/pyre-wasm-runner/src/main.rs +++ b/pyre/pyre-wasm-runner/src/main.rs @@ -544,6 +544,9 @@ fn run(module_path: &PathBuf, source: &str, script: &Path) -> Result { "cl_ok", "cl_decl_unsupported", "cl_decl_host_reject", + "cell_set", + "cell_missing", + "cell_rebridge", ]; let mut parts = Vec::new(); for (i, lbl) in labels.iter().enumerate() { From 654a5e024cd201b0b3c679ed22399e8bb6a857a6 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 5 Aug 2026 08:14:22 +0900 Subject: [PATCH 2/2] jit: state where the rvmprof_code side effect belongs `try_catch_exception_at` is a lookahead predicate called from the inlining and resume-snapshot walkers; the frame-popping loop that decodes `rvmprof_code` and calls `cintf::jit_rvmprof_code` is `MetaInterp::finishframe_exception`. The comment claimed the dropped call matched a non-trace-recorded upstream `cintf` call, which reads as a divergence from `pyjitpl.py:2547`. Assisted-by: Claude --- pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index f90e5bd7e19..6d7e05fbb3a 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -3107,8 +3107,11 @@ fn finishframe_lookahead_at(code: &[u8], position: usize) -> FinishframeLookahea // assert arg1 == 1 // cintf.jit_rvmprof_code(arg1, arg2) // Walker surfaces the operand byte indices for the caller to - // decide whether to symbolically record (today: drop, mirroring - // RPython's non-record direct cintf call). + // decide whether to symbolically record. The `cintf` call itself + // belongs to the frame-popping loop, which is + // `MetaInterp::finishframe_exception` and does perform it; firing + // it from here would mark an enter/leave on a lookahead that pops + // nothing. let arg1_reg = code[next.pc + 1]; let arg2_reg = code[next.pc + 2]; return FinishframeLookahead::RvmprofCode { arg1_reg, arg2_reg }; @@ -3120,9 +3123,10 @@ fn finishframe_lookahead_at(code: &[u8], position: usize) -> FinishframeLookahea /// `try_catch_exception_at(...) -> Option` shape used by /// existing callers. Returns `Some(target)` only on the /// `CatchTarget` arm; `RvmprofCode` and `NoMatch` collapse to `None` -/// (both cases continue unwinding from the caller's POV — the -/// instrumentation side effect is dropped today, matching RPython's -/// non-trace-recorded `cintf` call). +/// (both cases continue unwinding from the caller's POV). This is a +/// lookahead predicate, not a frame-popping loop, so it carries no +/// `jit_rvmprof_code` side effect — `MetaInterp::finishframe_exception` +/// is the loop that decodes `rvmprof_code` and calls it. pub(crate) fn try_catch_exception_at(code: &[u8], position: usize) -> Option { match finishframe_lookahead_at(code, position) { FinishframeLookahead::CatchTarget(target) => Some(target),