fix(codegen): root_reload's cost cap counts root loads, not derived values

crates/perry-codegen/src/root_reload.rs

@@ -253,13 +253,37 @@ const NON_COLLECTING: &[&str] = &[
 ];
 
 /// Guard against a pathological function turning this pass into the compile's
-/// bottleneck: the reachability walk is O(blocks) per slot load, so the product
-/// is what matters. Above the cap the function keeps today's IR — the pass is
-/// an improvement, not a correctness precondition, so declining is safe.
+/// bottleneck: the reachability walk is O(blocks) per ROOT LOAD — one walk per
+/// `groups` entry below, not one per reloadable value — so `blocks × groups` is
+/// the product that bounds the cost.
 ///
 /// Sized from the corpora: the largest function in the dependency-scale corpus
 /// (`zod`'s parse core) is ~1400 blocks with ~90 slot loads, an order of
 /// magnitude under this.
+///
+/// # Declining is NOT correctness-neutral (#9135 follow-up)
+///
+/// The previous comment here read "the pass is an improvement, not a
+/// correctness precondition, so declining is safe". Under the NATIVE root
+/// lowering that is false, and Claude-of-Duty's `Arm.constructor` is the
+/// counterexample: the receiver of an inline class-field store is read out of
+/// its slot, unmasked to an `i64`/`double`, and carried across `buildSleeve`.
+/// RS4GC relocates the `addrspace(1)` load but cannot touch the unmasked copy
+/// (the "case 2" shape in this module's header), so when this pass declines
+/// NOTHING re-reads the slot and the store lands in a from-space object. It
+/// faults under `PERRY_GC_PROTECT_FROMSPACE=1` and silently corrupts the field
+/// without it.
+///
+/// That function measured 4924 blocks × 747 root loads = 3.7M — comfortably
+/// under this cap — but the check multiplied by `values.len()` (2102, every
+/// pure-bit-op derivation counted separately since #7664) for 10.4M, so it
+/// declined on a metric ~2.8x larger than the cost it was guarding.
+///
+/// So the bound stays (a real pathological function must still be able to opt
+/// out), but it is measured against the walk's actual driver. A function that
+/// genuinely exceeds it still keeps today's IR and can still carry a stale
+/// register: the cap trades a compile-time cliff for a correctness risk, and
+/// that residual risk is why the number is generous rather than tight.
 const MAX_BLOCK_LOAD_PRODUCT: usize = 8_000_000;
 
 /// How long a derivation may get before the pass declines to re-materialise it.
@@ -549,7 +573,27 @@ pub(crate) fn apply_to_function(func: &mut LlFunction) -> usize {
             break;
         }
     }
-    if blocks.len().saturating_mul(values.len()) > MAX_BLOCK_LOAD_PRODUCT {
+    // Built BEFORE the cap check: `groups` is what the reachability walk below
+    // iterates (one walk per root load), so it is the term the cap must be
+    // measured against. See `MAX_BLOCK_LOAD_PRODUCT`.
+    //
+    // Keyed on `(recipe[0], root_ptr)`, not `recipe[0]` alone — #7725's capture-GET extension is
+    // the one case where a chain's `root_ptr` CHANGES partway through (the closure-ptr slot
+    // becomes a synthetic per-index key once the derivation passes through
+    // `js_closure_get_capture_bits`), so two sub-chains sharing one root LOAD can need two
+    // different invalidation conditions. Before #7725 every member of a `recipe[0]` group had
+    // the identical `root_ptr` by construction (it was always inherited, never overridden), so
+    // this is additive: it can only split a group that would otherwise have mixed two
+    // conditions under one, never change the grouping of any existing (non-capture) chain.
+    let mut groups: HashMap<((usize, usize), String), Vec<usize>> = HashMap::new();
+    for (i, v) in values.iter().enumerate() {
+        groups
+            .entry((v.recipe[0], v.root_ptr.clone()))
+            .or_default()
+            .push(i);
+    }
+
+    if blocks.len().saturating_mul(groups.len()) > MAX_BLOCK_LOAD_PRODUCT {
         return 0;
     }
 
@@ -604,21 +648,6 @@ pub(crate) fn apply_to_function(func: &mut LlFunction) -> usize {
     //
     // Grouping by root load also puts the cost back at O(blocks × loads).
     //
-    // Keyed on `(recipe[0], root_ptr)`, not `recipe[0]` alone — #7725's capture-GET extension is
-    // the one case where a chain's `root_ptr` CHANGES partway through (the closure-ptr slot
-    // becomes a synthetic per-index key once the derivation passes through
-    // `js_closure_get_capture_bits`), so two sub-chains sharing one root LOAD can need two
-    // different invalidation conditions. Before #7725 every member of a `recipe[0]` group had
-    // the identical `root_ptr` by construction (it was always inherited, never overridden), so
-    // this is additive: it can only split a group that would otherwise have mixed two
-    // conditions under one, never change the grouping of any existing (non-capture) chain.
-    let mut groups: HashMap<((usize, usize), String), Vec<usize>> = HashMap::new();
-    for (i, v) in values.iter().enumerate() {
-        groups
-            .entry((v.recipe[0], v.root_ptr.clone()))
-            .or_default()
-            .push(i);
-    }
     let mut group_keys: Vec<((usize, usize), String)> = groups.keys().cloned().collect();
     group_keys.sort_unstable();
 

crates/perry-codegen/src/root_reload_tests.rs

@@ -1076,3 +1076,79 @@ fn the_slot_layout_note_helpers_are_non_collecting() {
          transposition of js_gc_note_slot_layout"
     );
 }
+
+/// The cost cap must be measured against the number of ROOT LOADS, not the
+/// number of reloadable values.
+///
+/// `MAX_BLOCK_LOAD_PRODUCT` guards the reachability walk, and that walk runs
+/// once per `groups` entry — one per root load — not once per reloadable
+/// value. Since #7664 extended a recipe through pure bit ops, `values` counts
+/// every derivation separately, so `blocks * values` can be several times
+/// `blocks * groups`. Measuring the cap against `values` declined functions
+/// whose real cost was well inside the bound, and a declined function keeps
+/// every stale register: under the native root lowering nothing else re-reads
+/// the slot.
+///
+/// Each block below carries one root load plus a `MAX_RECIPE`-length pure
+/// derivation off it — the `masked_receiver` shape, replicated — so `values`
+/// is 8x `groups`. At 1100 blocks that is ~9.7M by the old metric against an
+/// 8M cap, and ~1.2M by the new one. The reloads must still be inserted.
+///
+/// Found on Claude-of-Duty's `Arm.constructor` (4924 blocks, 747 root loads,
+/// 2102 values): 10.35M by the old metric, 3.68M by the new. Declining there
+/// left the receiver of an inline class-field store unrooted across a call,
+/// and the store landed in a from-space object.
+#[test]
+fn the_cap_counts_root_loads_not_derived_values() {
+    // `load + bitcast + 6 * and` is exactly MAX_RECIPE (8) steps. A longer
+    // chain is refused outright by the recipe fixpoint, which is a different
+    // decline and would not exercise the cap at all.
+    const BLOCKS: usize = 1100;
+    const MASKS: usize = 6;
+
+    let mut f = LlFunction::new("wide", DOUBLE, vec![(DOUBLE, "%arg".into())]);
+    let mut labels = Vec::with_capacity(BLOCKS + 1);
+    labels.push(f.create_block("entry").label.clone());
+    for i in 0..BLOCKS {
+        labels.push(f.create_block(&format!("b{i}")).label.clone());
+    }
+
+    let slot;
+    {
+        let b = f.block_mut(0).unwrap();
+        slot = b.alloca(DOUBLE);
+        b.store(DOUBLE, "%arg", &slot);
+        b.call_void(
+            "js_shadow_slot_bind",
+            &[(crate::types::I32, "0"), (PTR, &slot)],
+        );
+        b.br(&labels[1]);
+    }
+
+    for i in 0..BLOCKS {
+        let next = labels.get(i + 2).cloned();
+        let b = f.block_mut(i + 1).unwrap();
+        let boxed = b.load(DOUBLE, &slot);
+        let mut cur = b.bitcast_double_to_i64(&boxed);
+        for _ in 0..MASKS {
+            cur = b.and(I64, &cur, "281474976710655");
+        }
+        b.call(DOUBLE, "js_object_get_field_by_name_f64", &[]);
+        b.call_void(
+            "js_object_set_field_by_name",
+            &[(I64, &cur), (DOUBLE, "0.0")],
+        );
+        match next {
+            Some(n) => b.br(&n),
+            None => b.ret(DOUBLE, "0.0"),
+        }
+    }
+
+    let rewrites = apply_to_function(&mut f);
+    assert_eq!(
+        rewrites, BLOCKS,
+        "every block holds one stale masked receiver across a collecting call; \
+         measuring the cap against the derived-value count declines the whole \
+         function and silently leaves all of them in place"
+    );
+}