jit: exception-edge bridge default-on, GC write-barrier fix, and FBW rollout-flag cleanup

majit/majit-backend-cranelift/src/compiler.rs

@@ -6751,11 +6751,19 @@ fn emit_guard_exit(
         }
     }
 
-    if info.can_have_bridge && !info.must_save_exception {
+    if info.can_have_bridge {
         // RPython/dynasm patched guards enter the bridge before failure
-        // recovery. Cranelift still has to publish failargs into jf_frame
-        // because bridge input locations are frame based, but a bridge hit
-        // should skip the deadframe-only gcmap/write-barrier/descr stores.
+        // recovery (`patch_jump_for_descr` rewrites the guard branch, so the
+        // failure-recovery stub never runs on a bridge hit).  Cranelift still
+        // has to publish failargs into jf_frame because bridge input locations
+        // are frame based, but a bridge hit should skip the deadframe-only
+        // gcmap/write-barrier/descr stores.  must_save_exception guards
+        // dispatch here too — BEFORE the exception staging below — so the
+        // pending-exception cells flow into the bridge intact and its entry
+        // flavor guard (GUARD_NO_EXCEPTION / GUARD_EXCEPTION,
+        // `prepare_resume_from_failure`) can check them; staging first would
+        // consume the exception and let the wrong-flavor entry run the
+        // recorded continuation on a NULL raised-call result.
         emit_attached_bridge_dispatch(
             builder,
             jf_ptr,
@@ -6805,22 +6813,6 @@ fn emit_guard_exit(
             .ins()
             .store(MemFlags::trusted(), zero, exc_type_addr, 0);
     }
-    if info.can_have_bridge && info.must_save_exception {
-        // Exception guards need the exception payload saved before a bridge
-        // sees the frame.  Do the bridge check before the deadframe-only
-        // write-barrier/descr stores: dynasm patches exception guards to jump
-        // to the bridge after staging jf_guard_exc, and the bridge prologue
-        // immediately re-roots the same frame on the shadow stack.  A bridge
-        // hit therefore does not need the host-call write barrier that only
-        // protects the returned deadframe path.
-        emit_attached_bridge_dispatch(
-            builder,
-            jf_ptr,
-            info.bridge_cache_addrs
-                .expect("can_have_bridge=true GuardInfo must carry bridge_cache_addrs"),
-            ptr_type,
-        );
-    }
     if info.gcmap != 0 || info.must_save_exception {
         // aarch64/assembler.py:967-980 `_reload_frame_if_necessary`:
         // RPython emits a JITFrame write-barrier after any potentially-

majit/majit-backend-dynasm/src/aarch64/assembler.rs

@@ -3388,8 +3388,11 @@ impl<'a> AssemblerARM64<'a> {
                     dynasm!(self.mc ; .arch aarch64 ; mov X(r.value), x29);
                 }
             }
-            OpCode::SaveException => self.genop_save_exception(op),
-            OpCode::SaveExcClass => self.genop_save_exc_class(op),
+            // assembler.py:1820-1821 genop_save_exception IS
+            // `_store_and_reset_exception(resloc)` — reuse the shared helper.
+            OpCode::SaveException => self.emit_store_and_reset_exception(result_loc),
+            OpCode::SaveExcClass => self.genop_save_exc_class(result_loc),
+            OpCode::RestoreException => self.genop_restore_exception(arglocs),
             // Guards never reach the non-guard regalloc dispatch — they
             // are emitted exclusively from `regalloc_perform_guard` via
             // the `RegAllocOp::PerformWithGuard` arm.
@@ -6468,20 +6471,46 @@ impl<'a> AssemblerARM64<'a> {
     // assembler.py:1817 genop_save_exc_class / genop_save_exception
     // ================================================================
 
-    /// assembler.py:1817 genop_save_exc_class — stub: returns 0.
-    fn genop_save_exc_class(&mut self, op: &Op) {
-        dynasm!(self.mc ; .arch aarch64 ; mov x0, 0);
-        if !op.pos.get().is_none() {
-            self.store_rax_to_result(op.pos.get());
+    /// assembler.py:1817-1818 genop_save_exc_class:
+    /// `MOV resloc, [pos_exception]`.  The regalloc always assigns the
+    /// result a register (`consider_no_arg_result`).
+    fn genop_save_exc_class(&mut self, result_loc: Option<&Loc>) {
+        self.emit_mov_imm64(16, crate::jit_exc_type_addr() as i64);
+        match result_loc {
+            Some(Loc::Reg(dst)) => {
+                dynasm!(self.mc ; .arch aarch64 ; ldr X(dst.value), [x16]);
+            }
+            Some(Loc::Frame(frame)) => {
+                dynasm!(self.mc ; .arch aarch64 ; ldr x17, [x16]);
+                self.emit_str_fp(17, frame.ebp_loc.value);
+            }
+            _ => {}
         }
     }
 
-    /// assembler.py:1827 genop_save_exception — stub: returns 0.
-    fn genop_save_exception(&mut self, op: &Op) {
-        dynasm!(self.mc ; .arch aarch64 ; mov x0, 0);
-        if !op.pos.get().is_none() {
-            self.store_rax_to_result(op.pos.get());
+    /// assembler.py:1845-1850 `_restore_exception`:
+    /// `MOV [pos_exc_value], excvalloc; MOV [pos_exception], exctploc`.
+    /// arglocs = [class, value]; the regalloc brings both into registers
+    /// (`consider_restore_exception`).  x16/x17 are the scratch pair.
+    fn genop_restore_exception(&mut self, arglocs: &[Loc]) {
+        if arglocs.len() < 2 {
+            return;
         }
+        let load_to_x17 = |this: &mut Self, loc: &Loc| match loc {
+            Loc::Reg(src) => {
+                let src = src.value;
+                dynasm!(this.mc ; .arch aarch64 ; mov x17, X(src));
+            }
+            Loc::Frame(frame) => this.emit_ldr_fp(17, frame.ebp_loc.value),
+            Loc::Immed(imm) => this.emit_mov_imm64(17, imm.value),
+            _ => {}
+        };
+        load_to_x17(self, &arglocs[1]); // value
+        self.emit_mov_imm64(16, crate::jit_exc_value_addr() as i64);
+        dynasm!(self.mc ; .arch aarch64 ; str x17, [x16]);
+        load_to_x17(self, &arglocs[0]); // class
+        self.emit_mov_imm64(16, crate::jit_exc_type_addr() as i64);
+        dynasm!(self.mc ; .arch aarch64 ; str x17, [x16]);
     }
 
     // ================================================================

majit/majit-backend-dynasm/src/x86/assembler.rs

@@ -4542,8 +4542,11 @@ impl<'a> Assembler386<'a> {
                     dynasm!(self.mc ; .arch x64 ; mov Rq(r.value), rbp);
                 }
             }
-            OpCode::SaveException => self.genop_save_exception(op),
-            OpCode::SaveExcClass => self.genop_save_exc_class(op),
+            // assembler.py:1820-1821 genop_save_exception IS
+            // `_store_and_reset_exception(resloc)` — reuse the shared helper.
+            OpCode::SaveException => self.emit_store_and_reset_exception(result_loc),
+            OpCode::SaveExcClass => self.genop_save_exc_class(result_loc),
+            OpCode::RestoreException => self.genop_restore_exception(arglocs),
             // Guards never reach the non-guard regalloc dispatch — they
             // are emitted exclusively from `regalloc_perform_guard` via
             // the `RegAllocOp::PerformWithGuard` arm
@@ -8432,20 +8435,66 @@ impl<'a> Assembler386<'a> {
     // assembler.py:1817 genop_save_exc_class / genop_save_exception
     // ================================================================
 
-    /// assembler.py:1817 genop_save_exc_class — stub: returns 0.
-    fn genop_save_exc_class(&mut self, op: &Op) {
-        dynasm!(self.mc ; .arch x64 ; xor eax, eax);
-        if !op.pos.get().is_none() {
-            self.store_rax_to_result(op.pos.get());
+    /// assembler.py:1817-1818 genop_save_exc_class:
+    /// `MOV resloc, [pos_exception]`.  The regalloc always assigns the
+    /// result a register (`consider_no_arg_result`).
+    fn genop_save_exc_class(&mut self, result_loc: Option<&Loc>) {
+        let scratch = crate::regloc::X86_64_SCRATCH_REG.value;
+        let exc_type_addr = crate::jit_exc_type_addr() as i64;
+        dynasm!(self.mc ; .arch x64 ; mov Rq(scratch), QWORD exc_type_addr);
+        match result_loc {
+            Some(Loc::Reg(dst)) => {
+                dynasm!(self.mc ; .arch x64 ; mov Rq(dst.value), [Rq(scratch)]);
+            }
+            Some(Loc::Frame(frame)) => {
+                let ofs = frame.ebp_loc.value;
+                dynasm!(self.mc ; .arch x64
+                    ; mov Rq(scratch), [Rq(scratch)]
+                    ; mov [rbp + ofs], Rq(scratch)
+                );
+            }
+            _ => {}
         }
     }
 
-    /// assembler.py:1827 genop_save_exception — stub: returns 0.
-    fn genop_save_exception(&mut self, op: &Op) {
-        dynasm!(self.mc ; .arch x64 ; xor eax, eax);
-        if !op.pos.get().is_none() {
-            self.store_rax_to_result(op.pos.get());
+    /// assembler.py:1845-1850 `_restore_exception`:
+    /// `MOV [pos_exc_value], excvalloc; MOV [pos_exception], exctploc`.
+    /// arglocs = [class, value]; the regalloc brings both into registers
+    /// (`consider_restore_exception`), so only the Reg arm is hot — the
+    /// non-Reg fallback round-trips through rax with a push/pop save.
+    fn genop_restore_exception(&mut self, arglocs: &[Loc]) {
+        if arglocs.len() < 2 {
+            return;
         }
+        let scratch = crate::regloc::X86_64_SCRATCH_REG.value;
+        let mut store_loc_to = |this: &mut Self, cell_addr: i64, loc: &Loc| {
+            dynasm!(this.mc ; .arch x64 ; mov Rq(scratch), QWORD cell_addr);
+            match loc {
+                Loc::Reg(src) => {
+                    dynasm!(this.mc ; .arch x64 ; mov [Rq(scratch)], Rq(src.value));
+                }
+                Loc::Frame(frame) => {
+                    let ofs = frame.ebp_loc.value;
+                    dynasm!(this.mc ; .arch x64
+                        ; push rax
+                        ; mov rax, [rbp + ofs]
+                        ; mov [Rq(scratch)], rax
+                        ; pop rax
+                    );
+                }
+                Loc::Immed(imm) => {
+                    dynasm!(this.mc ; .arch x64
+                        ; push rax
+                        ; mov rax, QWORD imm.value
+                        ; mov [Rq(scratch)], rax
+                        ; pop rax
+                    );
+                }
+                _ => {}
+            }
+        };
+        store_loc_to(self, crate::jit_exc_value_addr() as i64, &arglocs[1]);
+        store_loc_to(self, crate::jit_exc_type_addr() as i64, &arglocs[0]);
     }
 
     // ================================================================

majit/majit-gc/src/rewrite.rs

@@ -2869,6 +2869,15 @@ impl GcRewriterImpl {
     /// rewrite.py:988-1001 remove_bridge_exception: check a common
     /// case where SaveExcClass + SaveException + RestoreException
     /// appear at the start of a bridge and are unused. Strip them.
+    ///
+    /// rewrite.py:991 leaves an `XXX should check if the boxes are used
+    /// later; but we just assume they aren't for now`.  A routed
+    /// exception-guard handler bridge breaks that assumption: it records the
+    /// same prefix but keeps the SaveException result as `last_exc_value`
+    /// for handler code (`except E as e`), so stripping the prefix would
+    /// leave a dangling reference.  Do the deferred use-check — the
+    /// RESTORE_EXCEPTION carries the canonical class/value operands, so only
+    /// strip when no op after the prefix reuses either.
     fn remove_bridge_exception(ops: &[Op]) -> Vec<Op> {
         let mut start = 0;
         if ops
@@ -2882,10 +2891,24 @@ impl GcRewriterImpl {
             && ops[start + 1].opcode == OpCode::SaveException
             && ops[start + 2].opcode == OpCode::RestoreException
         {
-            let mut result = Vec::with_capacity(ops.len() - 3);
-            result.extend_from_slice(&ops[..start]);
-            result.extend_from_slice(&ops[start + 3..]);
-            return result;
+            let restore = &ops[start + 2];
+            let class_ref = restore.arg(0);
+            let value_ref = restore.arg(1);
+            let used_later = ops[start + 3..].iter().any(|op| {
+                (0..op.num_args()).any(|k| {
+                    let a = op.arg(k);
+                    a.same_box(&class_ref) || a.same_box(&value_ref)
+                }) || op.getfailargs().is_some_and(|fa| {
+                    fa.iter()
+                        .any(|a| a.same_box(&class_ref) || a.same_box(&value_ref))
+                })
+            });
+            if !used_later {
+                let mut result = Vec::with_capacity(ops.len() - 3);
+                result.extend_from_slice(&ops[..start]);
+                result.extend_from_slice(&ops[start + 3..]);
+                return result;
+            }
         }
         ops.to_vec()
     }
@@ -5487,4 +5510,55 @@ mod tests {
             "JIT_DEBUG must not route its ConstPtr through the gc_table"
         );
     }
+
+    #[test]
+    fn remove_bridge_exception_strips_unused_prefix() {
+        use std::rc::Rc;
+        let class = Rc::new(Op::new(OpCode::SaveExcClass, &[]));
+        let value = Rc::new(Op::new(OpCode::SaveException, &[]));
+        let restore = Op::new(
+            OpCode::RestoreException,
+            &[
+                Operand::from_bound_op(&class),
+                Operand::from_bound_op(&value),
+            ],
+        );
+        let ops = vec![
+            Op::new(OpCode::SaveExcClass, &[]),
+            Op::new(OpCode::SaveException, &[]),
+            restore,
+            Op::new(OpCode::Finish, &[]),
+        ];
+        let out = GcRewriterImpl::remove_bridge_exception(&ops);
+        // No op reuses the saved class/value, so the prefix is stripped.
+        assert_eq!(out.len(), 1);
+        assert_eq!(out[0].opcode, OpCode::Finish);
+    }
+
+    #[test]
+    fn remove_bridge_exception_keeps_prefix_when_value_reused() {
+        use std::rc::Rc;
+        let class = Rc::new(Op::new(OpCode::SaveExcClass, &[]));
+        let value = Rc::new(Op::new(OpCode::SaveException, &[]));
+        let restore = Op::new(
+            OpCode::RestoreException,
+            &[
+                Operand::from_bound_op(&class),
+                Operand::from_bound_op(&value),
+            ],
+        );
+        // A routed handler bridge keeps the saved value as `last_exc_value`
+        // for `except E as e`, so a later op still references it.
+        let consumer = Op::new(OpCode::SameAsR, &[Operand::from_bound_op(&value)]);
+        let ops = vec![
+            Op::new(OpCode::SaveExcClass, &[]),
+            Op::new(OpCode::SaveException, &[]),
+            restore,
+            consumer,
+        ];
+        let out = GcRewriterImpl::remove_bridge_exception(&ops);
+        // The reused value pins the whole prefix in place.
+        assert_eq!(out.len(), 4);
+        assert_eq!(out[0].opcode, OpCode::SaveExcClass);
+    }
 }

majit/majit-ir/src/effectinfo.rs

@@ -603,21 +603,21 @@ pub enum PyreHelperKind {
     /// without `from` (checked, never dereferenced when null), so a concrete
     /// NULL there is the normal shape — not the broken baked-NULL shape the
     /// walker's may-force NULL-ref gate rejects.  The full-body walker
-    /// recognises this tag (gated `PYRE_FBW_RAISE`) to exempt the trailing
+    /// recognises this tag to exempt the trailing
     /// NULL `cause` in both twin NULL-ref guards so the FBW path can own the
     /// raise instead of declining to the trait.
     RaiseVarargs,
     /// `get_current_exception()` — the PUSH_EXC_INFO `prev = ec.sys_exc_value`
     /// save residual (`() → Ref`, TLS read via `cpu.get_current_exception_fn`).
-    /// The full-body walker recognises this tag (gated `PYRE_FBW_RAISE`) to
+    /// The full-body walker recognises this tag to
     /// lower it to `GETFIELD_GC_R(ec, sys_exc_value)` so the exc-info save
     /// participates in the balanced save/restore the heap optimizer
     /// dead-store-eliminates, letting a non-escaping exception stay virtual.
     GetCurrentException,
     /// `set_current_exception(exc)` — the PUSH_EXC_INFO store and the
     /// POP_EXCEPT restore residual (`(exc: Ref) → Void`, TLS write via
     /// `cpu.set_current_exception_fn`).  The full-body walker recognises this
-    /// tag (gated `PYRE_FBW_RAISE`) to lower it to `SETFIELD_GC(ec, exc,
+    /// tag to lower it to `SETFIELD_GC(ec, exc,
     /// sys_exc_value)`; paired with the [`GetCurrentException`] save on the
     /// same descr-identity field, a balanced never-read save/restore is
     /// dead-store-eliminated so the virtual exception de-escapes and DCEs.

majit/majit-metainterp/src/virtualizable.rs

@@ -1064,6 +1064,14 @@ impl VirtualizableInfo {
                 Type::Ref => {
                     let ptr = obj_ptr.add(field.offset) as *mut usize;
                     *ptr = value as usize;
+                    // The ref may be nursery-young while the virtualizable is
+                    // old-gen and runs detached from the walked frame chain;
+                    // upstream's write_boxes stores run under the translated
+                    // write barrier (virtualizable.py:101-113), so arm the
+                    // object in the remembered set here.
+                    if majit_gc::gc_owns_object(obj_ptr as usize) {
+                        majit_gc::gc_write_barrier(majit_ir::GcRef(obj_ptr as usize));
+                    }
                 }
                 // Word-sized `Signed` field (`isize`/`usize`): 4 bytes on
                 // wasm32. Writing 8 bytes would clobber the adjacent field.
@@ -1168,6 +1176,18 @@ impl VirtualizableInfo {
                 Type::Ref => {
                     let ptr = array_ptr.add(item_offset) as *mut usize;
                     *ptr = value as usize;
+                    // The ref may be nursery-young while the array/frame are
+                    // old-gen and run detached from the walked frame chain
+                    // (virtualizable.py:101-113 write_boxes runs under the
+                    // translated write barrier). Arm whichever side the GC
+                    // owns: a GC array re-traces its own items; a stationary
+                    // block is re-walked through the owning frame's custom
+                    // trace.
+                    if majit_gc::gc_owns_object(array_ptr as usize) {
+                        majit_gc::gc_write_barrier(majit_ir::GcRef(array_ptr as usize));
+                    } else if majit_gc::gc_owns_object(obj_ptr as usize) {
+                        majit_gc::gc_write_barrier(majit_ir::GcRef(obj_ptr as usize));
+                    }
                 }
                 _ => {
                     let ptr = array_ptr.add(item_offset) as *mut i64;

pyre/bench/fannkuch.cranelift.jitstats

@@ -1,5 +1,5 @@
 bridges_compiled=40
-guard_failures=8202
+guard_failures=8203
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=5
+loops_compiled=6

pyre/bench/fannkuch.dynasm.jitstats

@@ -1,5 +1,5 @@
 bridges_compiled=40
-guard_failures=8202
+guard_failures=8203
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=5
+loops_compiled=6

pyre/bench/fib_loop.cranelift.jitstats

@@ -1,5 +1,5 @@
 bridges_compiled=0
-guard_failures=189
+guard_failures=190
 internal_compile_panics=0
 loops_aborted=0
-loops_compiled=1
+loops_compiled=2