From ce73419bb82f7a023e1749fae3014d71f530f6ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 22 Sep 2026 10:55:01 +0000 Subject: [PATCH 1/2] perf(codegen): take the `TAG_HOLE` compare off the polymorphic way hit too #10826 removed the compare from the shape-gated MRU hit, on the proof that every successful delete moves the receiver's ShapeId, so an exact-id match proves the slot it names is live. The way path kept its copy, with the emitter's own note that removing it was "a separate, measured change". This is that change. A way pair needs no argument of its own. `pic_prime_get` is the ONLY writer of a way, and the only values it ever writes into one are `prev_tok` / `prev_slot` -- the pair that was sitting in the MRU entry. Every `(token, slot)` a way holds is therefore an MRU pair that aged out; the token it is compared against is the same receiver ShapeId word the MRU compare reads; and ShapeIds are never reused. Whatever makes the MRU pair safe to load without a hole check makes the way pair safe -- the entry did not become weaker by moving one word over. The two ways in which a way pair differs both narrow it: an overflow-encoded slot is refused entry to a way at all, and a way is consulted only after the MRU entry has missed. `pic.way.live` goes with the compare: the load block has nothing left to decide, so it ends in the load and a branch to the merge. The two codegen tests that pinned the compare as PRESENT now pin it as absent, and the block-set test drops `pic.way.live`. --- .../src/expr/property_get/generic_dispatch.rs | 33 +++++++++++-------- .../src/expr/property_get/tests.rs | 24 +++++++++----- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs index dc7fd73997..9baff44586 100644 --- a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs +++ b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs @@ -717,10 +717,7 @@ pub(crate) fn lower_generic_property_get( // The hole stays in the SLOT, so every path that reaches a slot WITHOUT a // shape-hit proof — the spill arm, a keys-array scan, `object_field_at`, // every walker — must still treat it as absent, and does. The way path - // below keeps its compare too: a way hit is also an exact-ShapeId proof, - // so it is redundant there by the same argument, but it sits on the - // polymorphic path and not on the hit this tower is sized by; removing it - // is a separate, measured change. + // below no longer compares either: the argument is stated there. // // #10826 keeps `PERRY_DELETE_SHAPE_TRANSITION=0` as a kill switch that // restores the id-preserving publish. With this compare gone that switch @@ -867,9 +864,7 @@ pub(crate) fn lower_generic_property_get( .pop() .expect("PIC_WAYS is non-zero, so the reduction leaves exactly one lane"); let way_load_idx = ctx.new_block("pic.way.load"); - let way_live_idx = ctx.new_block("pic.way.live"); let way_load_label = ctx.block_label(way_load_idx); - let way_live_label = ctx.block_label(way_live_idx); ctx.block().cond_br(&way_any, &way_load_label, &call_label); ctx.current_block = way_load_idx; @@ -878,14 +873,24 @@ pub(crate) fn lower_generic_property_get( let way_field_addr = ctx.block().add(I64, &way_base, &way_offset); let way_field_ptr = ctx.block().inttoptr(I64, &way_field_addr); let val_way = ctx.block().load(DOUBLE, &way_field_ptr); - let val_way_bits = ctx.block().bitcast_double_to_i64(&val_way); - let way_deleted = ctx - .block() - .icmp_eq(I64, &val_way_bits, crate::nanbox::TAG_HOLE_I64); - ctx.block() - .cond_br(&way_deleted, &call_label, &way_live_label); - - ctx.current_block = way_live_idx; + // The loaded value is the answer here too, for the reason the shape-gated + // hit above needs no `TAG_HOLE` compare (#10826: a successful delete + // ALWAYS moves the receiver's ShapeId, so an exact-id match proves the + // slot it names is live). + // + // A way pair is not a second kind of cache entry needing its own + // argument. `pic_prime_get` is the ONLY writer of a way, and the only + // values it ever writes into one are `prev_tok`/`prev_slot` — the pair + // that was sitting in the MRU entry. Every `(token, slot)` a way holds is + // therefore an MRU pair that aged out; the token it is compared against is + // the same receiver ShapeId word the MRU compare reads; and ShapeIds are + // never reused. Whatever makes the MRU pair safe to load without a hole + // check makes the way pair safe — the entry did not become weaker by + // moving one word over. + // + // The two ways in which a way pair differs from an MRU pair both narrow + // it: an overflow-encoded slot is refused entry to a way at all, and a way + // is consulted only after the MRU entry has already missed. let way_end_label = ctx.block().label.clone(); ctx.block().br(&merge_label); diff --git a/crates/perry-codegen/src/expr/property_get/tests.rs b/crates/perry-codegen/src/expr/property_get/tests.rs index d99e196bcf..ffd1ac0202 100644 --- a/crates/perry-codegen/src/expr/property_get/tests.rs +++ b/crates/perry-codegen/src/expr/property_get/tests.rs @@ -1055,10 +1055,10 @@ fn generic_property_get_slot_load_is_reached_only_through_every_guard() { // load in the hit block. #10826 made every successful delete a shape // transition, so a ShapeId hit proves the slot it names is live, and the // four-instruction hole check was the patch for exactly that operation. - // The compare is asserted PRESENT on the way path, which keeps it: that - // pins that the constant did not merely vanish from the IR, and that the - // way path's removal is the separate, measured change the emitter says - // it is. + // The way path is pinned the same way below: a way holds nothing but an + // aged MRU pair (`pic_prime_get` writes ways only from `prev_tok`/ + // `prev_slot`) compared against the same ShapeId word, so it carries the + // same proof. let hit_body = blocks .iter() .find(|(l, _)| *l == load_label) @@ -1080,9 +1080,16 @@ fn generic_property_get_slot_load_is_reached_only_through_every_guard() { .map(|(_, body)| body.join("\n")) .expect("the way load block"); assert!( - way_body.contains(crate::nanbox::TAG_HOLE_I64), - "the way path keeps its TAG_HOLE compare (removing it is a separate \ - change):\n{way_body}" + !way_body.contains(crate::nanbox::TAG_HOLE_I64), + "the way path must not compare the loaded slot against TAG_HOLE — a \ + way holds an aged MRU pair and its token is the same ShapeId word, \ + so a way hit carries the same liveness proof as an MRU hit:\n\ + {way_body}" + ); + assert!( + way_body.contains("load double") && way_body.contains("br label %"), + "the way load block must end in the slot load and an unconditional \ + branch to the merge:\n{way_body}" ); } @@ -1505,8 +1512,9 @@ fn the_generic_tower_is_two_calls_and_a_bounded_number_of_blocks() { // the polymorphic ways, deliberately still inline (#7753) "pic.miss", "pic.ways", + // `pic.way.live` is GONE with the way path's `TAG_HOLE` compare: the + // load block has nothing left to decide and branches to the merge. "pic.way.load", - "pic.way.live", // the inherited-read hook, on the never-primed edge out of // `pic.token.ways` and nowhere else (`js_inherited_read_cache_hit_f64`, // a leaf); a decline continues to the one exit From 20ec470d2fc569e91efc468888331fbf0db6b465 Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Tue, 22 Sep 2026 11:28:57 +0000 Subject: [PATCH 2/2] test(#10826): pin a delete observed through a POLYMORPHIC read site test_parity_delete_shape_transition.ts covered deletes observed through monomorphic sites only. Section 9 makes five shapes resident in ONE site's cache -- the MRU entry plus the four ways -- then deletes the read key off two of the receivers, one with the key on its prototype and one without, reads back through the same site, and re-adds. It passes on both arms of this branch, as it must: it guards the way path's JS-visible behaviour, it is not a discriminator for the compare removal. The discriminator is the pair of codegen tests, which fail on the base emitter. --- .../test_parity_delete_shape_transition.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test-files/test_parity_delete_shape_transition.ts b/test-files/test_parity_delete_shape_transition.ts index 5c212a7e25..2abd940aad 100644 --- a/test-files/test_parity_delete_shape_transition.ts +++ b/test-files/test_parity_delete_shape_transition.ts @@ -122,3 +122,29 @@ for (let round = 0; round < 60; round++) { show("i1", bad); show("i2", Object.keys(churn).length); show("i3", Object.keys(churn).sort().join(",")); + +// 9. delete observed through a POLYMORPHIC read site. Five shapes are made +// resident in one site's cache — the MRU entry plus the four ways — and then +// the read key is deleted off two of the receivers, one with the key on its +// prototype and one without, and read back through the SAME site. A way hit +// is an exact-ShapeId proof exactly as an MRU hit is, so a delete has to stop +// the key hitting on both; the emitted way path carries no other check. +const wproto: Record = { pv: "P" }; +function site(o: Record): unknown { return o.pv; } +const w1: Record = Object.create(wproto); w1.pv = "1"; +const w2: Record = { q1: 1, pv: "2" }; +const w3: Record = { q1: 1, q2: 2, pv: "3" }; +const w4: Record = { q1: 1, q2: 2, q3: 3, pv: "4" }; +const w5: Record = { q1: 1, q2: 2, q3: 3, q4: 4, pv: "5" }; +function wwarm(): void { + for (let i = 0; i < 200; i++) { site(w1); site(w2); site(w3); site(w4); site(w5); } +} +wwarm(); +show("j1", "" + site(w1) + site(w2) + site(w3) + site(w4) + site(w5)); +delete w1.pv; +delete w3.pv; +wwarm(); +show("j2", "" + site(w1) + "/" + site(w3) + "/" + site(w2) + site(w4) + site(w5)); +w1.pv = "1b"; w3.pv = "3b"; +wwarm(); +show("j3", "" + site(w1) + site(w2) + site(w3) + site(w4) + site(w5));