majit dynasm: give a JUMP's surplus argument slot the frame base every other slot uses

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

@@ -2894,24 +2894,41 @@ impl<'a> AssemblerARM64<'a> {
                     target_arglocs.len()
                 };
                 for (i, src_loc) in arglocs[..remap_count].iter().enumerate() {
-                    let dst_loc = if i < target_arglocs.len() {
-                        target_arglocs[i]
-                    } else {
-                        let dst_ofs = crate::regalloc::get_ebp_ofs(0, i);
-                        Loc::Frame(crate::regloc::FrameLoc::new(i, dst_ofs, false))
-                    };
+                    // One classification, two consumers. It picks the location
+                    // set the pair rides — set 2 carries the floats and gets
+                    // the float scratch — and, when the destination has to be
+                    // synthesized below, the kind of the slot itself.
+                    // Hard-coding the slot's kind instead described a slot the
+                    // value never lands in: `regalloc_push` / `regalloc_pop`
+                    // read exactly this field to choose their scratch
+                    // register, and `loc_width` reads it for the width.
                     let arg_tp = op
                         .getarglist()
                         .get(i)
                         .and_then(|arg| self.opref_type_at(arg.to_opref(), Some(op_index)))
                         .unwrap_or(Type::Int);
-                    if arg_tp == Type::Float {
-                        src_locations2.push(*src_loc);
-                        dst_locations2.push(dst_loc);
+                    let is_float = arg_tp == Type::Float;
+                    let dst_loc = if i < target_arglocs.len() {
+                        target_arglocs[i]
                     } else {
-                        src_locations1.push(*src_loc);
-                        dst_locations1.push(dst_loc);
-                    }
+                        // The canonical base for a frame position, the
+                        // one `FrameManager` was built with
+                        // (`get_baseofs_of_frame_field`). Passing 0 here named
+                        // a slot `FIRST_ITEM_OFFSET` bytes below the one the
+                        // regalloc means by the same position, so a source and
+                        // this destination could denote the same value and
+                        // different storage.
+                        let base_ofs = crate::jitframe::FIRST_ITEM_OFFSET as i32;
+                        let dst_ofs = crate::regalloc::get_ebp_ofs(base_ofs, i);
+                        Loc::Frame(crate::regloc::FrameLoc::new(i, dst_ofs, is_float))
+                    };
+                    let (srcs, dsts) = if is_float {
+                        (&mut src_locations2, &mut dst_locations2)
+                    } else {
+                        (&mut src_locations1, &mut dst_locations1)
+                    };
+                    srcs.push(*src_loc);
+                    dsts.push(dst_loc);
                 }
                 let tmpreg1 = Loc::Reg(crate::regloc::RegLoc::new(16, false));
                 let tmpreg2 = Loc::Reg(crate::regloc::RegLoc::new(15, true));

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

@@ -3875,26 +3875,48 @@ impl<'a> Assembler386<'a> {
                     target_arglocs.len()
                 };
                 for (i, src_loc) in arglocs[..remap_count].iter().enumerate() {
+                    // One classification, two consumers. It picks the location
+                    // set the pair rides — set 2 carries the floats and gets
+                    // the float scratch — and, when the destination has to be
+                    // synthesized below, the kind of the slot itself.
+                    // Hard-coding the slot's kind instead described a slot the
+                    // value never lands in: `regalloc_push` / `regalloc_pop`
+                    // read exactly this field to choose their scratch
+                    // register, and `loc_width` reads it for the width.
+                    //
+                    // `ebp_loc_pat!` rather than `Loc::Frame` alone because a
+                    // frame-pointer location has two spellings and both carry
+                    // `is_float` (`regloc.py:113 class FrameLoc(RawEbpLoc)`);
+                    // naming one sent the other to the integer set.
+                    let is_float = match src_loc {
+                        Loc::Reg(r) => r.is_xmm,
+                        ebp_loc_pat!(e) => e.is_float,
+                        // An immediate is re-materialized into whichever set
+                        // its destination is in, and an address is not a legal
+                        // parallel-move operand at all.
+                        _ => false,
+                    };
                     let dst_loc = if i < target_arglocs.len() {
                         target_arglocs[i]
                     } else {
-                        let dst_ofs = crate::regalloc::get_ebp_ofs(0, i);
-                        Loc::Frame(crate::regloc::FrameLoc::new(i, dst_ofs, false))
+                        // The canonical base for a frame position, the
+                        // one `FrameManager` was built with
+                        // (`get_baseofs_of_frame_field`). Passing 0 here named
+                        // a slot `FIRST_ITEM_OFFSET` bytes below the one the
+                        // regalloc means by the same position, so a source and
+                        // this destination could denote the same value and
+                        // different storage.
+                        let base_ofs = crate::jitframe::FIRST_ITEM_OFFSET as i32;
+                        let dst_ofs = crate::regalloc::get_ebp_ofs(base_ofs, i);
+                        Loc::Frame(crate::regloc::FrameLoc::new(i, dst_ofs, is_float))
                     };
-                    match src_loc {
-                        Loc::Reg(r) if r.is_xmm => {
-                            src_locations2.push(*src_loc);
-                            dst_locations2.push(dst_loc);
-                        }
-                        Loc::Frame(f) if f.ebp_loc.is_float => {
-                            src_locations2.push(*src_loc);
-                            dst_locations2.push(dst_loc);
-                        }
-                        _ => {
-                            src_locations1.push(*src_loc);
-                            dst_locations1.push(dst_loc);
-                        }
-                    }
+                    let (srcs, dsts) = if is_float {
+                        (&mut src_locations2, &mut dst_locations2)
+                    } else {
+                        (&mut src_locations1, &mut dst_locations1)
+                    };
+                    srcs.push(*src_loc);
+                    dsts.push(dst_loc);
                 }
                 let tmpreg1 = Loc::Reg(crate::regloc::X86_64_SCRATCH_REG);
                 let tmpreg2 = Loc::Reg(crate::regloc::XMM15);

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

@@ -0,0 +1,222 @@
+//! A build-time jitcode table and the one descr pool its bodies index.
+//!
+//! `CodeWriter.make_jitcodes()` (codewriter.py:89) produces two lists that
+//! only mean anything together: `all_jitcodes`, and the single shared
+//! `Assembler.descrs` (assembler.py:23) that every `d`/`j` argcode in every
+//! body indexes. A host that runs the codewriter at build time and embeds the
+//! two serialized lists in its binary has to join them back into the runtime
+//! shapes — `Arc<JitCode>` shells and a `RuntimeBhDescr` pool. This type is
+//! that join, and it is the only place the two lists meet.
+//!
+//! **What the join has to preserve.** A `BhDescr::JitCode` slot names its
+//! callee by an `all_jitcodes` index, and the object that index resolves to
+//! must be *the* object the table holds at it — `codewriter.py:80
+//! all_jitcodes[jitcode.index] is jitcode`, an identity, not an equality.
+//! Minting a second shell per pool slot satisfies every read that only wants a
+//! body and breaks every read that asks whether two references name the same
+//! jitcode: an `Arc::ptr_eq` dedup while flattening a registry sees one callee
+//! twice, and an index stamped on one shell is read back off the other.
+//!
+//! **Why there is no cycle to break.** The shells carry an EMPTY per-jitcode
+//! `exec.descrs` and resolve every operand through this pool as the
+//! process-global fallback ([`JitCode::descr_at`],
+//! [`init_global_build_descr_pool`]). So the pool holds jitcodes and the
+//! jitcodes hold nothing — a `BC_INLINE_CALL` chain of any depth resolves
+//! against one table. Copying the pool into each shell instead makes the depth
+//! the copy was taken at the depth that resolves: the callees inside a copied
+//! pool are shells of their own, and whatever pool they were given is the one
+//! their own operands read.
+
+use std::sync::Arc;
+
+use super::{CanonicalBhDescr, CanonicalJitCode, JitCode, RuntimeBhDescr, RuntimeDescrTable};
+
+/// A materialized build-time `all_jitcodes` + `Assembler.descrs` pair.
+///
+/// Process-lifetime by construction: [`Self::materialize`] leaks both lists,
+/// because the pool is installed as the global `descr_at` fallback, whose
+/// entries are handed out as `&'static`.
+pub struct EmbeddedJitCodeTable {
+    jitcodes: &'static [Arc<JitCode>],
+    descrs: &'static [RuntimeBhDescr],
+}
+
+// SAFETY: `materialize` builds only the `Descr` and `JitCode` variants. The
+// two variants that carry raw pointers — `Call`'s `JitCallTarget` and
+// `AssemblerToken` — are never constructed here, so nothing in either list is
+// a pointer this type could hand across a thread. A future arm that mints one
+// invalidates this, which is why the constructor is the only writer.
+unsafe impl Send for EmbeddedJitCodeTable {}
+unsafe impl Sync for EmbeddedJitCodeTable {}
+
+impl EmbeddedJitCodeTable {
+    /// Join the two serialized lists into runtime shells and their pool.
+    ///
+    /// `canonical` must be `all_jitcodes` in allocation order, which
+    /// `codewriter.py:68 jitcode.index = index` makes the same thing as
+    /// "indexed by `jitcode.index`" — asserted here, since every `j` operand
+    /// in the pool is an index into it and a list that drifted from its own
+    /// indices resolves silently to the wrong callee.
+    pub fn materialize(
+        canonical: &[Arc<CanonicalJitCode>],
+        descrs: Vec<CanonicalBhDescr>,
+    ) -> &'static Self {
+        let jitcodes: &'static [Arc<JitCode>] = Box::leak(
+            canonical
+                .iter()
+                .enumerate()
+                .map(|(index, core)| {
+                    assert_eq!(
+                        core.try_index(),
+                        Some(index),
+                        "jitcode {:?} sits at position {index} but names index {:?}; \
+                         every `j` operand indexes this list",
+                        core.name,
+                        core.try_index(),
+                    );
+                    Arc::new(JitCode::from_canonical((**core).clone()))
+                })
+                .collect::<Vec<_>>()
+                .into_boxed_slice(),
+        );
+        let pool: &'static [RuntimeBhDescr] = Box::leak(
+            descrs
+                .into_iter()
+                .map(|descr| match descr {
+                    // The callee is the table's own entry, cloned — an
+                    // `Arc::clone`, so `ptr_eq` against the table holds.
+                    CanonicalBhDescr::JitCode { jitcode_index, .. } => {
+                        RuntimeBhDescr::JitCode(Arc::clone(&jitcodes[jitcode_index]))
+                    }
+                    // Every other variant is an ordinary `d` descr and carries
+                    // through unchanged: the runtime pool element and the
+                    // build-time one are the same type.
+                    other => RuntimeBhDescr::Descr(Box::new(other)),
+                })
+                .collect::<Vec<_>>()
+                .into_boxed_slice(),
+        );
+        Box::leak(Box::new(Self {
+            jitcodes,
+            descrs: pool,
+        }))
+    }
+
+    /// `metainterp_sd.jitcodes` (warmspot.py:281-282) — the flat registry
+    /// `resume.py:1338-1340` indexes by a frame's `jitcode_pos`.
+    pub fn jitcodes(&self) -> &'static [Arc<JitCode>] {
+        self.jitcodes
+    }
+
+    /// The shared pool, in `Assembler.descrs` order.
+    pub fn descrs(&self) -> &'static [RuntimeBhDescr] {
+        self.descrs
+    }
+
+    /// The jitcode an `all_jitcodes` index names.
+    pub fn by_index(&self, index: usize) -> Option<&'static Arc<JitCode>> {
+        self.jitcodes.get(index)
+    }
+
+    /// The jitcode a graph leaf name names, or `None`.
+    ///
+    /// A name is not unique — the codewriter derives it from the graph leaf,
+    /// and two distinct graphs can end on the same one — so this returns the
+    /// first match and is only a lookup for callers that hold a name and
+    /// nothing better. Anything that can carry an index should use
+    /// [`Self::by_index`], which is what the operands themselves do.
+    pub fn by_name(&self, name: &str) -> Option<&'static Arc<JitCode>> {
+        self.jitcodes.iter().find(|jitcode| jitcode.name() == name)
+    }
+
+    /// Install this pool as the process-global `descr_at` fallback.
+    ///
+    /// Idempotent through [`init_global_build_descr_pool`]: the first table
+    /// wins. Until this runs, a shell's operands resolve against its own empty
+    /// `exec.descrs` and every lookup returns `None`.
+    pub fn install_as_global_pool(&'static self) {
+        super::init_global_build_descr_pool(self);
+    }
+}
+
+impl RuntimeDescrTable for EmbeddedJitCodeTable {
+    fn get(&self, index: usize) -> Option<&'static RuntimeBhDescr> {
+        self.descrs.get(index)
+    }
+
+    fn len(&self) -> usize {
+        self.descrs.len()
+    }
+
+    fn jitcodes(&self) -> &'static [Arc<JitCode>] {
+        self.jitcodes
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Two jitcodes, the second reachable from the first through a `j` slot.
+    fn fixture() -> (Vec<Arc<CanonicalJitCode>>, Vec<CanonicalBhDescr>) {
+        let caller = Arc::new(CanonicalJitCode::new("caller"));
+        caller.set_index(0);
+        caller.set_body(Default::default());
+        let callee = Arc::new(CanonicalJitCode::new("callee"));
+        callee.set_index(1);
+        callee.set_body(Default::default());
+        (
+            vec![caller, callee],
+            vec![CanonicalBhDescr::JitCode {
+                jitcode_index: 1,
+                fnaddr: 0,
+                calldescr: Default::default(),
+            }],
+        )
+    }
+
+    /// The subject: a `j` slot resolves to the table's own entry, not to a
+    /// second shell that merely carries the same body.
+    #[test]
+    fn a_jitcode_slot_resolves_to_the_table_entry_itself() {
+        let (canonical, descrs) = fixture();
+        let table = EmbeddedJitCodeTable::materialize(&canonical, descrs);
+        let from_slot = table.descrs()[0]
+            .as_jitcode()
+            .expect("the `j` slot must resolve to a jitcode");
+        let from_table = table.by_index(1).expect("index 1 is in the table");
+        assert!(
+            Arc::ptr_eq(from_slot, from_table),
+            "`all_jitcodes[jitcode.index] is jitcode` (codewriter.py:80) is an \
+             identity: a second shell with the same body passes every read of \
+             the body and fails every `Arc::ptr_eq`",
+        );
+    }
+
+    /// The shells hold no pool of their own, so nothing is stored twice and
+    /// depth cannot decide what resolves.
+    #[test]
+    fn a_shell_carries_no_pool_of_its_own() {
+        let (canonical, descrs) = fixture();
+        let table = EmbeddedJitCodeTable::materialize(&canonical, descrs);
+        for jitcode in table.jitcodes() {
+            assert!(
+                jitcode.exec.descrs.is_empty(),
+                "a build-time shell resolves through the global pool; a \
+                 per-shell copy makes its callees' operands read whichever \
+                 pool those callees were handed",
+            );
+        }
+    }
+
+    /// A list whose positions disagree with its own `jitcode.index` stamps
+    /// cannot be indexed by a `j` operand, and says so at materialization
+    /// rather than resolving to the wrong callee.
+    #[test]
+    #[should_panic(expected = "names index")]
+    fn a_table_out_of_order_with_its_indices_is_refused() {
+        let (mut canonical, descrs) = fixture();
+        canonical.swap(0, 1);
+        let _ = EmbeddedJitCodeTable::materialize(&canonical, descrs);
+    }
+}

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

@@ -1,7 +1,9 @@
 mod assembler;
+mod embedded;
 
 pub(crate) use assembler::scalar_size;
 pub use assembler::{JitCodeBuilder, live_slots_for_state_field_jit};
+pub use embedded::EmbeddedJitCodeTable;
 pub use majit_translate::jitcode::{
     BhCallDescr as CanonicalBhCallDescr, BhDescr as CanonicalBhDescr, BhInteriorFieldSpec,
     JitCode as CanonicalJitCode,
@@ -297,6 +299,19 @@ pub trait RuntimeDescrTable: Sync {
     fn get(&self, index: usize) -> Option<&'static RuntimeBhDescr>;
     fn len(&self) -> usize;
 
+    /// The build-time `all_jitcodes` list this pool's `j` operands index
+    /// (`codewriter.py:89 make_jitcodes`), positioned by `jitcode.index`.
+    ///
+    /// Empty by default: a host that decodes its jitcodes on demand has no
+    /// such list to hand over, and nothing here requires one. A host that
+    /// does return it is stating that those indices are already assigned, so
+    /// a second numbering must continue above them rather than restart —
+    /// `resume.py:1338-1340` indexes one list by a frame's `jitcode_pos`, and
+    /// two numberings over the same slots make that lookup ambiguous.
+    fn jitcodes(&self) -> &'static [std::sync::Arc<JitCode>] {
+        &[]
+    }
+
     fn is_empty(&self) -> bool {
         self.len() == 0
     }
@@ -329,6 +344,14 @@ pub(crate) fn global_build_descr_pool() -> Option<&'static dyn RuntimeDescrTable
     GLOBAL_BUILD_DESCR_POOL.get().copied()
 }
 
+/// The build-time `all_jitcodes` the installed pool numbers against, or empty.
+///
+/// See [`RuntimeDescrTable::jitcodes`]: this is the prefix of the flat registry
+/// that is already assigned, so any numbering done at run time starts above it.
+pub(crate) fn global_build_jitcodes() -> &'static [std::sync::Arc<JitCode>] {
+    global_build_descr_pool().map_or(&[], |pool| pool.jitcodes())
+}
+
 /// Per-`JitCode` descrs.  Pyre's analog of
 /// `BlackholeInterpBuilder.descrs` (`blackhole.py:103`) /
 /// `BlackholeInterpreter.descrs` (`blackhole.py:288`).  RPython has a