jit: port arraylen_vable, drop the vable array getfield, and close the three #1358 follow-ups

majit/charon-corpus/src/lib.rs

@@ -31,6 +31,31 @@ pub fn branch_loop_sum(slice: &[i64], threshold: i64) -> i64 {
     acc
 }
 
+// 2b. Iterator element kinds.  `next()`'s payload carries one reference for
+// a slice iterator (`core::slice::iter::Iter` yields `Option<&T>`) and none
+// for a by-value one (`core::array::iter::IntoIter` yields `Option<T>`), so
+// the two spell the same `Option<&i64>` payload for different reasons: here
+// the element is `&i64` both times, and only the first has a reference the
+// iterator added.  A frontend that peels unconditionally, or never, types one
+// of the two into the wrong register bank.
+#[inline(never)]
+pub fn slice_of_refs_sum(slice: &[&i64]) -> i64 {
+    let mut acc: i64 = 0;
+    for r in slice {
+        acc += **r;
+    }
+    acc
+}
+
+#[inline(never)]
+pub fn array_of_refs_sum(refs: [&i64; 3]) -> i64 {
+    let mut acc: i64 = 0;
+    for r in refs {
+        acc += *r;
+    }
+    acc
+}
+
 // 3. Strategy dispatch (dict-strategy stand-in)
 pub enum Strategy {
     Empty,

majit/majit-charon-reader/tests/corpus.rs

@@ -29,7 +29,10 @@ fn loads_fixture_corpus() {
     // + 2 for the host-registered callback table: `host_registry_dispatch`
     // and `host_registry_dispatch_optional`. `HostCallback` is a type alias,
     // not an item, so it contributes no body.
-    assert_eq!(local_count, 14, "14 local fns expected");
+    //
+    // + 2 for the iterator element-kind pair, `slice_of_refs_sum` and
+    // `array_of_refs_sum`.
+    assert_eq!(local_count, 16, "16 local fns expected");
 }
 
 #[test]

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

@@ -46,13 +46,6 @@ pub(crate) struct VirtualizableConfig {
     /// Same role as `static_field_descrs`, but for the array-pointer
     /// fields on the virtualizable object.
     pub array_field_descrs: Vec<DescrRef>,
-    /// Trace-entry lengths of array fields, parallel to `array_field_offsets`.
-    ///
-    /// Standard virtualizable traces carry array elements in the input box
-    /// layout; the optimizer needs the concrete lengths to map those input
-    /// args back into VirtualizableFieldState without falling back to raw
-    /// heap reads.
-    pub array_lengths: Vec<usize>,
     /// Number of input slots between `OpRef::input_arg_ref(0)` (frame) and the first vable
     /// scalar slot. Equals `JitDriverStaticData::num_reds() - 1` after the
     /// frame is excluded — typically `NUM_EXTRA_REDS` from the
@@ -99,24 +92,6 @@ pub(crate) struct VirtualizableConfig {
     /// before it consults this field at all, so a tracker is still installed
     /// there with this set to `None`.
     pub identity_input_index: Option<usize>,
-    /// Whether the tracker seeds array-element state from the trace-entry
-    /// input args (`init`'s array loop).
-    ///
-    /// `true` (PyFrame and the optimizer unit tests) keeps the legacy
-    /// stopgap behavior: array elements are mapped from input args into
-    /// `VirtualizableFieldState.arrays`, and the loop boundary expands the
-    /// virtualizable back into those element slots.
-    ///
-    /// `false` (the macro state-field JIT) suppresses that seeding. The
-    /// state-field tracer carries `[int; virt]` elements through the live
-    /// `virtualizable_boxes` shadow and splices the symbolically-updated
-    /// boxes straight into the loop JUMP (`collect_jump_args_with_boxes`,
-    /// pyjitpl.py:2982-2989). Re-seeding from the trace-entry input args
-    /// would thread stale loop-entry boxes alongside the fresh shadow boxes,
-    /// double-counting the array at the loop boundary. The identity
-    /// `PtrInfo::Virtualizable` is still installed (so the base is not
-    /// forced); only the per-element seeding is skipped.
-    pub track_array_elements: bool,
 }
 
 /// JitVirtualRef field slot indices.
@@ -130,20 +105,31 @@ pub(crate) const VREF_FORCED_FIELD_INDEX: u32 = 1;
 /// Size descriptor index for the JitVirtualRef struct.
 const VREF_SIZE_DESCR_INDEX: u32 = 0x7F10;
 
-/// TODO: Virtualizable field tracking in the optimizer.
+/// TODO: Virtualizable field tracking in the optimizer — pyre-only, being
+/// retired.
 ///
 /// RPython does NOT track virtualizable field values in the optimizer.
 /// Field tracking happens during tracing (`pyjitpl.py:virtualizable_boxes`),
 /// not in the optimization pipeline. The optimizer only removes
 /// `COND_CALL(OS_JIT_FORCE_VIRTUALIZABLE)` when the target is virtual.
 ///
-/// Pyre's tracing model carries virtualizable fields as trace input args
-/// (`OpRef::input_arg_ref`), and the optimizer maps them via
-/// `VirtualizableFieldState`. This exists because pyre doesn't yet have
-/// RPython's `virtualizable_boxes` model in the metainterp.
+/// The tracing layer now has that model — `TraceCtx::virtualizable_boxes` is
+/// the live shadow, and both loop-close arms carry it into the JUMP — so the
+/// array-element half of this tracker is gone: `init` no longer seeds element
+/// state from the trace-entry input args, because the standard-path read
+/// answers from the shadow and records no op for a fold to match.
 ///
-/// If pyre's tracing layer grows RPython's `virtualizable_boxes` model, this
-/// optimizer-side tracker should no longer be needed.
+/// What is still here, and what retiring the rest costs:
+///
+/// - the identity `PtrInfo::Virtualizable` install. Not a deviation to
+///   remove — it is what keeps the base from being forced, which is the one
+///   virtualizable job upstream's optimizer does have.
+/// - the STATIC field map (`VirtualizableFieldState.fields`), still seeded
+///   from input args. Retiring it needs the same argument the array half
+///   got: that no recorded op reads a static vable field on the standard
+///   path. That has not been established.
+/// - `is_standard_ref` / `mirror_setarrayitem` / `invalidate_array`, which
+///   exist to keep the static map honest and follow it.
 pub(crate) struct VirtualizableTracker {
     config: VirtualizableConfig,
     needs_setup: bool,
@@ -308,50 +294,19 @@ impl VirtualizableTracker {
                 flat_input_idx += 1;
             }
 
-            // The state-field JIT carries array elements through the live
-            // `virtualizable_boxes` shadow into the loop JUMP, so seeding element
-            // state from the trace-entry input args here would double-count them
-            // at the loop boundary. Skip the per-element loop in that mode; the
-            // empty `PtrInfo::Virtualizable` installed below still keeps the
-            // identity base from being forced.
-            if self.config.track_array_elements {
-                for (array_idx, (&_offset, &length)) in self
-                    .config
-                    .array_field_offsets
-                    .iter()
-                    .zip(self.config.array_lengths.iter())
-                    .enumerate()
-                {
-                    let descr_for_slot = self.config.array_field_descrs.get(array_idx).cloned();
-                    let field_idx = descr_for_slot
-                        .as_ref()
-                        .and_then(|d| d.as_field_descr())
-                        .map(|fd| fd.index_in_parent() as u32)
-                        .unwrap_or((1 + num_static + array_idx) as u32);
-                    if let Some(descr) = descr_for_slot {
-                        set_field_descr(&mut state.field_descrs, field_idx, descr);
-                    }
-
-                    let mut elements = Vec::with_capacity(length);
-                    for _ in 0..length {
-                        if flat_input_idx >= ctx.num_inputs() {
-                            break;
-                        }
-                        let slot_tp = ctx
-                            .inputarg_type_at(flat_input_idx)
-                            .unwrap_or(majit_ir::Type::Ref);
-                        elements.push(OpRef::input_arg_typed(flat_input_idx as u32, slot_tp));
-                        flat_input_idx += 1;
-                    }
-                    if !elements.is_empty() {
-                        let elements: Vec<Operand> = elements
-                            .into_iter()
-                            .map(|r| ctx.materialize_operand_at(r))
-                            .collect();
-                        state.arrays.push((array_idx as u32, elements));
-                    }
-                }
-            }
+            // Array elements are deliberately not seeded. Every layout carries
+            // them into the loop JUMP through the tracer's live
+            // `virtualizable_boxes` shadow — the macro state-field JIT via
+            // `JitState::collect_jump_args_with_boxes`, PyFrame via
+            // `jitcode_dispatch::append_virtualizable_boxes` — and the tracer
+            // updates that shadow through `set_virtualizable_entry_at` without
+            // recording an op, so a seeded entry box is invisible to
+            // `mirror_setarrayitem` and goes stale. The standard-path read
+            // records no op either (`TraceCtx::vable_getarrayitem_*_checked`
+            // answers from the shadow, pyjitpl.py:1170-1184), so there is
+            // nothing for a seeded element to fold against in the first place.
+            // Measured before removal: check.py dynasm 434/434 with zero
+            // jit-stats counters moved.
         }
 
         let b = ctx.materialize_operand_at(identity_ref);
@@ -3495,10 +3450,8 @@ mod tests {
                 array_field_offsets: vec![8],
                 array_item_types: vec![Type::Ref],
                 array_field_descrs: vec![],
-                array_lengths: vec![1],
                 vable_input_offset: 0,
                 identity_input_index: Some(0),
-                track_array_elements: true,
             },
         )));
         let forced = opt.force_box(OpRef::input_arg_ref(0), &mut ctx);
@@ -3527,10 +3480,8 @@ mod tests {
             array_field_offsets: vec![8],
             array_item_types: vec![Type::Int],
             array_field_descrs: vec![],
-            array_lengths: vec![1],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -3628,10 +3579,8 @@ mod tests {
             array_field_offsets: vec![],
             array_item_types: vec![],
             array_field_descrs: vec![],
-            array_lengths: vec![],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -3678,10 +3627,8 @@ mod tests {
             array_field_offsets: vec![],
             array_item_types: vec![],
             array_field_descrs: vec![],
-            array_lengths: vec![],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -3710,10 +3657,8 @@ mod tests {
             array_field_offsets: vec![],
             array_item_types: vec![],
             array_field_descrs: vec![],
-            array_lengths: vec![],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -3730,6 +3675,37 @@ mod tests {
         assert!(matches!(result, OptimizationResult::PassOn));
     }
 
+    /// A config that declares an array field carries no lengths at all now:
+    /// the element seeding that needed them is gone, so the shape that used
+    /// to trip the length assertion is just an ordinary config. It still has
+    /// to install the identity `PtrInfo::Virtualizable`, which is the only
+    /// thing `ensure_setup` still owes a virtualizable with arrays.
+    #[test]
+    fn an_array_declaring_config_needs_no_lengths_and_still_installs_the_identity() {
+        let mut ctx = OptContext::with_inputarg_types(8, &[Type::Ref, Type::Int]);
+        let mut pass = OptVirtualize::with_virtualizable(VirtualizableConfig {
+            static_field_offsets: vec![],
+            static_field_types: vec![],
+            static_field_descrs: vec![],
+            array_field_offsets: vec![48, 56],
+            array_item_types: vec![Type::Ref, Type::Ref],
+            array_field_descrs: vec![],
+            vable_input_offset: 0,
+            identity_input_index: Some(0),
+        });
+        pass.setup();
+        if let Some(ref mut vt) = pass.vable {
+            vt.ensure_setup(&mut ctx);
+        }
+        let identity = ctx
+            .get_box_replacement_operand_opt(OpRef::input_arg_ref(0))
+            .expect("the identity inputarg must materialize");
+        assert!(
+            ctx.is_virtualizable(&identity),
+            "ensure_setup must still mark the identity virtualizable so the base is not forced",
+        );
+    }
+
     #[test]
     fn test_standard_virtualizable_init_uses_parent_backed_field_descrs() {
         let mut info = crate::virtualizable::VirtualizableInfo::new(0);
@@ -3789,10 +3765,8 @@ mod tests {
             array_field_offsets: vec![24],
             array_item_types: vec![Type::Int],
             array_field_descrs: vec![],
-            array_lengths: vec![1],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -3836,10 +3810,8 @@ mod tests {
             array_field_offsets: vec![24],
             array_item_types: vec![Type::Int],
             array_field_descrs: vec![],
-            array_lengths: vec![1],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -3888,10 +3860,8 @@ mod tests {
             array_field_offsets: vec![8],
             array_item_types: vec![Type::Int],
             array_field_descrs: vec![],
-            array_lengths: vec![1],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -3997,10 +3967,8 @@ mod tests {
             array_field_offsets: vec![8],
             array_item_types: vec![Type::Int],
             array_field_descrs: vec![],
-            array_lengths: vec![1],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         pass.setup();
 
@@ -4112,10 +4080,8 @@ mod tests {
             array_field_offsets: vec![24],
             array_item_types: vec![Type::Int],
             array_field_descrs: vec![],
-            array_lengths: vec![1],
             vable_input_offset: 0,
             identity_input_index: Some(0),
-            track_array_elements: true,
         });
         let mut constants: majit_ir::ConstMap<majit_ir::Value> = majit_ir::ConstMap::new();
         let mut ops = vec![

majit/majit-metainterp/src/pyjitpl.rs

@@ -4338,7 +4338,6 @@ impl<M: Clone> MetaInterp<M> {
             }
             self.virtualizable_info().map(|info| {
                 let mut config = info.to_optimizer_config();
-                config.array_lengths = ctx.virtualizable_array_lengths().unwrap_or(&[]).to_vec();
                 // virtualizable.py:90 read_boxes input layout = [frame,
                 // extra_reds..., vable_scalars..., array_items...]. The
                 // canonical source of `vable_input_offset` is the active
@@ -24743,7 +24742,6 @@ mod tests {
             config.array_item_types,
             info.to_optimizer_config().array_item_types
         );
-        assert_eq!(config.array_lengths, vec![2]);
     }
 
     // ── JitIface hook/callback parity tests (rpython/jit/metainterp/test/test_jitiface.py) ──

majit/majit-metainterp/src/pyjitpl/dispatch.rs

@@ -13462,4 +13462,21 @@ mod tests {
             )]
         );
     }
+
+    /// The two panics `TraceCtx::vable_snapshot_buildable` exists to keep
+    /// unreachable.  Its caller reports a false answer as
+    /// `GuardSnapshotVableUntyped` and aborts to interpretation; these pin
+    /// what that abort is worth, one per untyped position, since the
+    /// identity slot and the rest are read by separate arms above.
+    #[test]
+    #[should_panic(expected = "virtualizable identity must be typed")]
+    fn build_vable_snapshot_boxes_panics_on_an_untyped_identity() {
+        build_vable_snapshot_boxes(&[majit_ir::OpRef::int_op(3), majit_ir::OpRef::NONE]);
+    }
+
+    #[test]
+    #[should_panic(expected = "virtualizable_boxes entry must be typed")]
+    fn build_vable_snapshot_boxes_panics_on_an_untyped_entry() {
+        build_vable_snapshot_boxes(&[majit_ir::OpRef::NONE, majit_ir::OpRef::ref_op(7)]);
+    }
 }