perf(codegen): store reference values inline in the dynamic-key write IC (#8108)

changelog.d/8183-dyn-ic-reference-store.md

@@ -0,0 +1,79 @@
+`perf(codegen)`: the inline dynamic-key write IC now stores **reference** values
+(object / string / bigint) through a barriered inline arm instead of diverting
+them to the outlined helper. `o.k = { … }` in a loop drops **32.6% of its
+instructions, 28.9% of its cycles and 29.2% of its wall time** — 7.85x → 5.56x
+vs Node on the object-write matrix's `rhs_allocating` shape — with peak RSS and
+binary size unchanged (33,248 → 33,200 KB; 13,544,480 bytes both arms).
+
+Before this, `lower_put_value_dyn_ic_inline`'s entry predicate ANDed in "the
+value tag is not pointer/string/bigint", so every reference-valued write left
+the inline path before the receiver guards and took
+`js_put_value_set_dyn_ic` — one cross-crate call per write that re-validated,
+in Rust, exactly the guards the inline block had already proved. The tag now
+SELECTS a store arm: `put.dynic.store.scalar` keeps the pre-existing bare store
+(its `GC_STORE_AUDIT(POINTER_FREE)` claim is unchanged), and
+`put.dynic.store.ref` runs `emit_jsvalue_slot_store_scalar_aware_on_block` —
+byte-for-byte the static write PIC's pointer-capable store, reached under
+strictly stronger conditions, since the guards above it are that PIC's guards
+and this block additionally knows the value carries a reference tag.
+
+No new rooting obligation: the target is materialised below every operand that
+can collect (the call site's existing evaluation-order argument), and all three
+bookkeeping helpers are `gc-leaf-function`, so nothing between the re-read and
+the store is a collection point. `scripts/gc_root_dominance_check.py` over
+`scripts/gc_root_dominance_corpus.sh` stays at 0 violations / 0 unrooted
+allocas with an empty allowlist and 40/40 seeded violations caught.
+
+### This is #8108's measured prize, reached by a different route
+
+#8108 ("the static write PIC still rejects any safepointing RHS") names three
+cells — `rhs_call` 18.28x, `rhs_pointer` 9.21x, `rhs_allocating` 4.64x — and
+proposes admitting a safepointing RHS into the static write PIC. Measured on
+`a3118cfea` before writing any code, two thirds of that framing is wrong and
+the named lever would have made the largest cell slower:
+
+* **`rhs_pointer` is already on the static PIC.** Its RHS is `Expr::LocalGet`,
+  which `put_value_rhs_is_safepoint_free` has always admitted. The gate never
+  rejected it; its 9.21x is the pointer store path, not the safepoint rule.
+* **`rhs_call` would REGRESS.** Splitting the call into a local
+  (`const v = f(); o.x = v`) is exactly the IR slice A would produce, and it
+  costs **+21.4% instructions** (5.362G → 6.512G, +18% wall): the static PIC's
+  hit block emits three unconditional `gc-leaf` bookkeeping calls whenever the
+  value is not statically provable non-pointer, where the dyn IC proves it at
+  runtime and stores bare. 95% of that cell's cost is the closure call
+  (425 of 447 instructions per iteration), not the write.
+* **`rhs_allocating` is the real prize**, and it does not need the gate touched
+  at all: the dyn IC already roots correctly, so widening its inline store to
+  reference values captures the same win with no change to any safepoint rule.
+  Arm B lands within 2.8% of the static-PIC ceiling (2.860G vs 2.782G).
+
+The safepoint gate at `expr/proxy_reflect.rs` is therefore left in place and
+#8108's premise is corrected on the issue rather than implemented.
+
+### Tests
+
+`dyn_ic_inline_store_barriers_a_reference_value` pins all three bookkeeping
+calls in the reference arm, their ABSENCE from the scalar arm, and — because an
+emitted block is not a reached block — the `br i1` INTO the reference arm.
+`dyn_ic_inline_store_keeps_its_semantic_fallback_for_reference_values` pins the
+tag as an arm SELECTOR rather than an entry gate, and the retained
+`js_put_value_set_dyn_ic` fallback. Four sabotages were run and all four are
+caught: dropping the write barrier, dropping the layout note + string addref,
+routing reference values back to `put.dynic.slow` (dead-IR arm), and leaking a
+barrier into the scalar arm.
+
+`test-files/test_gap_8108_dyn_ic_reference_store.ts` is the behavioural half:
+every value tag through one site, frozen / sealed / non-extensible / accessor /
+read-only receivers, an inherited setter, a Proxy trap, array and typed-array
+receivers, a mid-loop shape transition, a throwing RHS leaving no store,
+target→key→RHS evaluation order, and a volume section whose producer is reached
+through an `any[]` so it cannot be inlined into a rooted temp. Byte-identical
+to Node 26.5.1 under the default GC and under
+`PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1`, `PERRY_GEN_GC=0`,
+`PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1`, and `PERRY_WRITE_BARRIERS=0`.
+
+**Recorded because it is the reason the IR assertions exist**: a release build
+with the write barrier removed from the reference arm passes that entire
+behavioural matrix — all four GC modes, byte-identical output, exit 0. A
+dropped barrier is invisible to every runtime probe here, so the static IR test
+is the only thing that can say no.

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

@@ -835,32 +835,40 @@ fn lower_put_value_dyn_ic_inline(
     let is_ptr = ctx.block().icmp_eq(I64, &t_tag, "32765");
     let above = ctx.block().icmp_ugt(I64, &t_handle, "1048575");
     // Value tag: reference-creating stores (pointer 0x7FFD, string 0x7FFF,
-    // bigint 0x7FFA) leave the inline path before any store.
+    // bigint 0x7FFA) need the layout note / string-alias / write-barrier
+    // bookkeeping, so they SELECT the barriered store arm below rather than
+    // gating entry. #8108: they used to leave the inline path here, which sent
+    // every `o.k = <reference>` through the outlined helper — one cross-crate
+    // call per write that re-validated, in Rust, exactly the guards this block
+    // has already proved.
     let v_tag = ctx.block().lshr(I64, &v_bits, "48");
     let v_not_obj = ctx.block().icmp_ne(I64, &v_tag, "32765");
     let v_not_str = ctx.block().icmp_ne(I64, &v_tag, "32767");
     let v_not_big = ctx.block().icmp_ne(I64, &v_tag, "32762");
+    let mut v_scalar = ctx.block().and(I1, &v_not_obj, &v_not_str);
+    v_scalar = ctx.block().and(I1, &v_scalar, &v_not_big);
     // Zero key bits are the empty-way sentinel (and the JS number 0):
     // they must never reach the way compares.
     let k_nonzero = ctx.block().icmp_ne(I64, &k_bits, "0");
     let mut entry_ok = ctx.block().and(I1, &is_ptr, &above);
-    entry_ok = ctx.block().and(I1, &entry_ok, &v_not_obj);
-    entry_ok = ctx.block().and(I1, &entry_ok, &v_not_str);
-    entry_ok = ctx.block().and(I1, &entry_ok, &v_not_big);
     entry_ok = ctx.block().and(I1, &entry_ok, &k_nonzero);
 
     let guard_idx = ctx.new_block("put.dynic.guard");
     let ways_idx = ctx.new_block("put.dynic.ways");
     let way1_idx = ctx.new_block("put.dynic.way1");
     let way2_idx = ctx.new_block("put.dynic.way2");
     let store_idx = ctx.new_block("put.dynic.store");
+    let store_scalar_idx = ctx.new_block("put.dynic.store.scalar");
+    let store_ref_idx = ctx.new_block("put.dynic.store.ref");
     let slow_idx = ctx.new_block("put.dynic.slow");
     let merge_idx = ctx.new_block("put.dynic.merge");
     let guard_label = ctx.block_label(guard_idx);
     let ways_label = ctx.block_label(ways_idx);
     let way1_label = ctx.block_label(way1_idx);
     let way2_label = ctx.block_label(way2_idx);
     let store_label = ctx.block_label(store_idx);
+    let store_scalar_label = ctx.block_label(store_scalar_idx);
+    let store_ref_label = ctx.block_label(store_ref_idx);
     let slow_label = ctx.block_label(slow_idx);
     let merge_label = ctx.block_label(merge_idx);
     ctx.block().cond_br(&entry_ok, &guard_label, &slow_label);
@@ -942,18 +950,45 @@ fn lower_put_value_dyn_ic_inline(
         I64,
         &[(&s0, &ways_label), (&s1, &way1_label), (&s2, &way2_label)],
     );
-    let header_words =
-        (crate::target_layout::object_header_size_bytes(ctx.target_triple) / 8).to_string();
+    let header_bytes = crate::target_layout::object_header_size_bytes(ctx.target_triple);
+    let header_words = (header_bytes / 8).to_string();
     let slot_word = ctx.block().add(I64, &slot, &header_words);
     let obj_ptr = ctx.block().inttoptr(I64, &t_handle);
     let slot_ptr = ctx
         .block()
         .gep_inbounds(I64, &obj_ptr, &[(I64, &slot_word)]);
-    // GC_STORE_AUDIT(POINTER_FREE): the entry tag test proved the value is
+    ctx.block()
+        .cond_br(&v_scalar, &store_scalar_label, &store_ref_label);
+
+    ctx.current_block = store_scalar_idx;
+    // GC_STORE_AUDIT(POINTER_FREE): the tag test above proved the value is
     // not pointer/string/bigint — non-reference bits need no barrier.
     ctx.block().store(DOUBLE, v, &slot_ptr);
     ctx.block().br(&merge_label);
 
+    // #8108: the reference arm. Byte-for-byte the static write PIC's
+    // pointer-capable store (`lower_put_value_static_write_ic`'s hit block),
+    // reached under STRICTLY STRONGER conditions: the guards above are that
+    // PIC's guards, and this block additionally knows the value carries a
+    // reference tag, which the PIC only knows statically or not at all.
+    //
+    // No new rooting obligation. `t` is materialised BELOW every operand that
+    // can collect (see the call site's evaluation-order note), and the three
+    // bookkeeping helpers are `gc-leaf-function`, so nothing between the
+    // re-read and the store is a collection point.
+    ctx.current_block = store_ref_idx;
+    {
+        let slot_i32 = ctx.block().trunc(I64, &slot, I32);
+        let slot_offset = ctx.block().shl(I64, &slot, "3");
+        let fields_base = ctx.block().add(I64, &t_handle, &header_bytes.to_string());
+        let slot_addr = ctx.block().add(I64, &fields_base, &slot_offset);
+        let blk = ctx.block();
+        emit_jsvalue_slot_store_scalar_aware_on_block(
+            blk, &slot_ptr, v, &t_handle, &slot_i32, true, &t_bits, &slot_addr, true,
+        );
+        blk.br(&merge_label);
+    }
+
     ctx.current_block = slow_idx;
     let slow_result = ctx.block().call(
         DOUBLE,
@@ -969,9 +1004,14 @@ fn lower_put_value_dyn_ic_inline(
     ctx.block().br(&merge_label);
 
     ctx.current_block = merge_idx;
-    let result = ctx
-        .block()
-        .phi(DOUBLE, &[(v, &store_label), (&slow_result, &slow_label)]);
+    let result = ctx.block().phi(
+        DOUBLE,
+        &[
+            (v, &store_scalar_label),
+            (v, &store_ref_label),
+            (&slow_result, &slow_label),
+        ],
+    );
     Ok(result)
 }
 

crates/perry-codegen/tests/native_proof_regressions.rs

@@ -14551,6 +14551,169 @@ fn static_put_value_rejects_write_pic_when_rhs_can_allocate() {
     );
 }
 
+/// The body of the first block whose label starts with `label_prefix`.
+///
+/// Block labels carry per-function numeric suffixes (`put.dynic.store.ref.60`),
+/// so callers pass the stable prefix. A block header is a line-initial
+/// `label:`; lines that merely mention the label (branches, phis) are indented
+/// and skipped.
+fn dyn_ic_block_body<'a>(ir: &'a str, label_prefix: &str) -> Option<&'a str> {
+    let needle = format!("\n{label_prefix}");
+    let mut from = 0;
+    while let Some(rel) = ir[from..].find(&needle) {
+        let label_start = from + rel + 1;
+        let line_end = label_start + ir[label_start..].find('\n')?;
+        if ir[label_start..line_end].ends_with(':') {
+            let rest = &ir[line_end + 1..];
+            let end = match (rest.find("\n\n"), rest.find("\n}")) {
+                (Some(a), Some(b)) => a.min(b),
+                (a, b) => a.or(b).unwrap_or(rest.len()),
+            };
+            return Some(&rest[..end]);
+        }
+        from = line_end;
+    }
+    None
+}
+
+fn dyn_ic_reference_store_ir() -> String {
+    let object = 1u32;
+    let value = 2u32;
+    let key = 3u32;
+    let module = module_with_classes_and_params(
+        "dyn_ic_reference_store",
+        Vec::new(),
+        vec![
+            param(object, "object", Type::Any),
+            param(value, "value", Type::Any),
+        ],
+        Type::Any,
+        vec![
+            Stmt::Let {
+                id: key,
+                name: "key".to_string(),
+                ty: Type::String,
+                mutable: true,
+                init: Some(Expr::String("x".to_string())),
+            },
+            Stmt::Return(Some(Expr::PutValueSet {
+                target: Box::new(Expr::LocalGet(object)),
+                key: Box::new(Expr::LocalGet(key)),
+                value: Box::new(Expr::LocalGet(value)),
+                receiver: Box::new(Expr::LocalGet(object)),
+                strict: false,
+            })),
+        ],
+    );
+    compile_ir_for_module_with_opts(module, empty_opts()).unwrap()
+}
+
+/// #8108: a reference-tagged value stored through the inline dynamic-key write
+/// IC takes a BARRIERED inline arm instead of leaving the inline path.
+///
+/// Before this, the value tag gated ENTRY: `o[k] = <object|string|bigint>` was
+/// pushed straight to `put.dynic.slow`, i.e. one cross-crate
+/// `js_put_value_set_dyn_ic` call per write that re-validated in Rust exactly
+/// the guards the inline block had already proved. The tag now SELECTS an arm.
+///
+/// The reference arm is byte-for-byte the static write PIC's pointer-capable
+/// store (`emit_jsvalue_slot_store_scalar_aware_on_block`) reached under
+/// strictly stronger conditions, so this test pins all three bookkeeping calls
+/// — dropping any one of them is the #5094 / #7511 family of silent-stranding
+/// bugs, and none of them is visible to a runtime GC probe.
+#[test]
+fn dyn_ic_inline_store_barriers_a_reference_value() {
+    let ir = dyn_ic_reference_store_ir();
+
+    let scalar = dyn_ic_block_body(&ir, "put.dynic.store.scalar")
+        .unwrap_or_else(|| panic!("the non-reference store arm must survive:\n{ir}"));
+    let reference = dyn_ic_block_body(&ir, "put.dynic.store.ref").unwrap_or_else(|| {
+        panic!("a reference-tagged value must take an inline barriered arm:\n{ir}")
+    });
+    // An emitted block is not a reached block. Routing reference values back to
+    // `put.dynic.slow` leaves this block behind as dead IR, which every
+    // assertion below would happily inspect — so require the branch INTO it
+    // before believing anything it contains.
+    assert!(
+        ir.lines()
+            .any(|line| line.contains("br i1") && line.contains("%put.dynic.store.ref")),
+        "the reference arm must be a branch target, not dead IR:\n{ir}"
+    );
+
+    for helper in [
+        "js_string_addref_if_heap_string",
+        "js_gc_note_slot_layout_aware",
+        "js_write_barrier_slot",
+    ] {
+        assert!(
+            reference.contains(helper),
+            "the reference store arm must keep the full layout-note / string-alias / \
+             write-barrier path; missing {helper}:\n{reference}"
+        );
+    }
+    assert!(
+        reference.contains("store double"),
+        "the reference arm must still perform the slot store:\n{reference}"
+    );
+
+    // The scalar arm is the pre-#8108 IR: a bare store, no bookkeeping. A
+    // barrier appearing here would mean the tag test stopped discriminating.
+    assert!(
+        scalar.contains("store double"),
+        "the non-reference arm must still store:\n{scalar}"
+    );
+    for helper in [
+        "js_string_addref_if_heap_string",
+        "js_gc_note_slot_layout_aware",
+        "js_write_barrier_slot",
+    ] {
+        assert!(
+            !scalar.contains(helper),
+            "GC_STORE_AUDIT(POINTER_FREE): the non-reference arm proved the value carries \
+             no heap pointer, so it must not call {helper}:\n{scalar}"
+        );
+    }
+}
+
+/// #8108, the other half: admitting reference values inline must not cost the
+/// semantic fallback. Every guard failure and every way miss still reaches
+/// `js_put_value_set_dyn_ic`, which bottoms out at full `[[Set]]`.
+#[test]
+fn dyn_ic_inline_store_keeps_its_semantic_fallback_for_reference_values() {
+    let ir = dyn_ic_reference_store_ir();
+
+    assert!(
+        ir.contains("call double @js_put_value_set_dyn_ic("),
+        "the outlined helper must remain the miss path:\n{ir}"
+    );
+    // The arm is SELECTED by the value tag, not gated at entry: the branch into
+    // the two store arms is what proves a reference value can reach the inline
+    // store at all, rather than being diverted to the slow block above it.
+    let selector = ir
+        .lines()
+        .find(|line| {
+            line.contains("br i1")
+                && line.contains("%put.dynic.store.scalar")
+                && line.contains("%put.dynic.store.ref")
+        })
+        .unwrap_or_else(|| {
+            panic!("the value tag must SELECT a store arm, not gate inline entry:\n{ir}")
+        });
+    assert!(
+        selector.trim_start().starts_with("br i1"),
+        "expected a conditional branch into the two store arms, got: {selector}"
+    );
+    // Entry must no longer reject on the value tag. The three tag compares
+    // still exist (they build the selector), but the entry predicate is now
+    // receiver-shaped plus the empty-way sentinel only.
+    let entry = dyn_ic_block_body(&ir, "put.dynic.guard")
+        .unwrap_or_else(|| panic!("the receiver guard block must exist:\n{ir}"));
+    assert!(
+        entry.contains("call double @js_put_value_set_dyn_ic(") || ir.contains("%put.dynic.slow"),
+        "the receiver guard must still fall through to the outlined helper:\n{ir}"
+    );
+}
+
 #[test]
 fn nested_same_shape_object_writes_version_one_through_four_fields() {
     let objects = 1u32;