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

[proggeramlug]

The polymorphic-way block of the generic property-get IC re-derived every
value the way compares need, because #7883 routed all four of the guard
chain's failure edges into one block. Two of those four are receiver-validation
failures, and a receiver that fails them can never resolve a way — way_hit
ANDs is_object in. Sending just those two to a new pic.miss.cold makes
pic.miss dominated by pic.token, and the re-derivation becomes
deletable: token, token_nonnull and epoch_eq are already in scope there,
and is_object is statically true.

That block 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, and the runtime's own pic_prime_get doc names evalNode as the case —
it runs on nearly every read. LLVM agrees: it lays the miss path out as the
fall-through and the MRU hit as the taken branch. Profiled on
gc-handoff/apps/interp.ts, the single hottest instruction in the whole
program was the csel materialising max(field_count, INLINE_SLOT_FLOOR)
inside that recomputation — 4.65% of evalNode, itself 56.6% of the program.

The general form worth keeping: "this block is only reached on a miss" is not
an argument that it is cold when the cache it is missing is a 1-entry MRU in
front of a k-way set.

What changed

Codegen only — git diff --name-only touches nothing under perry-runtime /
perry-stdlib. Three pieces, all value-preserving:

1. pic.miss.cold. The small-handle and non-object edges land there, record
   the same two typed-feedback counters on the same edges, and go straight to
   js_object_get_field_ic_miss — which reproduces the whole receiver ladder
   anyway. Deleted from pic.miss: the select substituting a safe address for
   a small-handle receiver, four header loads + compares, the keys_array and
   parent_class_id loads, the token select, and a second pair of
   cache[2] / @PERRY_IC_EPOCH loads.
2. The cached-slot bound is a disjunction. slot < FLOOR || slot < field_count
   instead of slot < max(field_count, FLOOR). Identical predicate
   (x < max(a,b) ⟺ x < a ∨ x < b); the max had to be materialised, and its
   csel sat on the dependency chain out of the field_count load. LLVM folds
   the pair into cmp + ccmp, and the slot < 4 half does not depend on the
   load at all.
3. The way (token, slot) match reduces as a balanced tree. The left fold
   made way_slot a chain of PIC_WAYS dependent selects 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.

Measured

The dev host ran at load 30–200 all session (five concurrent agents), so wall
clock there cannot resolve this. /usr/bin/time -l reports instructions
retired on Apple Silicon; that is load-independent and reproduced to ±0.02%
across repeats. It answers "did the work actually go away?", which is this
change's question.

	base 6d9f12e60	this PR	delta
interp	12,929,608,815	11,451,033,738	−11.4%
iso_miss	18,498,517,372	17,065,407,361	−7.7%
evalNode emitted code	6516 insns	5348 insns	−17.9%

Isolated, piece 1 is interp −10.9% and pieces 2+3 a further −0.4%.

Every one of the 13 corpus programs whose binary changed retires fewer
instructions — there is no regression anywhere:

prog	base	this PR	delta
interp	12,925,713,250	11,491,067,754	−11.10%
iso_miss	18,497,320,448	17,099,158,730	−7.56%
retain	2,620,319,147	2,567,499,252	−2.02%
pipeline	3,427,425,834	3,366,497,803	−1.78%
deeplist	1,024,549,000	1,009,246,010	−1.49%
cycles	1,819,814,006	1,794,052,314	−1.42%
retain_wide	3,138,189,640	3,099,007,782	−1.25%
retain1	1,204,412,675	1,190,842,949	−1.13%
retain_wide1	1,166,210,387	1,154,449,713	−1.01%
asyncpipe	2,031,293,342	2,025,315,154	−0.29%
shapes	943,921,714	942,185,284	−0.18%
churn_read	448,871,523	448,363,363	−0.11%
churn	3,161,456,925	3,160,630,928	−0.03%

(The remaining 6 compile byte-identically.) Two programs looked like small
wall-clock regressions on the loaded host — churn_read +1.6% and shapes
+1.1%, both sub-millisecond — and both retire fewer instructions, which is what
"that was noise" looks like.

Wall clock on the same box, best-of-11 interleaved and exit-checked, read
interp −4.9% and iso_miss −3.2%. Those are not the number — the quiet
mini owns that.

Validation

• 19/19 corpus programs byte-exact against node --experimental-strip-types,
  exit 0, on both arms. 6 binaries are byte-identical to baseline (the ones with
  no generic PIC site); 13 differ, expected for a change that touches every
  generic property get.
• The measured binaries are the shipped code: all 19 re-compiled from branch
  HEAD after the last edit and cmp byte-identical — 19/19.
• Canary checksum 437840 misses 0.
• Whole corpus byte-exact under
  PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=200 PERRY_GC_VERIFY_EVACUATION=1,
  instrument shown live (38 retired page-sets on interp, 50 on iso_miss).
• cargo test --release -p perry-codegen --no-fail-fast — 26 suites, 1344
  passed, 0 failed.
• Differential gap-suite run over all 544 test_gap_*.ts: compiled and
  executed under both compilers, stdout + exit code compared. (cmp-ing the
  binaries is useless for a change this broad — it touches every generic
  property get, so nearly everything differs and the technique says nothing.)
  537 identical, 7 different, 0 skipped, 0 build failures — and all 7 differ
  from THEMSELVES on the baseline binary alone, verified by running each
  baseline binary two or three times: test_gap_console_methods prints
  console.time durations, and six http/fetch/net tests abort with a Tokio
  panic whose message carries a thread id. So: zero behavioural differences.
• cargo fmt --all -- --check, scripts/check_file_size.sh.
• Merges cleanly onto current main (checked with git merge-tree, past fix(indexing): classify string receivers behind array claims (#7891) #7904
  and perf(codegen): serve .length on an untyped string receiver inline — pipeline −8.1%, shapes −10.2% #7905).

Coverage

Three new codegen contracts in expr/property_get/tests.rs, each asserting a
consequence rather than a block name:

• pic_miss_reuses_the_token_blocks_values_instead_of_re_deriving_them — one
  generic read loads @PERRY_IC_EPOCH exactly once, emits no
  ptrtoint ptr @perry_ic_ (the small-handle sentinel), and has a
  pic.miss.cold block.
• cached_slot_bound_is_a_disjunction_not_a_materialised_max — no
  select …, i64 4, i64 %fc.
• way_slot_reduction_is_a_balanced_tree — one select …, i64 <slot>, i64 0
  per way; a left fold produces exactly one.

Sabotage, run, RED: restoring 6d9f12e60's generic_dispatch.rs turns all
three red and leaves the nine pre-existing property-get tests green, so they are
detecting this change and not something incidental.

Sized but deliberately NOT taken

Round 7's item 1 — "31 @perry_ic_N sites for ~14 distinct
(receiver, property) pairs, so intern the cache by
(function, receiver local, property name)" — is refuted, and the notes in
gc-handoff/INTERP8-NOTES.md record why. After #7833's prop_cse collapses the
guard runs, the surviving repeats (n.name, n.body, n.op) sit on mutually
exclusive arms that see different node kinds, so a shared cache would thrash
rather than CSE; and a shared global would not enable a load CSE anyway, because
every non-fast-load arm of the diamond ends in an opaque call and the merge block
carries a MemoryPhi.

Also left alone on purpose: emit_parent_may_need_remembering_check still emits
a seq_cst load of @PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT — an ldar
on aarch64 — while the runtime reads the same global Relaxed for the same
decision and codegen's own doc says "one relaxed load of a static". It is
visible in the profile and evalNode has 42 barrier sites, but the failure mode
is a missed insertion barrier, so that one wants the owner's call, not mine.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 418f63cf-c260-4ee5-9a5e-f7070e67058a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}