jit: preserve box identity across short preambles

majit/majit-ir/src/ptr_info.rs

@@ -390,6 +390,16 @@ pub struct VirtualizableFieldState {
     /// Tracked array field values: (array_field_index, element_values).
     /// Indices correspond to VirtualizableInfo::array_fields order.
     pub arrays: Vec<(u32, Vec<Operand>)>,
+    /// Ordinary (non-virtualizable) heap fields read off this object, keyed by
+    /// `FieldDescr::index_in_parent` — the same `_fields` role
+    /// `AbstractStructPtrInfo` plays at `info.py:175-214`.
+    ///
+    /// `fields` above is a different index space (`VirtualizableInfo::
+    /// static_fields` order), so the two cannot share storage. Upstream needs
+    /// no such split: it has no virtualizable-specific `PtrInfo` subclass, so
+    /// a virtualizable frame carries a plain `InstancePtrInfo` and every field
+    /// read off it lands in the one `_fields` list the heap cache consults.
+    pub heap_fields: Vec<(u32, FieldEntry)>,
     /// info.py:91-92
     pub last_guard_pos: i32,
 }
@@ -1155,6 +1165,15 @@ impl PtrInfo {
                 }
                 v.fields.push((field_idx, value));
             }
+            PtrInfo::Virtualizable(v) => {
+                for entry in &mut v.heap_fields {
+                    if entry.0 == field_idx {
+                        entry.1 = FieldEntry::Value(value.clone());
+                        return;
+                    }
+                }
+                v.heap_fields.push((field_idx, FieldEntry::Value(value)));
+            }
             _ => {}
         }
     }
@@ -1171,6 +1190,13 @@ impl PtrInfo {
                 v.fields.retain(|(k, _)| *k != field_idx);
                 v.fields.push((field_idx, FieldEntry::Preamble(pop)));
             }
+            // The catch-all below re-seats `self` as an `InstancePtrInfo`,
+            // which would drop the tracked virtualizable state. Store the
+            // hoisted field alongside the other ordinary heap fields instead.
+            PtrInfo::Virtualizable(v) => {
+                v.heap_fields.retain(|(k, _)| *k != field_idx);
+                v.heap_fields.push((field_idx, FieldEntry::Preamble(pop)));
+            }
             _ => {
                 *self = PtrInfo::Instance(InstancePtrInfo {
                     descr: None,
@@ -1204,6 +1230,10 @@ impl PtrInfo {
                 .fields
                 .iter()
                 .any(|(k, e)| *k == field_idx && e.is_preamble()),
+            PtrInfo::Virtualizable(v) => v
+                .heap_fields
+                .iter()
+                .any(|(k, e)| *k == field_idx && e.is_preamble()),
             _ => false,
         }
     }
@@ -1276,6 +1306,7 @@ impl PtrInfo {
             }
             PtrInfo::Virtual(v) => v.fields.retain(|(k, _)| *k != field_idx),
             PtrInfo::VirtualStruct(v) => v.fields.retain(|(k, _)| *k != field_idx),
+            PtrInfo::Virtualizable(v) => v.heap_fields.retain(|(k, _)| *k != field_idx),
             _ => {}
         }
     }
@@ -1334,6 +1365,11 @@ impl PtrInfo {
                 .iter()
                 .find(|(k, _)| *k == field_idx)
                 .map(|(_, v)| FieldEntry::Value(v.clone())),
+            PtrInfo::Virtualizable(v) => v
+                .heap_fields
+                .iter()
+                .find(|(k, _)| *k == field_idx)
+                .map(|(_, e)| e.clone()),
             _ => None,
         }
     }

majit/majit-metainterp/src/blackhole.rs

@@ -128,7 +128,7 @@ pub enum BhReturnType {
 /// Re-export BhDescr from codewriter::jitcode — shared descriptor type
 /// between codewriter assembler and blackhole interpreter.
 /// RPython `history.py:AbstractDescr` parity.
-pub use majit_translate::jitcode::{BhCallDescr, BhDescr};
+pub use majit_translate::jitcode::{BhCallDescr, BhDescr, DescrTable, EMPTY_DESCR_TABLE};
 
 /// Per-jitdriver static data visible to the blackhole interpreter.
 ///
@@ -201,7 +201,7 @@ pub struct BlackholeInterpreter {
     /// stores the assembler list itself, so the table is shared and never
     /// copied; :154 is its only consumer and only reads. This has the same
     /// lifetime shape as the sibling `cpu: Option<&'static dyn Backend>`.
-    pub descrs: &'static [BhDescr],
+    pub descrs: &'static dyn DescrTable,
     /// RPython `blackhole.py:289` `self.op_catch_exception = builder.op_catch_exception`.
     pub op_catch_exception: u8,
     /// RPython `blackhole.py:290` `self.op_rvmprof_code = builder.op_rvmprof_code`.
@@ -385,7 +385,7 @@ impl Default for BlackholeInterpreter {
         Self {
             cpu: None,
             // blackhole.py:280 `EMPTY_LIST_I = [] # shared`.
-            descrs: &[],
+            descrs: EMPTY_DESCR_TABLE,
             // RPython blackhole.py:289 — copied from builder in `acquire_interp`.
             // Sentinel `u8::MAX` matches RPython's `insns.get('…', -1)` fallback.
             op_catch_exception: u8::MAX,
@@ -2043,7 +2043,7 @@ pub struct BlackholeInterpBuilder {
     pub op_rvmprof_code: u8,
     /// RPython `blackhole.py:103` `self.descrs`.
     /// Populated by `setup_descrs()` from the assembler's descriptor table.
-    pub descrs: &'static [BhDescr],
+    pub descrs: &'static dyn DescrTable,
     /// Dispatch table: opcode byte → handler fn pointer.
     /// RPython builds `dispatch_loop` closure via `unrolling_iterable`;
     /// Rust uses indirect call through this table.
@@ -2080,7 +2080,7 @@ impl BlackholeInterpBuilder {
             op_catch_exception: u8::MAX,
             op_rvmprof_code: u8::MAX,
             // blackhole.py:280 `EMPTY_LIST_I = [] # shared`.
-            descrs: &[],
+            descrs: EMPTY_DESCR_TABLE,
             dispatch_table: std::sync::Arc::new(Vec::new()),
             jitdrivers_sd: Vec::new(),
         }
@@ -2222,7 +2222,7 @@ impl BlackholeInterpBuilder {
     }
 
     /// RPython `blackhole.py:102-103` `setup_descrs(descrs)`.
-    pub fn setup_descrs(&mut self, descrs: &'static [BhDescr]) {
+    pub fn setup_descrs(&mut self, descrs: &'static dyn DescrTable) {
         self.descrs = descrs;
     }
 
@@ -4741,13 +4741,10 @@ mod tests {
         fn test_clone_context_from_mirrors_acquire_interp_fields() {
             let mut builder = super::build_inline_call_only_bh_builder();
             let mut parent = builder.acquire_interp();
-            let table: &'static [BhDescr] = Box::leak(
-                vec![
-                    BhDescr::VableField { index: 1 },
-                    BhDescr::VableArray { index: 2 },
-                ]
-                .into_boxed_slice(),
-            );
+            let table: &'static [BhDescr; 2] = Box::leak(Box::new([
+                BhDescr::VableField { index: 1 },
+                BhDescr::VableArray { index: 2 },
+            ]));
             parent.descrs = table;
             // Make the parent's vable / jitdriver state non-default so
             // the assertion below distinguishes "copied" from
@@ -4766,7 +4763,10 @@ mod tests {
             assert_eq!(callee.op_rvmprof_code, parent.op_rvmprof_code);
             assert_eq!(callee.op_live, parent.op_live);
             assert!(
-                std::ptr::eq(callee.descrs.as_ptr(), parent.descrs.as_ptr()),
+                std::ptr::eq(
+                    std::ptr::from_ref(callee.descrs).cast::<()>(),
+                    std::ptr::from_ref(parent.descrs).cast::<()>(),
+                ),
                 "clone_context_from must alias the parent table"
             );
             assert_eq!(callee.virtualizable_ptr, parent.virtualizable_ptr);
@@ -6874,7 +6874,12 @@ fn read_descr<'a>(bh: &'a BlackholeInterpreter, code: &[u8], pos: usize) -> (&'a
         });
         return (descr, pos + 2);
     }
-    let descr = &bh.descrs[descr_idx]; // RPython: no fallback, index must be valid
+    let descr = bh.descrs.get(descr_idx).unwrap_or_else(|| {
+        panic!(
+            "d-arg descrs[{descr_idx}] is out of range for {} entries",
+            bh.descrs.len()
+        )
+    }); // RPython: no fallback, index must be valid
     (descr, pos + 2)
 }
 

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

@@ -292,22 +292,15 @@ impl RuntimeBhDescr {
     }
 }
 
-/// Newtype so the write-once, read-only global descr pool can live in a
-/// `static`.  `RuntimeBhDescr` is `!Sync` only because its
-/// `Call(JitCallTarget)` variant carries raw function-address pointers; the
-/// pool this crate installs never holds that variant (build-time jitcodes'
-/// residual-call targets resolve from a funcptr register at dispatch time),
-/// and the pool is written once through the `OnceLock` and thereafter only
-/// read, so sharing it across threads is sound — the same `unsafe impl`
-/// rationale as [`JitCode`] itself above.
-struct GlobalDescrPool(Vec<RuntimeBhDescr>);
-
-// SAFETY: `GlobalDescrPool` is written once (via `OnceLock::set`) and read-only
-// thereafter; the raw pointers `RuntimeBhDescr` can carry are stable code
-// addresses, and the pool this crate installs carries none. `OnceLock<T>: Sync`
-// additionally requires `T: Send`.
-unsafe impl Send for GlobalDescrPool {}
-unsafe impl Sync for GlobalDescrPool {}
+/// Runtime view of the process-global build-time descriptor pool.
+pub trait RuntimeDescrTable: Sync {
+    fn get(&self, index: usize) -> Option<&'static RuntimeBhDescr>;
+    fn len(&self) -> usize;
+
+    fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+}
 
 /// Process-global build-time descr pool — RPython's single shared
 /// `Assembler.descrs` (`assembler.py:23`).  Runtime-emitted jitcodes keep a
@@ -318,27 +311,22 @@ unsafe impl Sync for GlobalDescrPool {}
 /// crate (`pyre-jit-trace`) from its build-time `ALL_DESCRS` / `ALL_JITCODES`
 /// tables; `majit-metainterp` cannot build it because those tables live above
 /// it.
-static GLOBAL_BUILD_DESCR_POOL: std::sync::OnceLock<GlobalDescrPool> = std::sync::OnceLock::new();
+static GLOBAL_BUILD_DESCR_POOL: std::sync::OnceLock<&'static dyn RuntimeDescrTable> =
+    std::sync::OnceLock::new();
 
 /// Install the process-global build-time descr pool.  Idempotent: the first
 /// call wins and later calls are ignored (the pool is a frozen build artifact,
 /// identical across callers).  See `GLOBAL_BUILD_DESCR_POOL`.
 ///
-/// `build` runs only on the call that installs the pool.  It takes a closure
-/// rather than a built `Vec` because the callers sit on hot paths — the jd1
-/// driver installs before every `_unpackiterable_unknown_length` walk — and
-/// building the pool clones every `BhDescr` in the binary (including each call
-/// descr's `EffectInfo` raw descr sets).  Materializing that just to have
-/// `OnceLock::set` drop it is the whole cost of the call.
-pub fn init_global_build_descr_pool(build: impl FnOnce() -> Vec<RuntimeBhDescr>) {
-    GLOBAL_BUILD_DESCR_POOL.get_or_init(|| GlobalDescrPool(build()));
+pub fn init_global_build_descr_pool(table: &'static dyn RuntimeDescrTable) {
+    let _ = GLOBAL_BUILD_DESCR_POOL.set(table);
 }
 
 /// The installed global build-time descr pool, or `None` if the embedding
 /// crate has not installed one (e.g. a standalone metainterp unit test that
 /// only exercises runtime-built jitcodes).
-pub(crate) fn global_build_descr_pool() -> Option<&'static [RuntimeBhDescr]> {
-    GLOBAL_BUILD_DESCR_POOL.get().map(|pool| pool.0.as_slice())
+pub(crate) fn global_build_descr_pool() -> Option<&'static dyn RuntimeDescrTable> {
+    GLOBAL_BUILD_DESCR_POOL.get().copied()
 }
 
 /// Per-`JitCode` descrs.  Pyre's analog of

majit/majit-metainterp/src/lib.rs

@@ -123,7 +123,7 @@ pub use jit_state::{
     bridge_decode_red,
 };
 pub use jitcode::{
-    BC_GOTO, JitArgKind, JitCallArg, JitCode, JitCodeBuilder, RuntimeBhDescr,
+    BC_GOTO, JitArgKind, JitCallArg, JitCode, JitCodeBuilder, RuntimeBhDescr, RuntimeDescrTable,
     init_global_build_descr_pool, insns, live_slots_for_state_field_jit,
 };
 pub use jitdriver::{