perf(codegen): the PIC miss block re-derived the whole receiver ladder — interp −11.2% instructions (#7907)

* perf(codegen): make the PIC miss block dominated by the token block

The polymorphic-way block re-derived every value the way compares need --
four header loads, keys_array, parent_class_id, the token select and a
second pair of epoch loads -- because #7883 routed the two
receiver-validation failures into the same block, leaving those values live
on only some edges. Route those two edges to a new pic.miss.cold instead:
a receiver that fails them also fails way_hit, so the compares were dead
work for it, and with them gone pic.miss is dominated by pic.token and can
use its values directly.

Also spell the cached-slot bound as slot < FLOOR || slot < field_count
instead of materialising max(field_count, FLOOR); identical predicate, one
fewer dependency on the field_count load.

* perf(codegen): reduce the PIC way (token, slot) match as a balanced tree

The left fold made way_slot a chain of PIC_WAYS dependent selects whose
last node is the operand of the bounds compare that gates the branch out
of pic.ways. At most one way can hold a given token, so the association is
free to change.

Adds the codegen contracts for both halves of #7902.

* docs(codegen): reference PR #7907 and add the changelog fragment

Also fixes the pic.ways slice in way_slot_reduction_is_a_balanced_tree:
block labels carry a numeric suffix, so the search for the next block has
to start past this block's own label or it matches itself.

---------

Co-authored-by: Ralph Küpper <ralph@skelpo.com>

Author: proggeramlug

changelog.d/7907-pic-miss-token-block-dominance.md

@@ -0,0 +1,58 @@
+### Performance
+
+**The generic property-get IC's miss block re-derived the whole receiver ladder.**
+`interp.ts` retires **11.2% fewer instructions**, `iso_miss.ts` **7.6% fewer**, and
+`evalNode`'s emitted code shrinks from 6516 to 5348 instructions.
+
+#7883 routed all four of the guard chain's failure edges — small-handle receiver,
+non-object receiver, MRU token mismatch, cached slot out of bounds — into a single
+`pic.miss` block. That left `token`, `token_nonnull` and `epoch_eq` live on only
+some of those edges, so the block **recomputed** them: four header loads and
+compares, the `keys_array` and `parent_class_id` loads, the token select, a second
+pair of `cache[2]` / `@PERRY_IC_EPOCH` loads, and a `select` substituting a safe
+address for a small-handle receiver.
+
+It was justified as cold. It is not cold. On a site whose receiver rotates over
+more shapes than the MRU entry holds — the shape #7753's polymorphic ways exist
+for, i.e. every discriminated-union dispatch — that block runs on nearly every
+read. An `xctrace` profile of `gc-handoff/apps/interp.ts` put the **single hottest
+instruction in the whole program** inside the recomputation: the `csel`
+materialising `max(field_count, INLINE_SLOT_FLOOR)`, at 4.65% of `evalNode`, itself
+56.6% of the program. (`sample` cannot profile a deeply recursive function and
+attributes that time to the return addresses of `evalNode`'s own recursive calls.)
+
+Two of the four edges are receiver-validation failures, and a receiver that fails
+them can never resolve a way — `way_hit` ANDs `is_object` in, so the compares were
+dead work for it. Routing just those two to a new `pic.miss.cold` (which records the
+same two typed-feedback counters and goes straight to `js_object_get_field_ic_miss`)
+makes `pic.miss` **dominated by `pic.token`**, and every re-derived value is
+deletable: they are the values that block already computed, from the same memory
+with no intervening store, and `is_object` is statically true.
+
+Two smaller changes ride along, both value-preserving:
+
+* The cached-slot bound is spelled `slot < INLINE_SLOT_FLOOR || slot < field_count`
+  instead of `slot < max(field_count, INLINE_SLOT_FLOOR)`. Identical predicate
+  (`x < max(a, b)` ⟺ `x < a ∨ x < b`), but the `max` had to be materialised and its
+  `csel` sat on the dependency chain out of the `field_count` load; LLVM folds the
+  disjunction into `cmp` + `ccmp`, and the `slot < 4` half does not depend on the
+  load at all.
+* The way `(token, slot)` match reduces as a balanced tree rather than a left fold,
+  halving the depth of the `select` chain whose last node feeds the bounds compare
+  that gates the branch out of `pic.ways`. At most one way can hold a given token
+  (`pic_prime_get` evicts a duplicate before writing one, and a zero token is
+  excluded by `token_nonnull`), so reassociating is value-preserving.
+
+Codegen only — nothing under `perry-runtime` / `perry-stdlib` changes. Validated
+with all 19 `gc-handoff` corpus programs byte-exact against
+`node --experimental-strip-types` and exit 0, the `iso_miss` canary at
+`checksum 437840 misses 0`, the whole corpus byte-exact under
+`PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=200
+PERRY_GC_VERIFY_EVACUATION=1` with the instrument shown live (38 retired page-sets
+on `interp`, 50 on `iso_miss`), and a differential run of the whole `test_gap_*`
+suite compiled AND executed under both compilers with stdout and exit code compared.
+
+Three new codegen contracts in `expr/property_get/tests.rs` assert the consequences
+rather than the block names — one `@PERRY_IC_EPOCH` load per generic read, no
+small-handle sentinel `ptrtoint`, no materialised `max`, and one lane select per way
+— and all three go red against the pre-change lowering.

crates/perry-codegen/src/expr/property_get/generic_dispatch.rs

@@ -33,6 +33,24 @@ pub(crate) const PIC_WAYS: usize = 4;
 /// both skip them. Mirrors the runtime's `PIC_WAY_STATE`.
 pub(crate) const PIC_WAY_STATE: usize = 3;
 
+/// `slot < max(field_count, INLINE_SLOT_FLOOR)` — the per-receiver
+/// inline-capacity bound both the MRU hit path and the polymorphic ways apply
+/// to a cached slot (#6804).
+///
+/// Spelled as the equivalent disjunction `slot < FLOOR || slot < field_count`
+/// rather than as a `max` followed by one compare. The predicate is identical
+/// for every input (`x < max(a, b)` ⟺ `x < a ∨ x < b`), but the `max` had to be
+/// materialised — `mov w, #4` / `cmp` / `csel` — and that `csel` was the single
+/// hottest instruction in `interp.ts` (4.65% of `evalNode`, #7907), because it
+/// sits on the dependency chain out of the `field_count` load. The disjunction
+/// has no such node: LLVM folds the pair into `cmp` + `ccmp`, and the
+/// `slot < 4` half does not depend on the load at all.
+fn emit_slot_in_bounds(ctx: &mut FnCtx<'_>, slot: &str, field_count: &str) -> String {
+    let below_floor = ctx.block().icmp_ult(I64, slot, "4"); // INLINE_SLOT_FLOOR
+    let below_count = ctx.block().icmp_ult(I64, slot, field_count);
+    ctx.block().or(I1, &below_floor, &below_count)
+}
+
 /// The generic per-site monomorphic inline-cache dispatch for `obj.property`.
 /// This is the fall-through tail of the general catch-all arm: all earlier
 /// specializations have been ruled out.
@@ -329,9 +347,16 @@ pub(crate) fn lower_generic_property_get(
     // what the flat predicate computed there).
     let hit_idx = ctx.new_block("pic.hit");
     let miss_idx = ctx.new_block("pic.miss");
+    // #7907: the two receiver-validation failures get their own landing block
+    // so `pic.miss` is dominated by `pic.token`. See the comment on
+    // `pic.miss.cold` below for why that is the whole point of this split.
+    let cold_idx = ctx.new_block("pic.miss.cold");
+    let call_idx = ctx.new_block("pic.miss.call");
     let merge_idx = ctx.new_block("pic.merge");
     let hit_label = ctx.block_label(hit_idx);
     let miss_label = ctx.block_label(miss_idx);
+    let cold_label = ctx.block_label(cold_idx);
+    let call_label = ctx.block_label(call_idx);
     let merge_label = ctx.block_label(merge_idx);
     let hdr_idx = ctx.new_block("pic.recv_hdr");
     let hdr_label = ctx.block_label(hdr_idx);
@@ -344,8 +369,10 @@ pub(crate) fn lower_generic_property_get(
     // materialisation) in front of every real object read. The miss path
     // still substitutes the sentinel, because the way compares below load
     // `field_count` unconditionally.
-    // (edge labels are no longer needed: the miss block recomputes.)
-    ctx.block().cond_br(&is_real_ptr, &hdr_label, &miss_label);
+    // A small-handle receiver can never resolve a way (`way_hit` requires a
+    // real object), so it leaves for `pic.miss.cold` and never enters the
+    // block the ways live in.
+    ctx.block().cond_br(&is_real_ptr, &hdr_label, &cold_label);
     ctx.current_block = hdr_idx;
 
     // GcHeader sits 8 bytes before the user pointer; obj_type is the
@@ -421,7 +448,13 @@ pub(crate) fn lower_generic_property_get(
     // below: the keys load, the token select and the two epoch loads all
     // hang off the same predicate, so a non-object receiver used to execute
     // them before the flat `hit` could reject it.
-    ctx.block().cond_br(&is_object, &tok_label, &miss_label);
+    //
+    // #7907: the false edge goes to `pic.miss.cold`, not `pic.miss` — a
+    // receiver that is not a plain descriptor-free `ObjectHeader` fails
+    // `way_hit` by construction, so consulting the ways for it was always dead
+    // work, and keeping it out is what lets `pic.miss` reuse this block's
+    // values instead of re-deriving them.
+    ctx.block().cond_br(&is_object, &tok_label, &cold_label);
     ctx.current_block = tok_idx;
 
     // Load obj->keys_array at offset 16 of ObjectHeader.
@@ -510,9 +543,7 @@ pub(crate) fn lower_generic_property_get(
     let fc_ptr = ctx.block().inttoptr(I64, &fc_addr);
     let fc = ctx.block().load(I32, &fc_ptr);
     let fc64 = ctx.block().zext(I32, &fc, I64);
-    let fc_floor = ctx.block().icmp_ult(I64, &fc64, "4"); // INLINE_SLOT_FLOOR
-    let limit = ctx.block().select(I1, &fc_floor, I64, "4", &fc64);
-    let slot_in_bounds = ctx.block().icmp_ult(I64, &slot, &limit);
+    let slot_in_bounds = emit_slot_in_bounds(ctx, &slot, &fc64);
     let bounds_hit = ctx.new_block("pic.hit.load");
     let bounds_hit_label = ctx.block_label(bounds_hit);
     ctx.block()
@@ -557,62 +588,34 @@ pub(crate) fn lower_generic_property_get(
     // way hit still reports guard-fail + fallback-call exactly as it did when
     // it was a real miss — the feedback heuristics see an unchanged signal
     // (the site IS polymorphic; only the cost of that changed).
-    ctx.current_block = miss_idx;
-    // #7883: the guard chain now branches out at three points, so the values
-    // the polymorphic way compares consult are no longer live on every edge
-    // into this block — and phi-ing them would drag their materialisation
-    // (`cset`/`csinc` per value) back onto the hot path, which is the whole
-    // point of branching. They are recomputed here instead, from the SAME
-    // memory with no intervening store, so every one is bit-identical to
-    // what the pre-#7883 flat predicate computed. This block is cold — every
-    // path out of it either loads a way slot or calls the miss handler.
     //
-    // The small-handle sentinel substitution lives here for the same reason:
-    // the way compares load `field_count` unconditionally, and a native
-    // registry-id receiver reaches this block without ever being a pointer.
-    let cache_addr = ctx.block().ptrtoint(&cache_ref, I64);
-    let safe_obj_handle = ctx
-        .block()
-        .select(I1, &is_real_ptr, I64, &obj_handle, &cache_addr);
-    let m_gc_type_addr = ctx.block().sub(I64, &safe_obj_handle, "8");
-    let m_gc_type_ptr = ctx.block().inttoptr(I64, &m_gc_type_addr);
-    let m_gc_type = ctx.block().load(I8, &m_gc_type_ptr);
-    let m_gc_type_ok = ctx.block().icmp_eq(I8, &m_gc_type, "2");
-    let is_object = ctx.block().and(I1, &is_real_ptr, &m_gc_type_ok);
-    let m_magic_addr = ctx.block().add(I64, &safe_obj_handle, "12");
-    let m_magic_ptr = ctx.block().inttoptr(I64, &m_magic_addr);
-    let m_magic = ctx.block().load(I32, &m_magic_ptr);
-    let m_is_closure = ctx.block().icmp_eq(I32, &m_magic, "1129268819");
-    let m_not_closure = ctx.block().xor(I1, &m_is_closure, "true");
-    let is_object = ctx.block().and(I1, &is_object, &m_not_closure);
-    let m_ot_ptr = ctx.block().inttoptr(I64, &safe_obj_handle);
-    let m_ot = ctx.block().load(I32, &m_ot_ptr);
-    let m_ot_ok = ctx.block().icmp_eq(I32, &m_ot, "1");
-    let is_object = ctx.block().and(I1, &is_object, &m_ot_ok);
-    let m_res_addr = ctx.block().sub(I64, &safe_obj_handle, "6");
-    let m_res_ptr = ctx.block().inttoptr(I64, &m_res_addr);
-    let m_res = ctx.block().load(crate::types::I16, &m_res_ptr);
-    let m_has_desc = ctx.block().and(crate::types::I16, &m_res, "2048");
-    let m_no_desc = ctx.block().icmp_eq(crate::types::I16, &m_has_desc, "0");
-    let is_object = ctx.block().and(I1, &is_object, &m_no_desc);
-    let m_keys_addr = ctx.block().add(I64, &safe_obj_handle, "16");
-    let m_keys_ptr = ctx.block().inttoptr(I64, &m_keys_addr);
-    let m_keys = ctx.block().load(I64, &m_keys_ptr);
-    let m_pcid_addr = ctx.block().add(I64, &safe_obj_handle, "8");
-    let m_pcid_ptr = ctx.block().inttoptr(I64, &m_pcid_addr);
-    let m_pcid = ctx.block().load(I32, &m_pcid_ptr);
-    let m_pcid_rel = ctx.block().add(I32, &m_pcid, "-2147483648");
-    let m_is_stamp = ctx.block().icmp_ult(I32, &m_pcid_rel, "1073741824");
-    let m_pcid64 = ctx.block().zext(I32, &m_pcid, I64);
-    let m_id_token = ctx.block().or(I64, &m_pcid64, "4611686018427387904");
-    let token = ctx
-        .block()
-        .select(I1, &m_is_stamp, I64, &m_id_token, &m_keys);
-    let token_nonnull = ctx.block().icmp_ne(I64, &token, "0");
-    let m_cache_epoch_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "2")]);
-    let m_cache_epoch = ctx.block().load(I64, &m_cache_epoch_ptr);
-    let m_live_epoch = ctx.block().load(I64, "@PERRY_IC_EPOCH");
-    let epoch_eq = ctx.block().icmp_eq(I64, &m_cache_epoch, &m_live_epoch);
+    // # Why this block is DOMINATED by `pic.token` (#7907)
+    //
+    // Its only predecessors are `pic.token` (the MRU token did not match) and
+    // `pic.hit` (it matched but the cached slot is outside this receiver's
+    // inline capacity), and `pic.hit` is itself dominated by `pic.token`. So
+    // `token`, `token_nonnull` and `epoch_eq` — everything the way compares
+    // need — are already in scope here, and `is_object` is statically TRUE.
+    //
+    // #7883 could not rely on that: it routed the two receiver-validation
+    // failures here as well, which left the values live on only some edges, so
+    // the block **re-derived them** — four header loads, the `keys_array` and
+    // `parent_class_id` loads, the token select, a second pair of epoch loads,
+    // and a `select` substituting a safe address for a small-handle receiver.
+    // That was correct, and it was justified as cold. It is not cold: on a site
+    // whose receiver rotates over more shapes than the MRU entry holds — the
+    // shape #7753's ways exist for — this block runs on nearly every read, so
+    // the duplicate ladder sat on the hot path. Measured on `interp.ts`'s
+    // `evalNode`, the single hottest instruction in the whole program was the
+    // `csel` materialising `max(field_count, INLINE_SLOT_FLOOR)` *inside this
+    // recomputation*.
+    //
+    // Sending the two validation failures to `pic.miss.cold` instead is what
+    // establishes the dominance. Nothing about the predicate changed: a
+    // receiver that fails either check also fails `way_hit` (which ANDs
+    // `is_object` in), so it could never have resolved a way — the compares
+    // were dead work for it.
+    ctx.current_block = miss_idx;
     crate::expr::emit_typed_feedback_record_call(
         ctx.block(),
         "js_typed_feedback_record_guard_fail",
@@ -655,16 +658,23 @@ pub(crate) fn lower_generic_property_get(
     let way_state = ctx.block().load(I64, &state_ptr);
     let ways_live = ctx.block().icmp_sgt(I64, &way_state, "0");
     let ways_idx = ctx.new_block("pic.ways");
-    let call_idx = ctx.new_block("pic.miss.call");
     let ways_label = ctx.block_label(ways_idx);
-    let call_label = ctx.block_label(call_idx);
     ctx.block().cond_br(&ways_live, &ways_label, &call_label);
 
     ctx.current_block = ways_idx;
-    let mut way_hit = ctx.block().and(I1, &is_object, &epoch_eq);
-    way_hit = ctx.block().and(I1, &way_hit, &token_nonnull);
-    let mut way_any = String::from("false");
-    let mut way_slot = String::from("0");
+    // `is_object` is not ANDed in any more: it is statically true on every edge
+    // that reaches here (#7907 — see the dominance note above). `epoch_eq` and
+    // `token_nonnull` are the values `pic.token` computed, from the same memory
+    // with no intervening store, so the predicate is unchanged.
+    let mut way_hit = ctx.block().and(I1, &epoch_eq, &token_nonnull);
+    // Reduced as a BALANCED TREE, not as a left fold. At most one way can hold
+    // a given token (`pic_prime_get` evicts a duplicate before it writes one,
+    // and a zero token is excluded by `token_nonnull`), so the association is
+    // free to change — but the fold made `way_slot` a chain of `PIC_WAYS`
+    // dependent `csel`s whose last node is the operand of the bounds compare
+    // that gates the branch out of this block. On `interp.ts` that node was the
+    // hottest instruction in `evalNode` (#7907). The tree halves the chain.
+    let mut lanes: Vec<(String, String)> = Vec::with_capacity(PIC_WAYS);
     for w in 0..PIC_WAYS {
         let tok_ptr = ctx.block().gep(
             I64,
@@ -679,20 +689,40 @@ pub(crate) fn lower_generic_property_get(
             &[(I64, &(PIC_WAY_BASE + w * 2 + 1).to_string())],
         );
         let way_slot_val = ctx.block().load(I64, &slot_ptr);
-        way_slot = ctx.block().select(I1, &eq, I64, &way_slot_val, &way_slot);
-        way_any = ctx.block().or(I1, &way_any, &eq);
+        let lane_slot = ctx.block().select(I1, &eq, I64, &way_slot_val, "0");
+        lanes.push((eq, lane_slot));
+    }
+    while lanes.len() > 1 {
+        let mut merged: Vec<(String, String)> = Vec::with_capacity(lanes.len().div_ceil(2));
+        for pair in lanes.chunks(2) {
+            match pair {
+                [(a_any, a_slot), (b_any, b_slot)] => {
+                    let any = ctx.block().or(I1, a_any, b_any);
+                    let slot = ctx.block().select(I1, a_any, I64, a_slot, b_slot);
+                    merged.push((any, slot));
+                }
+                [single] => merged.push(single.clone()),
+                _ => unreachable!("chunks(2) yields one or two elements"),
+            }
+        }
+        lanes = merged;
     }
+    let (way_any, way_slot) = lanes
+        .pop()
+        .expect("PIC_WAYS is non-zero, so the reduction leaves exactly one lane");
     way_hit = ctx.block().and(I1, &way_hit, &way_any);
     // Same per-receiver inline-capacity bound the MRU hit path applies: a slot
     // primed from a larger-capacity sibling of the same shape must not drive a
     // raw load past this receiver's field region (#6804).
-    let way_fc_addr = ctx.block().add(I64, &safe_obj_handle, "12");
+    //
+    // The load is off `obj_handle` rather than the deleted small-handle
+    // sentinel, so it is the SAME address `pic.recv_hdr` already read for the
+    // closure-magic check and GVN folds the two together.
+    let way_fc_addr = ctx.block().add(I64, &obj_handle, "12");
     let way_fc_ptr = ctx.block().inttoptr(I64, &way_fc_addr);
     let way_fc = ctx.block().load(I32, &way_fc_ptr);
     let way_fc64 = ctx.block().zext(I32, &way_fc, I64);
-    let way_fc_floor = ctx.block().icmp_ult(I64, &way_fc64, "4"); // INLINE_SLOT_FLOOR
-    let way_limit = ctx.block().select(I1, &way_fc_floor, I64, "4", &way_fc64);
-    let way_in_bounds = ctx.block().icmp_ult(I64, &way_slot, &way_limit);
+    let way_in_bounds = emit_slot_in_bounds(ctx, &way_slot, &way_fc64);
     let way_ok = ctx.block().and(I1, &way_hit, &way_in_bounds);
     let way_load_idx = ctx.new_block("pic.way.load");
     let way_load_label = ctx.block_label(way_load_idx);
@@ -707,6 +737,26 @@ pub(crate) fn lower_generic_property_get(
     let way_end_label = ctx.block().label.clone();
     ctx.block().br(&merge_label);
 
+    // #7907: receiver-validation failure. `way_hit` requires a real pointer to
+    // a plain descriptor-free `ObjectHeader`, so a receiver that got here can
+    // never match a way — it goes straight to the handler, which reproduces the
+    // whole ladder anyway (proxy band, closure magic, buffer/typed-array
+    // registries, small-handle dispatch). The typed-feedback counters are the
+    // same two `pic.miss` records on the same edges, so the feedback signal is
+    // byte-identical to what the merged block reported.
+    ctx.current_block = cold_idx;
+    crate::expr::emit_typed_feedback_record_call(
+        ctx.block(),
+        "js_typed_feedback_record_guard_fail",
+        &[(I64, &feedback_site_id)],
+    );
+    crate::expr::emit_typed_feedback_record_call(
+        ctx.block(),
+        "js_typed_feedback_record_fallback_call",
+        &[(I64, &feedback_site_id)],
+    );
+    ctx.block().br(&call_label);
+
     // PIC miss: slow path with cache population.
     ctx.current_block = call_idx;
     let val_miss = ctx.block().call(