perf(gc): bind scalar-replacement root slots once per frame, not once per store

crates/perry-codegen/src/expr/mod.rs

@@ -147,8 +147,9 @@ pub(crate) use scalar_slot_root::{
     root_scalar_replaced_slot, root_scalar_replaced_slot_unconditional,
 };
 pub(crate) use shadow_slot::{
-    emit_shadow_slot_bind_for_local, emit_shadow_slot_clear, emit_shadow_slot_update_for_expr,
-    enable_persistent_shadow_slot_for_array_alias, expr_is_known_non_pointer_shadow_value,
+    emit_persistent_shadow_root_barrier, emit_shadow_slot_bind_for_local, emit_shadow_slot_clear,
+    emit_shadow_slot_update_for_expr, enable_persistent_shadow_slot_for_array_alias,
+    expr_is_known_non_pointer_shadow_value,
 };
 
 /// One in-flight inline-constructor return target. See

crates/perry-codegen/src/expr/scalar_slot_root.rs

@@ -32,11 +32,31 @@
 //! alloca`, so both a mark-sweep root walk and an evacuating minor's
 //! rewrite pass reach the real alloca rather than a stale mirror.
 //!
-//! The bind is not repeated per alloca-per-store *shape*, only per store
-//! *site*: the alloca is entry-hoisted and never moves, so one bind covers
-//! the rest of the frame's life. Re-binding at a later store to the same
-//! field is what re-runs the incremental-mark root barrier, which is
-//! required for the same reason an ordinary local's re-assignment re-binds.
+//! The bind runs **once, in the function-entry setup** — not at each store.
+//! What a bind does is `slot_ptrs[idx] = alloca; stack[idx] = *alloca;
+//! active[idx] = true; root_barrier(*alloca)`. For an entry-hoisted alloca the
+//! first three are loop-invariant: the address never changes, and every reader
+//! of a bound slot (`visit_shadow_stack_root_slots`, `js_shadow_slot_get`)
+//! dereferences `slot_ptrs[idx]` in preference to the `stack[idx]` mirror, so
+//! the mirror is dead storage. Only the root barrier is per-store work, and it
+//! is emitted inline and guarded (`emit_persistent_shadow_root_barrier`).
+//!
+//! This is the same treatment `enable_persistent_shadow_slot_for_array_alias`
+//! already gives a `const item = arr[i]` alias, for the same reason.
+//!
+//! The hoist does **not** move when the rooted value is read. The collector
+//! reads the alloca at collection time, exactly as it did when the bind sat at
+//! the store; nothing is snapshotted into a register and re-read later. What
+//! disappears is a redundant copy, not an observation point.
+//!
+//! # Why the slot must be initialized before the bind
+//!
+//! Binding at entry makes the slot `active` from function entry, so the
+//! collector starts dereferencing the alloca *before* any store reaches it. An
+//! uninitialized alloca would hand the root-word decoder stack garbage that
+//! can pass `is_plausible_heap_addr`. Every path that hoists a bind therefore
+//! initializes its alloca to `undefined` in `entry_allocas`, ahead of the
+//! `entry_post_init_setup` region the bind lands in.
 //!
 //! # The gate
 //!
@@ -51,17 +71,17 @@ use super::*;
 
 use perry_hir::Expr;
 
-use crate::types::{I32, PTR};
+use crate::types::{I32, I64, PTR};
 
 /// Root the scalar-replacement alloca `slot` against the value expression
 /// that was just stored into it.
 ///
-/// Call *after* the `store` — `js_shadow_slot_bind` reads the alloca to seed
-/// the shadow mirror and to run the root write barrier, so the new value has
-/// to be in place. Callers that store a canonicalized raw `f64` (the
-/// `numeric_store` arm of `expr::property_set`) must not call this at all:
-/// those bits are a plain double by construction, and the shared root-word
-/// decoder rejects them, but reserving a slot for them would be pure waste.
+/// Call *after* the `store` — the emitted root barrier reads the alloca back,
+/// so the new value has to be in place. Callers that store a canonicalized raw
+/// `f64` (the `numeric_store` arm of `expr::property_set`) must not call this
+/// at all: those bits are a plain double by construction, and the shared
+/// root-word decoder rejects them, but reserving a slot for them would be pure
+/// waste.
 pub(crate) fn root_scalar_replaced_slot(ctx: &mut FnCtx<'_>, slot: &str, value: &Expr) {
     if expr_is_known_non_pointer_shadow_value(ctx, value) {
         return;
@@ -80,20 +100,35 @@ pub(crate) fn root_scalar_replaced_slot_unconditional(ctx: &mut FnCtx<'_>, slot:
 }
 
 fn bind_scalar_replaced_slot(ctx: &mut FnCtx<'_>, slot: &str) {
-    let slot_idx = match ctx.scalar_slot_shadow_slots.get(slot).copied() {
-        Some(idx) => idx,
-        None => {
-            // `None` means shadow-stack emission is off for this build; the
-            // caller must not emit slot traffic either.
-            let Some(idx) = ctx.func.reserve_shadow_slot() else {
-                return;
-            };
-            ctx.scalar_slot_shadow_slots.insert(slot.to_string(), idx);
-            idx
-        }
-    };
-    ctx.block().call_void(
-        "js_shadow_slot_bind",
-        &[(I32, &slot_idx.to_string()), (PTR, slot)],
-    );
+    if !ctx.scalar_slot_shadow_slots.contains_key(slot) {
+        // `None` means shadow-stack emission is off for this build; the
+        // caller must not emit slot traffic either.
+        let Some(idx) = ctx.func.reserve_shadow_slot() else {
+            return;
+        };
+        ctx.scalar_slot_shadow_slots.insert(slot.to_string(), idx);
+        // One bind for the whole frame. `reserve_shadow_slot` runs first so a
+        // lazily-created `js_shadow_frame_push` is already in
+        // `entry_post_init_setup` when this call is appended after it — a bind
+        // that ran before the push would land in the caller's frame.
+        ctx.func.entry_setup_call_void(
+            "js_shadow_slot_bind",
+            &[(I32, &idx.to_string()), (PTR, slot)],
+        );
+    }
+    emit_scalar_slot_store_barrier(ctx, slot);
+}
+
+/// The per-store remainder of a bind: shade the newly stored value so an
+/// in-flight incremental mark cannot miss it.
+///
+/// The operand is read back from the alloca rather than threaded down from the
+/// caller's value register **because that is precisely what the bind it
+/// replaces did** (`js_shadow_slot_bind` dereferences `value_slot`). The load
+/// sits in the same block, immediately after the store that produced the value,
+/// with nothing in between — it cannot observe a later write, and LLVM forwards
+/// it to the stored register.
+fn emit_scalar_slot_store_barrier(ctx: &mut FnCtx<'_>, slot: &str) {
+    let value_bits = ctx.block().load(I64, slot);
+    crate::expr::emit_persistent_shadow_root_barrier(ctx, &value_bits);
 }

crates/perry-codegen/src/expr/shadow_slot.rs

@@ -170,7 +170,17 @@ pub(crate) fn emit_shadow_slot_bind_for_local(ctx: &mut FnCtx<'_>, local_id: u32
     );
 }
 
-fn emit_persistent_shadow_root_barrier(ctx: &mut FnCtx<'_>, value_bits: &str) {
+/// Emit the incremental-mark root shading barrier for a value that has just
+/// been written into an already-bound (persistent) root slot.
+///
+/// This is the only part of `js_shadow_slot_bind` that is genuinely per-store:
+/// re-recording `slot_ptrs[idx]` and re-mirroring the value are loop-invariant
+/// for an entry-hoisted alloca, but a pointer stored into a root *after* the
+/// collector scanned roots still has to be shaded. Guarding on
+/// `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` inline keeps the common
+/// (no incremental cycle in flight) path down to a load, a compare, and a
+/// not-taken branch instead of a TLS-touching call.
+pub(crate) fn emit_persistent_shadow_root_barrier(ctx: &mut FnCtx<'_>, value_bits: &str) {
     let active =
         ctx.block()
             .load_atomic_seq_cst(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4);

crates/perry-codegen/src/stmt/let_stmt.rs

@@ -452,6 +452,13 @@ pub(crate) fn lower_let(
             };
             let source = lower_expr(ctx, object)?;
             let source_slot = ctx.func.alloca_entry(DOUBLE);
+            // See the array-element slots below: the root bind is hoisted to
+            // function entry, so this alloca is a live root before the store
+            // below runs. Give it a decodable `undefined` first.
+            let source_undef =
+                crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED));
+            ctx.func
+                .entry_allocas_push_store(DOUBLE, &source_undef, &source_slot);
             ctx.block().store(DOUBLE, &source, &source_slot);
             // #6968: the whole point of capturing the receiver here is that the
             // source local may be overwritten afterwards — at which moment this
@@ -599,8 +606,18 @@ pub(crate) fn lower_let(
         if ctx.non_escaping_arrays.contains_key(&id) {
             let n = elements.len();
             let mut slots: Vec<String> = Vec::with_capacity(n);
+            // Initialize to `undefined` in the entry block, like the
+            // object-literal field slots below. `root_scalar_replaced_slot`
+            // binds a pointer-capable element's alloca as a GC root once at
+            // function entry, which makes the collector dereference it from
+            // entry onward — before the element store runs, and on paths where
+            // it never runs at all. An uninitialized alloca would feed the
+            // root-word decoder stack garbage.
+            let undef = crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED));
             for _ in 0..n {
-                slots.push(ctx.func.alloca_entry(DOUBLE));
+                let slot = ctx.func.alloca_entry(DOUBLE);
+                ctx.func.entry_allocas_push_store(DOUBLE, &undef, &slot);
+                slots.push(slot);
             }
             // Evaluate each element expression first; store the
             // result into its slot. Order matches source, so any