Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions changelog.d/7907-pic-miss-token-block-dominance.md
Original file line number Diff line number Diff line change
@@ -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.
196 changes: 123 additions & 73 deletions crates/perry-codegen/src/expr/property_get/generic_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -286,9 +304,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);
Expand All @@ -301,8 +326,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
Expand Down Expand Up @@ -378,7 +405,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.
Expand Down Expand Up @@ -467,9 +500,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()
Expand Down Expand Up @@ -514,62 +545,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",
Expand Down Expand Up @@ -612,16 +615,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,
Expand All @@ -636,20 +646,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);
Expand All @@ -664,6 +694,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(
Expand Down
Loading