jit: size vable scalar stores by the target word

majit/majit-metainterp/src/blackhole.rs

@@ -6056,10 +6056,15 @@ fn read_descr<'a>(bh: &'a BlackholeInterpreter, code: &[u8], pos: usize) -> (&'a
 /// RPython: fielddescr carries byte offset directly; pyre VableField.index
 /// needs vinfo.static_fields[index].offset resolution.
 ///
-/// Vable scalar word-size invariant: `field_size: 8`, `field_type: Ref`,
-/// `field_flag: Pointer`, `is_field_signed: false`.  Every vable scalar
-/// field in pyre is laid out as a single machine word, so the synthesized
-/// BhDescr can hard-code these.  The dynasm / cranelift `bh_getfield_gc_*`
+/// Vable scalar word-size invariant: `field_size: size_of::<usize>()`,
+/// `field_type: Ref`, `field_flag: Pointer`, `is_field_signed: false`.
+/// Every vable scalar field in pyre is laid out as a single machine word,
+/// so the synthesized BhDescr can derive these.  The width has to be the
+/// target's word, not a literal 8: the size-dispatching
+/// `Backend::bh_setfield_gc_i` picks its store width from it, and on a
+/// 32-bit target an 8-byte store at `valuestackdepth` runs past the field
+/// and clears the `last_instr` that follows it.  The dynasm / cranelift
+/// `bh_getfield_gc_*`
 /// overrides on this BhDescr therefore read i64 / GcRef / f64 at the
 /// resolved offset without consulting size/sign — equivalent to the
 /// llmodel.py:705 `read_int_at_mem(struct, ofs, 8, False)` call.
@@ -6098,7 +6103,7 @@ fn read_descr_vable_field(bh: &BlackholeInterpreter, code: &[u8], pos: usize) ->
         BhDescr::Field {
             offset,
             // Vable scalar word-size invariant — see fn doc-block.
-            field_size: 8,
+            field_size: std::mem::size_of::<usize>(),
             field_type: majit_ir::value::Type::Ref,
             field_flag: majit_ir::descr::ArrayFlag::Pointer,
             is_field_signed: false,
@@ -6144,7 +6149,11 @@ fn read_descr_vable_array(bh: &BlackholeInterpreter, code: &[u8], pos: usize) ->
     (
         BhDescr::Field {
             offset,
-            field_size: 8,
+            // The array field is a pointer, so its width is the target word
+            // like the scalars above.  Latent rather than live: the `_gc_r`
+            // accessors this descr reaches take `as_offset()` and store at
+            // pointer width without consulting the size.
+            field_size: std::mem::size_of::<usize>(),
             field_type: majit_ir::value::Type::Ref,
             field_flag: majit_ir::descr::ArrayFlag::Pointer,
             is_field_signed: false,

majit/majit-metainterp/src/optimizeopt/mod.rs

@@ -578,6 +578,12 @@ pub struct OptContext {
     /// Set by rewrite pass, executed by emit_operation after the guard
     /// is added to new_operations (matching RPython's callback pattern).
     pub(crate) pending_guard_class_postprocess: Option<PendingGuardClassPostprocess>,
+    /// virtualize.py:84-90 postprocess_FINISH queues the stashed
+    /// GUARD_NOT_FORCED_2 here so the outer optimizer can insert it at
+    /// `new_operations.len() - 1` with full `store_final_boxes_in_guard`
+    /// semantics — the pass that stashes it holds no Optimizer, and both the
+    /// finalization and the knowledge collection are Optimizer-side.
+    pub(crate) pending_finish_guard_postprocess: Option<Op>,
     /// rewrite.py:282: postprocess_GUARD_NONNULL → mark_last_guard.
     /// Deferred until emit adds the guard to new_operations.
     pub(crate) pending_mark_last_guard: Option<OpRef>,
@@ -1659,6 +1665,7 @@ impl OptContext {
             extra_operations_after: VecDeque::new(),
             pending_guard_class_postprocess: None,
             pending_mark_last_guard: None,
+            pending_finish_guard_postprocess: None,
             imported_short_pure_ops: Vec::new(),
             imported_virtual_args: None,
             imported_loop_invariant_results: Vec::new(),
@@ -2264,6 +2271,7 @@ impl OptContext {
             extra_operations_after: VecDeque::new(),
             pending_guard_class_postprocess: None,
             pending_mark_last_guard: None,
+            pending_finish_guard_postprocess: None,
             imported_short_pure_ops: Vec::new(),
             imported_virtual_args: None,
             imported_loop_invariant_results: Vec::new(),

majit/majit-metainterp/src/optimizeopt/optimizer.rs

@@ -4493,6 +4493,7 @@ impl Optimizer {
                     for &pp_idx in postprocess_passes.iter().rev() {
                         self.passes[pp_idx].propagate_postprocess(&op, ctx);
                     }
+                    self.drain_pending_finish_guard_postprocess(ctx);
                     return Ok(());
                 }
                 OptimizationResult::Replace(op) => {
@@ -4551,6 +4552,7 @@ impl Optimizer {
                     for &pp_idx in postprocess_passes.iter().rev() {
                         self.passes[pp_idx].propagate_postprocess(&current_op, ctx);
                     }
+                    self.drain_pending_finish_guard_postprocess(ctx);
                     return Ok(());
                 }
                 OptimizationResult::Remove => {
@@ -4588,6 +4590,10 @@ impl Optimizer {
         for &pp_idx in postprocess_passes.iter().rev() {
             self.passes[pp_idx].propagate_postprocess(&current_op, ctx);
         }
+        // The FINISH reaches its emit down this path — `optimize_FINISH`
+        // returns PassOn — so this is the drain that postprocess_FINISH
+        // actually goes through.
+        self.drain_pending_finish_guard_postprocess(ctx);
         Ok(())
     }
 
@@ -5205,6 +5211,45 @@ impl Optimizer {
         new_idx
     }
 
+    /// virtualize.py:84-90 postprocess_FINISH, Optimizer half.
+    ///
+    /// `OptVirtualize::propagate_postprocess` stashed the GUARD_NOT_FORCED_2 it
+    /// took off the FINISH; the finalization needs `store_final_boxes_in_guard`
+    /// and the knowledge `collect_optimizer_knowledge_for_resume` gathers, both
+    /// of which live here.  Running after the FINISH's own emit is the point:
+    /// that emit force_box'd the FINISH args, so a return box that was virtual
+    /// is materialized by the time it is numbered.
+    ///
+    /// The guard goes straight into `new_operations` rather than back through
+    /// the pass chain, matching `_newoperations.insert` — it has already been
+    /// through every pass once, on its way to being stashed.
+    fn drain_pending_finish_guard_postprocess(&mut self, ctx: &mut OptContext) {
+        let Some(guard_op) = ctx.pending_finish_guard_postprocess.take() else {
+            return;
+        };
+        // virtualize.py:87 store_final_boxes_in_guard(guard_op, []) — the
+        // pendingfields argument is the empty list.
+        let knowledge_for_resume = self.collect_optimizer_knowledge_for_resume(ctx);
+        let knowledge = if knowledge_for_resume.is_empty() {
+            None
+        } else {
+            Some(knowledge_for_resume)
+        };
+        let guard_op = Self::store_final_boxes_in_guard(guard_op, ctx, knowledge, Vec::new());
+        // virtualize.py:88-90 `i = len(_newoperations) - 1; assert i >= 0;
+        // insert(i, guard_op)` — the FINISH this postprocess belongs to is the
+        // last element, so the guard lands immediately in front of it.
+        let Some(i) = ctx.new_operations.len().checked_sub(1) else {
+            debug_assert!(false, "virtualize.py:89 assert i >= 0");
+            return;
+        };
+        ctx.new_operations.insert(i, std::rc::Rc::new(guard_op));
+        // `new_operations_index` maps position -> op with last-occurrence-wins
+        // semantics, which an insert in the middle cannot maintain
+        // incrementally.
+        ctx.rebuild_new_operations_index();
+    }
+
     fn collect_optimizer_knowledge_for_resume(
         &mut self,
         ctx: &mut OptContext,

majit/majit-metainterp/src/optimizeopt/virtualize.rs

@@ -2099,44 +2099,23 @@ impl Optimization for OptVirtualize {
             //           assert i >= 0
             //           self.optimizer._newoperations.insert(i, guard_op)
             //
-            // majit ordering: upstream INSERTS because its postprocess runs
-            // after the FINISH is already appended.  This pass runs BEFORE the
-            // FINISH reaches the terminal emit, so `emit_extra` queues the
-            // stashed guard for the passes after virtualize and
-            // `drain_extra_operations_from` (called right after this method
-            // returns) flushes it through the pipeline first.  The guard lands
-            // in `new_operations` first, the FINISH second — the same final
-            // op order.
+            // The stash here is only half the port: the guard has to be
+            // finalized and inserted AFTER the FINISH is emitted, because
+            // `emit(op)` force_box's the FINISH args and
+            // `store_final_boxes_in_guard` has to see a return box that was
+            // virtual as already materialized.  Finalizing it on the way
+            // through the pipeline instead would encode that same box as still
+            // virtual — a consistent image, but not upstream's image.
             //
-            // The RESUME DATA is where the two diverge.  Upstream finalizes the
-            // guard in `postprocess_FINISH`, i.e. after `emit(op)` forced the
-            // FINISH args, so `store_final_boxes_in_guard` sees a return box
-            // that was virtual as already materialized.  Here the guard is
-            // finalized on the way through the pipeline, before that forcing,
-            // and encodes the same box as still virtual.  Both are consistent
-            // images, but they are not the same image.
-            //
-            // BLOCKER for the faithful order.  `propagate_postprocess` (the
-            // port of optimizer.py's postprocess dispatch) is a method on a
-            // PASS, and the finalization a guard needs is
-            // `Optimizer::store_final_boxes_in_guard` with the knowledge
-            // `collect_optimizer_knowledge_for_resume(&self)` gathers — which
-            // needs the Optimizer, not a pass.  Running it from here with no
-            // knowledge would drop the bridgeopt sections that
-            // `serialize_optimizer_knowledge` puts in every other guard, buying
-            // one ordering divergence with a worse one.  Reaching upstream's
-            // shape needs an Optimizer-side FINISH postprocess that can insert
-            // at `new_operations.len() - 1` after its own emit.
-            //
-            // Nothing arms the token today — the portal-return
-            // `gen_store_back_in_vable` sets `forced_virtualizable`, so
-            // `store_token_in_vable` early-returns and no `GUARD_NOT_FORCED_2`
-            // reaches a FINISH — so neither image is currently observable.
+            // `propagate_postprocess` below runs at the right moment but is a
+            // method on a PASS, and both the finalization and the
+            // `collect_optimizer_knowledge_for_resume` that feeds it are
+            // Optimizer-side.  So it hands the guard to the Optimizer through
+            // `ctx.pending_finish_guard_postprocess`, the same shape
+            // `pending_guard_class_postprocess` uses, and
+            // `drain_pending_finish_guard_postprocess` does the insert.
             OpCode::Finish => {
                 self.finish_guard_op = self.last_guard_not_forced_2.take();
-                if let Some(guard_op) = self.finish_guard_op.clone() {
-                    ctx.emit_extra(ctx.current_pass_idx, guard_op);
-                }
                 OptimizationResult::PassOn
             }
 
@@ -2291,6 +2270,36 @@ impl Optimization for OptVirtualize {
         self.finish_guard_op = None;
     }
 
+    // virtualize.py:84-90 postprocess_FINISH
+    //
+    //   def postprocess_FINISH(self, op):
+    //       guard_op = self._finish_guard_op
+    //       if guard_op is not None:
+    //           guard_op = self.optimizer.store_final_boxes_in_guard(guard_op, [])
+    //           i = len(self.optimizer._newoperations) - 1
+    //           assert i >= 0
+    //           self.optimizer._newoperations.insert(i, guard_op)
+    //
+    // The two Optimizer-side halves are in
+    // `Optimizer::drain_pending_finish_guard_postprocess`, which this hands
+    // the guard to.
+    fn have_postprocess_op(&self, opcode: OpCode) -> bool {
+        matches!(opcode, OpCode::Finish)
+    }
+
+    fn propagate_postprocess(&mut self, op: &Op, ctx: &mut OptContext) {
+        if op.opcode != OpCode::Finish {
+            return;
+        }
+        if let Some(guard_op) = self.finish_guard_op.take() {
+            debug_assert!(
+                ctx.pending_finish_guard_postprocess.is_none(),
+                "postprocess_FINISH queued multiple guards"
+            );
+            ctx.pending_finish_guard_postprocess = Some(guard_op);
+        }
+    }
+
     fn name(&self) -> &'static str {
         "virtualize"
     }

pyre/bench/frame_lineno_mid_replay_regression.py

@@ -15,20 +15,19 @@
 # answer and the replayed one together and a divergence appears as a SECOND
 # element rather than a shifted single value.
 #
-# Why this is not a synthetic bench: the wasm backend does not satisfy the
-# invariant today, and check.py's synthetic suite has no per-backend scoping.
-# Measured on this HEAD -- `plain` below reports `[0, 4]` on wasm against `[4]`
-# on pypy3, CPython, dynasm and cranelift, with the divergence appearing from
-# the first COMPILED call onward and persisting.  Instrumenting both ends showed
-# the marker hook writing the right coordinate into the right frame at the right
-# offset and reading it back intact, and the interpreter then reading 0 from
-# that same address -- so a wasm-side writer clears it between the publish and
-# the residual `sys._getframe`.  Neither `restore_resume_state_from` nor
-# `set_last_instr_from_next_instr` is that writer (probed: neither ever targets
-# the caller frame), and a blackhole `setfield_vable_i` cannot be, because the
-# wasm blackhole builder carries no cpu and that handler would panic.  The
-# invariant is asserted here for the native backends while that is open; the
-# post-return coordinate, which wasm does satisfy, stays in the synthetic bench.
+# It asserts rather than diffing against pypy3 so the recursive case can pin an
+# exact per-level tuple, which an output diff cannot express.
+#
+# It is also the guard for a store-width defect, which is why it is worth
+# keeping separate: `PyFrame.valuestackdepth` and `PyFrame.last_instr` are
+# adjacent machine words, so a store that takes its width from a descr
+# declaring a fixed 8 bytes runs past the first field and deposits the zero
+# high half onto the second.  On a 64-bit target the two are the same number
+# and nothing happens; on wasm32 the words are 4 bytes and `plain` below
+# reported `[0, 4]` against `[4]` everywhere else, from the first compiled call
+# onward.  The publish itself was never at fault -- it stores through
+# `*mut isize` and read back intact; the clobber came afterwards, from the
+# blackhole replaying a `setfield_vable_i` at the neighbouring offset.
 import sys
 
 N = 4000

pyre/check.py

@@ -1806,16 +1806,13 @@ def main():
         # The coordinate a frame reports WHILE it is still running: compiled code
         # runs no per-opcode `last_instr` store, so a replayed frame answers for
         # the instruction it is on only if the blackhole publishes at the
-        # `-live-` marker. Skipped on wasm, which does not satisfy it today --
-        # the guard's own header carries the measurement (the publish lands and
-        # reads back, and a wasm-side writer clears it before the residual
-        # `sys._getframe`). The post-return coordinate, which wasm does satisfy,
-        # stays in the synthetic bench (exception_traceback_frame_lineno).
+        # `-live-` marker. Runs on every backend -- it is also the guard that
+        # catches a store whose width overruns `valuestackdepth` onto the
+        # `last_instr` next to it, which is a 32-bit-only failure.
         chk.run_selfcheck(
             "frame_lineno_mid_replay",
             f"{B}/frame_lineno_mid_replay_regression.py",
             20,
-            skip_backends=("wasm",),
         )
         # The branchy-inlined-callee guard (gh#343) lives in the synthetic parity
         # suite as bridge_branchy_callee.py, gated against pypy by

pyre/pyre-jit-trace/src/descr.rs

@@ -1360,7 +1360,13 @@ static PYFRAME_DESCR_GROUP: LazyLock<PyreObjectDescrGroup> = LazyLock::new(|| {
             (
                 "valuestackdepth",
                 crate::frame_layout::PYFRAME_VALUESTACKDEPTH_OFFSET,
-                8,
+                // `usize`, not a fixed 64-bit int.  The other `Type::Int`
+                // fields in this table are all `i64`, so 8 is right for
+                // them; this one and `last_instr` below are the two that
+                // are a machine word wide.  A literal 8 makes the store a
+                // byte pair too wide on a 32-bit target, and the overrun
+                // lands on `last_instr`, which sits immediately after it.
+                std::mem::size_of::<usize>(),
                 Type::Int,
                 true,
                 false,
@@ -1369,7 +1375,8 @@ static PYFRAME_DESCR_GROUP: LazyLock<PyreObjectDescrGroup> = LazyLock::new(|| {
             (
                 "last_instr",
                 crate::frame_layout::PYFRAME_LAST_INSTR_OFFSET,
-                8,
+                // `isize` — see the width note on `valuestackdepth` above.
+                std::mem::size_of::<isize>(),
                 Type::Int,
                 true,
                 false,

pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs

@@ -1382,11 +1382,15 @@ pub(crate) fn fbw_terminate_with_finish<Sym: WalkSym>(
 ///
 /// A frame the function-entry portal compiled can outlive its trace the same
 /// way a generator's does — a traceback it hands out keeps it alive — and the
-/// lazy route is not available to narrow this back down: the backend frees the
-/// jitframe chain before `execute_token` returns, so that marker would name
-/// freed memory rather than a retained deadframe.  Narrowing the force to the
-/// frames that actually escape needs that retention first; the escape is a
-/// runtime property, which is exactly what the token protocol exists to answer.
+/// lazy route is not available to narrow this back down.  Two things have to
+/// land before it is: the backend frees the jitframe chain before
+/// `execute_token` returns, so the marker would name freed memory rather than
+/// a retained deadframe; and no backend arms `jf_force_descr` for a standalone
+/// trailing `GUARD_NOT_FORCED_2`, which upstream does from
+/// `consider_guard_not_forced_2` (x86/regalloc.py), so the armed-token test
+/// would answer false for a portal exit even once the chain is retained.
+/// Narrowing the force to the frames that actually escape needs both; the
+/// escape is a runtime property, which is what the token protocol answers.
 ///
 /// Storing back here is what makes the token store unnecessary rather than
 /// merely redundant: `gen_store_back_in_vable` sets `forced_virtualizable`, and