jit: trace a nested _getframe chain through its virtualref pairs

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

@@ -994,6 +994,11 @@ impl OptHeap {
     /// array, so a sparse high slot costs one entry and nothing else.
     const HEADER_FIELD_SLOT_BASE: u32 = 0x8000_0000;
 
+    /// Slot base for a field descriptor the parent's slot list does not place
+    /// (see [`Self::field_slot_index`]).  Disjoint from both the dense
+    /// `index_in_parent` space and [`Self::HEADER_FIELD_SLOT_BASE`].
+    const UNSLOTTED_FIELD_SLOT_BASE: u32 = 0x4000_0000;
+
     /// Compute the `PtrInfo._fields` slot for a field descriptor.
     ///
     /// RPython uses `descr.get_index()` only for `info._fields[index]`
@@ -1013,17 +1018,19 @@ impl OptHeap {
     /// which is what `make_equal_to`'s `Box.type` invariant fires on.
     ///
     /// So use the position only where the parent's list actually holds this
-    /// field at it, and otherwise fall back to the descr's own key, which for
-    /// an unnumbered field is minted out of its payload and cannot collide
-    /// with a position.
+    /// field at it.  A field that list does not place carries no position, and
+    /// the descr's own `index()` is not one either: every `VirtualizableInfo`
+    /// field descriptor is minted with index `0`, so answering with it puts
+    /// the vable's array-pointer read on the same slot as the vable token
+    /// store, and the read comes back with the token.  Key those by offset in
+    /// a band of their own instead.
     ///
-    /// That fallback does NOT cover the header words, which is why they are
-    /// answered before it: `PyObject.ob_type` carries `index()` `0`, a real
-    /// position, so routing it through the fallback returns it to the very slot
-    /// the check just refused. They resolve through no field list at all --
-    /// `OptVirtualize` folds `is_typeptr` from `known_class` and `is_w_class`
-    /// from the virtual's class identity -- so give them a band above the
-    /// positional space instead, keyed by offset to keep the two apart.
+    /// The header words are answered before all of that, in a band of their
+    /// own.  They resolve through no field list at all -- `OptVirtualize`
+    /// folds `is_typeptr` from `known_class` and `is_w_class` from the
+    /// virtual's class identity -- and an offset alone would not separate a
+    /// header word from an unplaced field naming the same word, so the two
+    /// bands stay apart.
     pub(crate) fn field_slot_index(descr: &DescrRef) -> u32 {
         let descr_idx = descr.index();
         let Some(field_descr) = descr.as_field_descr() else {
@@ -1045,7 +1052,16 @@ impl OptHeap {
         if holds_this_field {
             index as u32
         } else {
-            descr_idx
+            // The parent places this field nowhere, so there is no slot number
+            // for it — and `descr.index()` is not one either.  Slot numbers are
+            // `index_in_parent` values, small and dense; a descriptor minted
+            // without a parent slot would land on top of whatever the parent
+            // really holds at that slot, and a read through one descriptor
+            // would then answer with the value stored through the other.
+            // Offset is the identity such a field does carry, so key it by
+            // that: descriptors naming the same word alias, descriptors naming
+            // different words never do.
+            Self::UNSLOTTED_FIELD_SLOT_BASE + field_descr.offset() as u32
         }
     }
 
@@ -6223,8 +6239,12 @@ mod tests {
         let pos100 = ctx.materialize_operand_at(OpRef::ref_op(100));
         ctx.set_ptr_info(&pos100, PtrInfo::instance(None, None));
         let val101 = ctx.materialize_operand_at(OpRef::ref_op(101));
+        // Seed the slot the reader will consult: `field_slot_index`, not the
+        // descriptor's own key.  A descriptor with no parent carries no slot
+        // number, so the two differ.
+        let slot = OptHeap::field_slot_index(&descr);
         ctx.with_ptr_info_mut(&pos100, |info| {
-            info.setfield(descr.index(), val101.clone());
+            info.setfield(slot, val101.clone());
         })
         .unwrap();
         pass.produce_potential_short_preamble_ops(&mut sb, &mut ctx);

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

@@ -4618,7 +4618,22 @@ impl Optimizer {
         // innermost level may absorb what the current propagate queues.
         ctx.extra_pending.push(pending);
         let result = self.drain_innermost_pending(ctx);
-        ctx.extra_pending.pop();
+        let unfinished = ctx
+            .extra_pending
+            .pop()
+            .expect("the active extra-operation drain level must remain installed");
+        if result.is_err() {
+            // `InvalidLoop` unwinds the recursive Rust drain in place of
+            // RPython's ordinary exception unwinding.  Keep both operations
+            // emitted by the failing propagation and the untouched tail from
+            // this level available to the caller's unwind.  Each outer level
+            // appends its own untouched tail in turn, so no parked producer is
+            // silently discarded and a reused context never observes a
+            // half-drained scheduling state.
+            ctx.extra_operations_after.extend(unfinished);
+        } else {
+            debug_assert!(unfinished.is_empty());
+        }
         result
     }
 
@@ -6176,6 +6191,30 @@ mod tests {
         }
     }
 
+    struct QueueExtraThenInvalidate;
+
+    impl Optimization for QueueExtraThenInvalidate {
+        fn propagate_forward(
+            &mut self,
+            op: &Op,
+            _op_rc: &majit_ir::OpRc,
+            ctx: &mut OptContext,
+        ) -> OptimizationResult {
+            if op.opcode == OpCode::IntAdd {
+                ctx.emit_extra(
+                    ctx.current_pass_idx,
+                    Op::new(OpCode::IntMul, &[op.arg(0), op.arg(1)]),
+                );
+                ctx.signal_invalid_loop("test invalid loop while draining extras");
+            }
+            OptimizationResult::PassOn
+        }
+
+        fn name(&self) -> &'static str {
+            "queue_extra_then_invalidate"
+        }
+    }
+
     #[test]
     fn test_optimizer_passthrough() {
         let mut opt = Optimizer::new();
@@ -6192,6 +6231,28 @@ mod tests {
         assert_eq!(result[0].opcode, OpCode::IntAdd);
     }
 
+    #[test]
+    fn invalid_loop_preserves_queued_and_unprocessed_extra_operations() {
+        let mut opt = Optimizer::new();
+        opt.add_pass(Box::new(QueueExtraThenInvalidate));
+        let mut ctx = OptContext::new(2);
+        let lhs = rooted_resop_operand(Type::Int, 0);
+        let rhs = rooted_resop_operand(Type::Int, 1);
+        ctx.emit_extra_at(0, Op::new(OpCode::IntAdd, &[lhs.clone(), rhs.clone()]));
+        ctx.emit_extra_at(0, Op::new(OpCode::IntSub, &[lhs, rhs]));
+
+        let result = opt.drain_extra_operations_from(0, &mut ctx);
+
+        assert!(result.is_err());
+        assert!(ctx.extra_pending.is_empty());
+        let queued: Vec<_> = ctx
+            .extra_operations_after
+            .iter()
+            .map(|(_, op)| op.opcode)
+            .collect();
+        assert_eq!(queued, [OpCode::IntMul, OpCode::IntSub]);
+    }
+
     #[test]
     fn test_restart_from_extra_operation_rediscovers_first_pass() {
         let hits = Rc::new(Cell::new(0));

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

@@ -7666,7 +7666,11 @@ mod tests {
         // source of truth, matching RPython's HeapOp.produce_op → opinfo.setfield.
         let obj_box = ctx2.get_box_replacement_operand_opt(targetargs[0]).unwrap();
         let pop = ctx2
-            .with_ptr_info_mut(&obj_box, |info| info.take_preamble_field(0))
+            .with_ptr_info_mut(&obj_box, |info| {
+                info.take_preamble_field(crate::optimizeopt::heap::OptHeap::field_slot_index(
+                    &field_descr,
+                ))
+            })
             .flatten();
         assert!(pop.is_some(), "PreambleOp must be in PtrInfo._fields");
         let pop = pop.unwrap();
@@ -7777,7 +7781,11 @@ mod tests {
         // the Const as the body-visible Box.
         let obj_box = ctx2.get_box_replacement_operand_opt(targetargs[0]).unwrap();
         let pop = ctx2
-            .with_ptr_info_mut(&obj_box, |info| info.take_preamble_field(0))
+            .with_ptr_info_mut(&obj_box, |info| {
+                info.take_preamble_field(crate::optimizeopt::heap::OptHeap::field_slot_index(
+                    &field_descr,
+                ))
+            })
             .flatten();
         assert!(
             pop.is_some(),

majit/majit-metainterp/src/trace_ctx.rs

@@ -1362,6 +1362,57 @@ impl TraceCtx {
         self.virtualref_boxes.last().copied()
     }
 
+    /// The current concrete address of one `virtualref_boxes` entry.
+    ///
+    /// The `usize` beside each box is the address the object had when the pair
+    /// was pushed, and the object it names is movable: a minor collection
+    /// relocates it and forwards the stamp, leaving the pushed copy naming the
+    /// old address.  `opimpl_virtual_ref_finish` documents the same hazard on
+    /// the same list.  Read the address back through `concrete_of_opref` —
+    /// pyre's `getref_base()` — so a pair that has moved still matches, and
+    /// keep the pushed copy only for an entry carrying no stamp at all.
+    fn virtualref_entry_ptr(&self, entry: (OpRef, usize)) -> usize {
+        match self.concrete_of_opref(entry.0) {
+            Some(Value::Ref(r)) => r.as_usize(),
+            _ => entry.1,
+        }
+    }
+
+    /// Resolve a live tracing-time vref back to its `[virtualbox, vrefbox]`
+    /// pair.  This is the paired walk `vrefs_after_residual_call` makes over
+    /// `MetaInterp.virtualref_boxes` (`pyjitpl.py`): callers that execute
+    /// `jit_force_virtual(vref)` need the paired virtual box that
+    /// `stop_tracking_virtualref` publishes through `VIRTUAL_REF_FINISH`.
+    ///
+    /// Search from the innermost pair because frame-chain vrefs are nested in
+    /// the same order as `virtualref_boxes`.  A stopped pair has had its vref
+    /// entry replaced by `CONST_NULL`, exactly as upstream, so it cannot match.
+    pub fn live_virtualref_pair_for_ptr(&self, vref_ptr: usize) -> Option<(OpRef, OpRef)> {
+        if vref_ptr == 0 {
+            return None;
+        }
+        self.virtualref_boxes
+            .chunks_exact(2)
+            .rev()
+            .find(|pair| self.virtualref_entry_ptr(pair[1]) == vref_ptr)
+            .map(|pair| (pair[0].0, pair[1].0))
+    }
+
+    /// Find the virtual box for a concrete object named by either a live or an
+    /// already-stopped vref pair. `stop_tracking_virtualref` replaces only
+    /// `virtualref_boxes[i + 1]` with `CONST_NULL`; the adjacent virtual box
+    /// remains in the upstream list until `virtual_ref_finish` pops the scope.
+    pub fn virtualref_virtual_for_object_ptr(&self, object_ptr: usize) -> Option<OpRef> {
+        if object_ptr == 0 {
+            return None;
+        }
+        self.virtualref_boxes
+            .chunks_exact(2)
+            .rev()
+            .find(|pair| self.virtualref_entry_ptr(pair[0]) == object_ptr)
+            .map(|pair| pair[0].0)
+    }
+
     /// `pyjitpl.py rebuild_state_after_failure`'s
     /// `self.virtualref_boxes = virtualref_boxes`.  A bridge resumes into its
     /// parent's still-open `virtual_ref` scopes, so the pairs the parent guard
@@ -5186,6 +5237,59 @@ impl TraceCtx {
         )
     }
 
+    /// The array half of `virtualizable.py write_boxes`, emitted into the trace
+    /// for one array field of the STANDARD virtualizable.
+    ///
+    /// `pyjitpl.py synchronize_virtualizable` runs that write-back after every
+    /// vable store, but only against the recording-time virtualizable: upstream
+    /// readers of a virtualizable array are traced through and read the boxes,
+    /// so the compiled trace never needs the array itself to be current.  A
+    /// consumer that reads the array at run time instead needs the same writes
+    /// emitted, which is what this records.
+    ///
+    /// `items` is `(element index, box)` pairs; only the listed slots are
+    /// written, so a caller covering a sub-range of the array leaves the rest
+    /// alone.  The emission shape is `gen_store_back_in_vable`'s array loop —
+    /// one `getfield_gc_r` of `array_pointer_field_descr` followed by a
+    /// `setarrayitem_gc` per item under `array_item_descr`.  Neither the token
+    /// store nor `forced_virtualizable` is touched: this writes the image out,
+    /// it does not force the virtualizable.
+    ///
+    /// The shadow is left alone — it already holds these values and stays
+    /// authoritative for the rest of the trace.
+    pub fn vable_array_region_write_back(
+        &mut self,
+        vable_opref: OpRef,
+        array_index: usize,
+        items: &[(i64, OpRef)],
+    ) -> bool {
+        let Some(info) = self.virtualizable_info.clone() else {
+            return false;
+        };
+        if array_index >= info.array_fields.len() {
+            return false;
+        }
+        let field_descr = info.array_pointer_field_descr(array_index);
+        let array_descr = info.array_item_descr(array_index);
+        let array_opref = self.vable_getfield_ref_descr(vable_opref, field_descr.clone());
+        // `executor.execute` for the read: the array base has to carry its
+        // concrete half, or the consumer below reaches the backend with an
+        // operand no producer answers for.  Same step every other recorded
+        // vable array-base read takes.
+        let vable_concrete = self.concrete_of_opref(vable_opref);
+        self.stamp_vable_array_base(array_opref, vable_concrete, &field_descr);
+        self.heapcache_getfield_now_known(vable_opref, field_descr.index(), array_opref);
+        for &(item_index, value) in items {
+            let index = self.const_int(item_index);
+            self.profiler()
+                .count_ops(OpCode::SetarrayitemGc, crate::counters::OPS);
+            self.profiler()
+                .count_ops(OpCode::SetarrayitemGc, crate::counters::RECORDED_OPS);
+            self.vable_setarrayitem_descr(array_opref, index, value, array_descr.clone());
+        }
+        true
+    }
+
     /// `_opimpl_setarrayitem_vable` body with the `_nonstandard_virtualizable`
     /// decision already taken by the caller (see
     /// [`Self::nonstandard_virtualizable`]).

pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.cranelift.jitstats

@@ -3,12 +3,13 @@ descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
 fbw_blackhole_adopted_multi_frame=0
-fbw_blackhole_adopted_single_frame=5
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=1
 internal_compile_panics=0
-loops_aborted=5
-loops_compiled=1
+loops_aborted=0
+loops_compiled=2
+retraces_compiled=0

pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.dynasm.jitstats

@@ -3,12 +3,13 @@ descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
 fbw_blackhole_adopted_multi_frame=0
-fbw_blackhole_adopted_single_frame=5
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=1
 internal_compile_panics=0
-loops_aborted=5
-loops_compiled=1
+loops_aborted=0
+loops_compiled=2
+retraces_compiled=0

pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.wasm.jitstats

@@ -3,12 +3,13 @@ descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
 fbw_blackhole_adopted_multi_frame=0
-fbw_blackhole_adopted_single_frame=5
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=1
 internal_compile_panics=0
-loops_aborted=5
-loops_compiled=1
+loops_aborted=0
+loops_compiled=2
+retraces_compiled=0

pyre/bench/synth/getframe_bridge_force_after_store_declined.cranelift.jitstats

@@ -1,14 +1,15 @@
-bridges_compiled=0
+bridges_compiled=1
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
 fbw_blackhole_adopted_multi_frame=0
-fbw_blackhole_adopted_single_frame=20
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
-guard_failures=4114
+guard_failures=201
 internal_compile_panics=0
-loops_aborted=20
+loops_aborted=0
 loops_compiled=1
+retraces_compiled=0

pyre/bench/synth/getframe_bridge_force_after_store_declined.dynasm.jitstats

@@ -1,14 +1,15 @@
-bridges_compiled=0
+bridges_compiled=1
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
 fbw_blackhole_adopted_multi_frame=0
-fbw_blackhole_adopted_single_frame=20
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
 fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
-guard_failures=4114
+guard_failures=201
 internal_compile_panics=0
-loops_aborted=20
+loops_aborted=0
 loops_compiled=1
+retraces_compiled=0