From a46769da0aa5cebda74dc2203024637e943c67f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 13:54:43 +0200 Subject: [PATCH 1/2] perf(codegen): collapse the generic property-get tower to two exits Per untyped `obj.prop`, `lower_generic_property_get` expanded 33 basic blocks, 191 pre-RS4GC IR instructions and SIX runtime call sites: an SSO arm, an INT32 class-ref arm, a nullish-throw arm, a non-object-receiver arm, an overflow-slot load, a deleted-slot miss, two Array-subclass named-prefix ladders, and the miss+prime. Every call is a statepoint, so `.text` and `.perry_gcmap` both scale with call sites x live GC values -- on @babel/parser that tower was 29% of all emitted IR across 6,487 sites. The hit is worth its bytes; the arms around it are not. What stays inline is what it was: the receiver-tag test, the small-handle test, the packed header kind/descriptor word, the packed-MRU compare, the overflow-bit test, the inline field load with its hole check, the polymorphic ways (#7753), and the `.length`/`.size` short-circuits. Everything else branches to one of two new runtime entries that reproduce the arms in the same ORDER and with the same cache-priming decisions: js_object_get_field_ic_nonptr(obj_bits, key, site_id) the receiver-tag ladder -- SSO, INT32 class ref, nullish TypeError, and the by-name fallback for every other tag. Takes no cache: none of these arms can prime one. js_object_get_field_ic_slow(obj_handle, key, cache_slot, packed) the heap-pointer arms -- the overflow slot (including its fallback to the THREE-argument miss entry, i.e. WITHOUT republishing the packed word, which is what the old helper did), the deleted-slot miss, the Array-subclass named-prefix proof, and the priming `get_field_ic_miss_impl`. Per site: 33 -> 14 tower blocks, 191 -> 106 IR instructions, 6 -> 2 calls. @babel/parser .text -14.4%, .perry_gcmap -11.7%, O0-fallback units unchanged. TWO exits and not one, measured. With the receiver-tag test and the small-handle test failing to the SAME block, SimplifyCFG folds them into one flat predicate -- `cmp; sete; cmp; setae; test; je` where the chain was `cmp; jne; cmp; ja` -- and every property-read HIT pays +4.00 instructions on a 10M-read monomorphic loop. That is #7883's flat-predicate cost arriving from the optimiser instead of from codegen. Distinct callees keep the chain branchy and let the unmasked receiver bits die in the entry block instead of living across the whole hit path. Three further shapes were needed to finish paying for the hit, each measured on the same 10M-read loop (instructions retired, mine vs base): * the field address is a typed `gep double` rather than `shl 3` + `add`. With an explicit shift the slot is the packed word's last use, so InstCombine folds `(packed >> 32) << 3` into `(packed >> 29) & mask` and isel pays a 10-byte `movabs` plus an `and`; as a GEP index there is no shift to fold and the scaled addressing mode survives. +3.00 -> +1.00 per hit. * the spill-buffer read moved into a `#[cold] #[inline(never)]` `overflow_arm`. Inlined, its `overflow_get` call forced the entry to save callee-saved registers, which gave it a frame and stopped the miss handler from being a sibling call. +49 -> +31 -> +20 per megamorphic miss (the first step was the exit split). * the named-prefix conjunction asks `ObjectMeta` first: that is one load from the header line the entry has already touched, against two dependent loads through the site's cache slot, and an object with no metadata record cannot carry the token. +20 -> +17 per miss. Final micro numbers, instructions retired per read vs base: monomorphic hit +1.00 (one `jmp`, because the hit's and the way's hole checks become congruent after their empty successors fold and SimplifyCFG tail-merges them -- fixing it needs `!prof` branch weights the IR builder cannot emit today), 4-shape polymorphic (the inline ways) -1.24, megamorphic miss +17.02. Registries updated with the new symbols: runtime declarations, the `cold` placement hint, the eh_mode throwing-callee assertion, gc_call_effects' allocating-helper list, and POLL_CAPABLE_RUNTIME in scripts/gc_root_dominance_check.py (both directions of its property-GET self-test fixture now run against both entries -- omitting them would silently re-open the #7154 GET hole, since the calls they replaced no longer appear at those sites). `test-files/test_gap_generic_get_one_exit_arms.ts` is the behavioural witness: every arm that moved out of line, checked against node's own output. PERRY_IC_DIAG counters are identical between the two toolchains on a deterministic fixture (41,216 misses over 5 sites, same per-reason split, same 20,216 primes, same fresh/armed/megamorphic distribution), and the gap-suite subset (10 filters, 290 tests) has byte-identical verdicts. --- .../src/codegen/trusted_box_callback_tests.rs | 4 +- crates/perry-codegen/src/eh_mode.rs | 16 +- .../src/expr/property_get/generic_dispatch.rs | 834 +++++++----------- .../src/expr/property_get/tests.rs | 299 +++++-- crates/perry-codegen/src/gc_call_effects.rs | 5 + crates/perry-codegen/src/module/linkage.rs | 5 +- .../src/runtime_decls/objects.rs | 11 + .../src/stmt/cached_field_index_return.rs | 6 +- .../src/stmt/element_shape_loop_tests.rs | 5 +- .../tests/native_proof_regressions.rs | 6 +- .../native_proof_regressions/pod_manifest.rs | 6 +- crates/perry-codegen/tests/typed_feedback.rs | 14 +- .../perry-runtime/src/object/field_get_set.rs | 6 + .../src/object/field_get_set/ic_miss.rs | 2 +- .../object/field_get_set/ic_miss/ic_slow.rs | 582 ++++++++++++ crates/perry-transform/src/prop_cse.rs | 12 +- docs/src/internals/gc-rooting-invariant.md | 8 +- scripts/gc_root_dominance_check.py | 63 +- scripts/shape_descriptor_census.py | 2 +- .../test_gap_generic_get_one_exit_arms.ts | 131 +++ 20 files changed, 1383 insertions(+), 634 deletions(-) create mode 100644 crates/perry-runtime/src/object/field_get_set/ic_miss/ic_slow.rs create mode 100644 test-files/test_gap_generic_get_one_exit_arms.ts diff --git a/crates/perry-codegen/src/codegen/trusted_box_callback_tests.rs b/crates/perry-codegen/src/codegen/trusted_box_callback_tests.rs index 03bbccdb48..89d929b797 100644 --- a/crates/perry-codegen/src/codegen/trusted_box_callback_tests.rs +++ b/crates/perry-codegen/src/codegen/trusted_box_callback_tests.rs @@ -347,7 +347,9 @@ fn additive_property_callback_gets_a_cold_deopting_private_body() { ); assert!( special.contains("pic.miss.call") - && special.contains("js_object_get_field_ic_miss") + // T1 renamed the property-GET slow path this cold arm reaches + // (`js_object_get_field_ic_miss_packed` -> the tower's object exit). + && special.contains("js_object_get_field_ic_slow") && special.contains("guarded_add.dynamic") && special.contains("versioned_callback.deopt.mark"), "both observable cold arms must poison the loop before fallback:\n{special}" diff --git a/crates/perry-codegen/src/eh_mode.rs b/crates/perry-codegen/src/eh_mode.rs index 0c436443d0..d694ff75a3 100644 --- a/crates/perry-codegen/src/eh_mode.rs +++ b/crates/perry-codegen/src/eh_mode.rs @@ -105,9 +105,19 @@ mod tests { #[test] fn cold_property_miss_is_still_throwing() { - let name = "js_object_get_field_ic_miss_packed"; - assert_eq!(crate::module::helper_decl_attrs(name), " cold"); - assert!(!callee_is_nothrow(name)); + for name in [ + "js_object_get_field_ic_miss_packed", + // T1: the generic-get tower's two slow exits carry the same `cold` + // placement hint — and the same ability to throw, now including + // the nullish-receiver TypeError the emitted arm used to raise + // inline. A `cold` helper that lost its invoke edge would silently + // swallow every `Cannot read properties of undefined`. + "js_object_get_field_ic_slow", + "js_object_get_field_ic_nonptr", + ] { + assert_eq!(crate::module::helper_decl_attrs(name), " cold", "{name}"); + assert!(!callee_is_nothrow(name), "{name}"); + } assert!(!callee_is_nothrow("js_object_get_field_ic_miss")); assert!(!callee_is_nothrow("unknown_runtime_helper")); } 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 28cb608ffe..889e059eaa 100644 --- a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs +++ b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs @@ -157,6 +157,32 @@ pub(crate) fn lower_generic_property_get( return Ok(val); } + // # Inline hit, two exits (T1) + // + // What stays inline below is exactly the hit: the receiver-tag test, the + // small-handle test, the packed header kind/descriptor word, the compact + // MRU compare, the overflow-bit test, the raw slot load with its hole + // check, and the bounded polymorphic ways. EVERY other arm this tower used + // to expand — the SSO receiver, the INT32 class ref, the nullish throw, the + // non-object receiver, the overflow load, the deleted-slot miss, the two + // Array-subclass named-prefix ladders, and the miss+prime — is now a branch + // to one of TWO calls that reproduce them in the same order: + // `js_object_get_field_ic_nonptr` for a receiver that is not a heap + // pointer, `js_object_get_field_ic_slow` for one that is. The split is not + // cosmetic — see `pget.recv_other` below for the +4 instructions per HIT + // that a single shared exit cost. + // + // The arms were not cheap to keep: ~37 basic blocks, ~177 pre-RS4GC IR + // instructions and 6-7 call sites per site, each call a statepoint whose + // live GC values are written into `.perry_gcmap`. On @babel/parser the + // tower was 29% of all emitted IR across 6,487 sites. It is also not a + // trade against the fast path: the hit sequence below is instruction for + // instruction what it was, with ONE deliberate difference — the + // overflow-bit test is spelled `== 0` with its successors swapped, so the + // guard-passing edge is the true edge like every other link in the chain + // (see `pic.hit` below) — and the ways still resolve `PIC_WAYS + 1` shapes + // without a call. + // // Issue #70/#73/#128: guard against non-pointer receivers // before the PIC deref. Tag-based check on the unmasked // NaN-box: real heap references have high-16-bits POINTER_TAG @@ -164,8 +190,9 @@ pub(crate) fn lower_generic_property_get( // to 0x7FFD; everything else (undefined/null/bool=0x7FFC, // int32=0x7FFE, bigint=0x7FFA, plain f64 like 0.0 globalThis // or 3.14, corrupt bit-patterns like 0x00FF_0000_0000 read as - // a BufferHeader) falls through to the invalid branch and - // returns undefined safely. + // a BufferHeader) falls through to the slow entry, whose tag + // ladder throws for a nullish receiver (#462) and routes every + // other tag to the by-name helper. // // Previously used a Darwin mimalloc heap-window check // (`> 2 TB && < 128 TB`). On aarch64-linux-android (issue @@ -176,41 +203,9 @@ pub(crate) fn lower_generic_property_get( // Tag check is platform-independent: same two LLVM ops // (`lshr` + `and`) + one `icmp`, branch-predicted taken. let obj_tag = ctx.block().lshr(I64, &obj_bits, "48"); - // SSO receiver fast path (Step 1.5 of SSO migration). - // SHORT_STRING_TAG = 0x7FF9 can't pass the POINTER/STRING - // check (its masked tag is 0x7FF9, not 0x7FFD) and we - // can't widen the mask because the PIC fast path's - // `*(obj_handle + 16)` would read arbitrary memory from - // the SSO data bits. Instead: check SSO explicitly first, - // route to a dedicated block that calls the SSO-aware - // `js_object_get_field_by_name_f64` runtime entry (which - // handles `.length` directly from the NaN-box length - // byte and returns `undefined` for other keys). - // v0.5.747: INT32-tagged class refs (top16 == 0x7FFE) used - // as PropertyGet receivers. Pre-fix these fell through to - // the invalid-recv path (returning undefined) because the - // 0xFFFD-masked tag check (0x7FFE & 0xFFFD = 0x7FFC, not - // 0x7FFD) treated them as non-pointer values. Drizzle's - // `is(value, type)` chain depends on `Cls.kind` reads through - // an Any-typed local. Refs #420 / #618 followup. - // - // Note: this also catches plain int32 numeric values (e.g. - // `(42).property`). The runtime helper's INT32-tag arm at - // js_object_get_field_by_name returns undefined for any - // class_id not registered in CLASS_DYNAMIC_PROPS, matching - // the previous behavior — pure ints have no static fields. let obj_tag_masked = ctx.block().and(I64, &obj_tag, "65533"); // 0xFFFD let is_valid = ctx.block().icmp_eq(I64, &obj_tag_masked, "32765"); // 0x7FFD - let sso_idx = ctx.new_block("pget.recv_sso"); - let pic_idx = ctx.new_block("pget.recv_ok"); - let invalid_idx = ctx.new_block("pget.recv_bad"); - let class_ref_idx = ctx.new_block("pget.recv_class_ref"); - let final_merge_idx = ctx.new_block("pget.recv_merge"); - let sso_label = ctx.block_label(sso_idx); - let pic_label = ctx.block_label(pic_idx); - let invalid_label = ctx.block_label(invalid_idx); - let class_ref_label = ctx.block_label(class_ref_idx); - let final_merge_label = ctx.block_label(final_merge_idx); + // `.length` on a receiver whose static type is not a proven string. // // The three-arm string-length dispatch in `property_get.rs` (SSO length @@ -221,11 +216,8 @@ pub(crate) fn lower_generic_property_get( // type (`rec.tag.length` where `rec` is an object-literal type, a JSON // `any`, an array element) therefore lands in this generic tower instead, // where a heap string can never be served: the PIC requires a - // GC_TYPE_OBJECT receiver by construction (#72), so EVERY such read misses - // to `js_object_get_field_ic_miss` and walks a ladder built for objects — - // closure-magic deref, buffer and typed-array registry probes, then - // `js_object_get_field_by_name`'s own dispatch, which decodes the key with - // `str::from_utf8` again before reaching the string arm. On `pipeline.ts` + // GC_TYPE_OBJECT receiver by construction (#72), so EVERY such read would + // otherwise call out and walk a ladder built for objects. On `pipeline.ts` // that one read was ~9% of total run time. // // Both string tags are disjoint from POINTER_TAG, so serving them here is @@ -233,11 +225,6 @@ pub(crate) fn lower_generic_property_get( // non-configurable, cannot be shadowed by an own property, and is exactly // what the runtime ladder computes. Everything else keeps the tower. let inline_string_length = property == "length"; - let strlen_heap_idx = if inline_string_length { - Some(ctx.new_block("pget.strlen_heap")) - } else { - None - }; // A dynamically typed `receiver.size` can still be served without the // object PIC when the live receiver is a native Map or Set. Both payloads // start with the same `u32 size` field, and their distinct GcHeader kinds @@ -246,52 +233,105 @@ pub(crate) fn lower_generic_property_get( // as `this.ctx.hooks.size` commonly lose their static Set type, while an // erased annotation alone must never authorize a native-layout load. let inline_collection_size = property == "size"; - let collection_size_idx = if inline_collection_size { - Some(ctx.new_block("pget.collection_size")) - } else { - None - }; - // #7883: the POINTER/STRING test goes FIRST, and the two rare tags are - // discriminated in a cold block off its false edge. The three tag classes - // are pairwise disjoint — `is_valid` is `(tag & 0xFFFD) == 0x7FFD`, true - // only for 0x7FFD/0x7FFF, while SSO is 0x7FF9 and an INT32 class ref is - // 0x7FFE — so testing them in any order gives the same routing. The old - // order (SSO, then class-ref, then pointer) put two 16-bit constant - // materialisations, two compares and two branches in front of every real - // object receiver: 13 instructions before the PIC on the path that is - // taken essentially always. Now it is `lshr` + `and` + `cmp` + branch. + + // A compact per-site word holds the exact ShapeId and slot for the last + // cacheable receiver. The lazily allocated full cache retains bounded + // polymorphic ways and the Array-subclass named-prefix proof. Both are + // materialised before the first branch now: the single slow exit takes + // them as arguments, and every failing guard reaches it. + let cache_name = overridden_cache_name(ctx, object, property) + .unwrap_or_else(|| allocate_property_cache(ctx)); + let cache_slot_ref = format!("@{cache_name}"); + // A compact atomic MRU removes the cache-pointer dependency on a hit. + // The full cache stays lazy and serves prefix/overflow/polymorphic misses. + let packed_site = ctx.ic_site_counter; + ctx.ic_site_counter += 1; + let packed_name = format!( + "{}_packed_get", + crate::expr::inline_cache_global_name(ctx, packed_site) + ); + ctx.typed_parse_rodata + .push(format!("@{packed_name} = private global i64 0, align 8")); + let packed_ref = format!("@{packed_name}"); + + let pic_idx = ctx.new_block("pget.recv_ok"); + // The object exit. Named `pic.miss.call` because that is what it still is: + // the block that calls out when the inline cache cannot serve the read. It + // now also lands every receiver-validation failure ON THE POINTER PATH, + // which is what collapses five of the six old call sites into it. + let call_idx = ctx.new_block("pic.miss.call"); + // The non-pointer exit. Its own block and its own callee, deliberately: + // when the receiver-tag test and the small-handle test failed to the SAME + // block, SimplifyCFG folded the two guards into one flat predicate + // (`cmp; sete; cmp; setae; test; je` for `cmp; jne; cmp; ja`) and every + // property-read HIT paid +4.00 instructions — measured on a 10M-read + // monomorphic loop, 1,231,521,261 -> 1,271,522,669 retired. That is + // #7883's flat-predicate cost arriving from the optimiser instead of from + // codegen. Distinct callees keep the chain branchy, and as a bonus the + // unmasked `obj_bits` dies here instead of staying live across the hit + // path for a call that might need it. let other_idx = ctx.new_block("pget.recv_other"); + let merge_idx = ctx.new_block("pget.recv_merge"); + let pic_label = ctx.block_label(pic_idx); + let call_label = ctx.block_label(call_idx); let other_label = ctx.block_label(other_idx); - let check_class_ref_idx = ctx.new_block("pget.check_class_ref"); - let check_class_ref_label = ctx.block_label(check_class_ref_idx); + let merge_label = ctx.block_label(merge_idx); + + // Typed-feedback bookkeeping is COMPILE-TIME gated (off by default), so + // the recording landing block only exists when it has something to record. + // Without it every failing guard branches straight to `pic.miss.call`; + // with it, the guard-fail/fallback-call pair is recorded on exactly the + // edges that recorded it before (`pic.miss.cold` and `pic.miss`), and the + // edges that recorded nothing — the non-pointer tags, the overflow slot, + // a way that turned out to hold a hole — still record nothing. + let cold_idx = + crate::expr::typed_feedback_emission_enabled().then(|| ctx.new_block("pic.miss.cold")); + let cold_label = cold_idx + .map(|idx| ctx.block_label(idx)) + .unwrap_or_else(|| call_label.clone()); + ctx.block().cond_br(&is_valid, &pic_label, &other_label); + + // Off the pointer path only ONE arm is still worth inlining: an SSO + // string's `.length`, which is a byte of the NaN-box itself. Every other + // non-pointer receiver — an SSO string with any other key, an INT32 class + // ref, `null`/`undefined`, a plain double — is the non-pointer entry's tag + // ladder, in the same order it evaluated them here. ctx.current_block = other_idx; - let is_sso = ctx.block().icmp_eq(I64, &obj_tag, "32761"); // 0x7FF9 - ctx.block() - .cond_br(&is_sso, &sso_label, &check_class_ref_label); - ctx.current_block = check_class_ref_idx; - let is_int32_class = ctx.block().icmp_eq(I64, &obj_tag, "32766"); // 0x7FFE - ctx.block() - .cond_br(&is_int32_class, &class_ref_label, &invalid_label); + let sso_arm = inline_string_length.then(|| { + let sso_idx = ctx.new_block("pget.recv_sso"); + let sso_label = ctx.block_label(sso_idx); + let nonptr_idx = ctx.new_block("pget.recv_nonptr"); + let nonptr_label = ctx.block_label(nonptr_idx); + let is_sso = ctx.block().icmp_eq(I64, &obj_tag, "32761"); // 0x7FF9 + ctx.block().cond_br(&is_sso, &sso_label, &nonptr_label); - // Class-ref dispatch: route through the runtime helper which - // detects INT32 class-ref bits and consults CLASS_DYNAMIC_PROPS - // for the static field / dynamic IIFE-set property / synthetic - // `constructor` lookup. Pass full obj_bits (NOT obj_handle — - // the runtime needs the unmasked top16 to detect the tag). - ctx.current_block = class_ref_idx; - let key_handle = emit_key_handle(ctx, &key_handle_global); - let class_ref_result = ctx.block().call( + // `.length` of an SSO string is the length byte in bits 40..47 of the + // NaN-box itself — the same extract `js_object_get_field_by_name_f64` + // performs, minus the call and the key decode. + ctx.current_block = sso_idx; + let len_shifted = ctx.block().lshr(I64, &obj_bits, "40"); + let len_byte = ctx.block().and(I64, &len_shifted, "255"); + let sso_val = ctx.block().uitofp(I64, &len_byte, DOUBLE); + let sso_end_label = ctx.block().label.clone(); + ctx.block().br(&merge_label); + ctx.current_block = nonptr_idx; + (sso_val, sso_end_label) + }); + // The non-pointer exit: SSO / INT32 class ref / nullish throw / everything + // else, in that order, behind one call that needs no cache. + let nonptr_key_handle = emit_key_handle(ctx, &key_handle_global); + let val_nonptr = ctx.block().call( DOUBLE, - "js_typed_feedback_object_get_field_by_name_f64", + "js_object_get_field_ic_nonptr", &[ - (I64, &feedback_site_id), (I64, &obj_bits), - (I64, &key_handle), + (I64, &nonptr_key_handle), + (I64, &feedback_site_id), ], ); - let class_ref_end_label = ctx.block().label.clone(); - ctx.block().br(&final_merge_label); + let nonptr_end_label = ctx.block().label.clone(); + ctx.block().br(&merge_label); ctx.current_block = pic_idx; let observed_key = key_handle_observed.clone().unwrap_or_default(); @@ -309,7 +349,8 @@ pub(crate) fn lower_generic_property_get( // typed-feedback observation on purpose: the site keeps recording every // receiver it sees, so a mixed object/string site cannot be mis-profiled // as monomorphic-object by the arm that is no longer traced here. - if let Some(heap_idx) = strlen_heap_idx { + let strlen_heap_idx = inline_string_length.then(|| { + let heap_idx = ctx.new_block("pget.strlen_heap"); let strlen_heap_label = ctx.block_label(heap_idx); let not_string_idx = ctx.new_block("pget.recv_obj"); let not_string_label = ctx.block_label(not_string_idx); @@ -319,24 +360,8 @@ pub(crate) fn lower_generic_property_get( ctx.block() .cond_br(&is_heap_string, &strlen_heap_label, ¬_string_label); ctx.current_block = not_string_idx; - } - - // A compact per-site word holds the exact ShapeId and slot for the last - // cacheable receiver. The lazily allocated full cache retains bounded - // polymorphic ways and the Array-subclass named-prefix proof. - let cache_name = overridden_cache_name(ctx, object, property) - .unwrap_or_else(|| allocate_property_cache(ctx)); - // A compact atomic MRU removes the cache-pointer dependency on a hit. - // The full cache stays lazy and serves prefix/overflow/polymorphic misses. - let packed_site = ctx.ic_site_counter; - ctx.ic_site_counter += 1; - let packed_name = format!( - "{}_packed_get", - crate::expr::inline_cache_global_name(ctx, packed_site) - ); - ctx.typed_parse_rodata - .push(format!("@{packed_name} = private global i64 0, align 8")); - let packed_ref = format!("@{packed_name}"); + heap_idx + }); // Issue #72: validate the receiver is actually a GC_TYPE_OBJECT // before reading its ShapeId. The receiver @@ -346,20 +371,19 @@ pub(crate) fn lower_generic_property_get( // analysis can't prove `obj.rowsRaw` is an Array — the outer // PropertyGet falls into this generic dispatch) hands the array's // pointer to this PIC. Reading an ObjectHeader ShapeId from that payload - // would be invalid. The slow `js_object_get_field_by_name` - // already routes by `gc_type` (handles Array.length, String.length, - // Set.size, Buffer.length, Error.message, etc.), so funneling - // non-OBJECT receivers through the miss handler fixes correctness - // without giving up the PIC for real objects. + // would be invalid. The slow entry already routes by `gc_type` (it handles + // Array.length, String.length, Set.size, Buffer.length, Error.message, + // etc. through the same miss handler), so funneling non-OBJECT receivers + // to it fixes correctness without giving up the PIC for real objects. // // Issue #340/#341: small-handle guard. Receivers from // native modules (axios, fastify, ioredis, better-sqlite3, // ...) are NaN-boxed POINTER values whose lower-48 is a // small registry id (1, 2, 3, ...). The PIC fast path // below deref's `obj_handle - 8` for the GcHeader byte - // and `obj_handle + 8` for the ShapeId slot — both + // and `obj_handle + 4` for the ShapeId — both // SIGSEGV when `obj_handle` is a small int. Funnel - // small-handle receivers through the slow path so they + // small-handle receivers through the slow entry so they // reach the runtime's `HANDLE_PROPERTY_DISPATCH` table // (axios `r.status` / `r.data`, fastify `req.query` / // `req.params`, etc.). @@ -368,60 +392,43 @@ pub(crate) fn lower_generic_property_get( // detection (raw_ptr < 0x100000). let is_real_ptr = ctx.block().icmp_ugt(I64, &obj_handle, "1048575"); // 0x100000 - // #7883: the hit/miss/merge blocks are minted here so the guard chain - // below can BRANCH OUT to the miss on the first failing predicate - // instead of AND-ing eight of them into one flat `hit`. LLVM if-converts - // a flat predicate, so every receiver paid every load and every compare - // even after the very first one had - // already decided the answer. Each group now ends in its own `cond_br`; - // the miss block reconstructs what the polymorphic-way compares need - // through phis (`false`/`0` on the early-exit edges, which is exactly - // what the flat predicate computed there). - let hit_idx = ctx.new_block("pic.hit"); - let hit_live_idx = ctx.new_block("pic.hit.live"); - let prefix_guard_idx = ctx.new_block("pic.prefix.guard"); - let prefix_meta_idx = ctx.new_block("pic.prefix.meta"); - let prefix_token_idx = ctx.new_block("pic.prefix.token"); - let prefix_hit_idx = ctx.new_block("pic.prefix.hit"); - let desc_classify_idx = ctx.new_block("pic.desc.classify"); - let desc_prefix_guard_idx = ctx.new_block("pic.desc.prefix.guard"); - let desc_prefix_meta_idx = ctx.new_block("pic.desc.prefix.meta"); - let desc_prefix_token_idx = ctx.new_block("pic.desc.prefix.token"); - let desc_prefix_hit_idx = ctx.new_block("pic.desc.prefix.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 hit_live_label = ctx.block_label(hit_live_idx); - let prefix_guard_label = ctx.block_label(prefix_guard_idx); - let prefix_meta_label = ctx.block_label(prefix_meta_idx); - let prefix_token_label = ctx.block_label(prefix_token_idx); - let prefix_hit_label = ctx.block_label(prefix_hit_idx); - let desc_classify_label = ctx.block_label(desc_classify_idx); - let desc_prefix_guard_label = ctx.block_label(desc_prefix_guard_idx); - let desc_prefix_meta_label = ctx.block_label(desc_prefix_meta_idx); - let desc_prefix_token_label = ctx.block_label(desc_prefix_token_idx); - let desc_prefix_hit_label = ctx.block_label(desc_prefix_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); + // #7883: the guard chain BRANCHES OUT to the slow exit on the first + // failing predicate instead of AND-ing eight of them into one flat `hit`. + // LLVM if-converts a flat predicate, so every receiver paid every load and + // every compare even after the very first one had already decided the + // answer. let hdr_idx = ctx.new_block("pic.recv_hdr"); let hdr_label = ctx.block_label(hdr_idx); let tok_idx = ctx.new_block("pic.token"); let tok_label = ctx.block_label(tok_idx); + let hit_idx = ctx.new_block("pic.hit"); + // The inline hit's LIVE edge goes straight to the merge unless typed + // feedback has a guard-pass record to put on it. An empty `pic.hit.live` + // is congruent with `pic.way.live`, so SimplifyCFG tail-merges the two and + // the hit path pays a `jmp` to the survivor instead of falling through. + let hit_live_idx = + crate::expr::typed_feedback_emission_enabled().then(|| ctx.new_block("pic.hit.live")); + // The hole edge gets its own landing block, as it did before T1. Note what + // that does and does not buy, measured: `pic.hit.inline` and `pic.way.load` + // end in the same three instructions (bitcast, TAG_HOLE compare, branch), + // and SimplifyCFG folds this block and `pic.way.live` away, so the two + // tails end up congruent and get merged anyway — the hit still reaches the + // shared tail by a `jmp`, and removing this block changed the 10M-read + // monomorphic loop by exactly 0 instructions. It is kept because it keeps + // the emitted hole edge structurally distinct from the way path's, which is + // the shape every reader of this tower since #9287 expects; the remaining + // jump needs branch weights (`!prof`) to fix, which the IR builder has no + // way to emit today. + let deleted_idx = ctx.new_block("pic.hit.deleted"); + let miss_idx = ctx.new_block("pic.miss"); + let hit_label = ctx.block_label(hit_idx); + let deleted_label = ctx.block_label(deleted_idx); + let miss_label = ctx.block_label(miss_idx); // Small-handle receivers (native-module registry ids) must never be // dereferenced. Pre-#7883 they were kept out of the loads by selecting a // sentinel address and AND-ing `is_real_ptr` into `hit`; the branch does // the same job without putting a `select` (and the sentinel's address // materialisation) in front of every real object read. - // 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; @@ -440,33 +447,42 @@ pub(crate) fn lower_generic_property_get( "aarch64" | "arm64" | "arm64_32" | "x86_64" | "i686" | "i386" | "riscv64" | "wasm32" ) .then(|| ctx.block().load(I32, &gc_type_ptr)); - let gc_type = match &packed_header { - Some(word) => ctx.block().trunc(I32, word, I8), - None => ctx.block().load(I8, &gc_type_ptr), - }; + // The separate kind BYTE is needed only by the native-endian descriptor + // guard and by the Map/Set split. With the packed header word the kind is + // already inside it, so materialising the byte anyway would leave a dead + // `trunc` on every hit. + let gc_type = + (inline_collection_size || packed_header.is_none()).then(|| match &packed_header { + Some(word) => ctx.block().trunc(I32, word, I8), + None => ctx.block().load(I8, &gc_type_ptr), + }); // `MapHeader` and `SetHeader` both begin with `size: u32`. A native // collection is not an ObjectHeader and can never hit this PIC, so split // it off immediately after the already-required GC-kind load. The generic // miss handler recognizes the same two kinds before ordinary object // lookup; this only removes that repeated classification and call ladder. - if let Some(collection_idx) = collection_size_idx { + let collection_size_idx = inline_collection_size.then(|| { + let collection_idx = ctx.new_block("pget.collection_size"); let collection_label = ctx.block_label(collection_idx); let object_check_idx = ctx.new_block("pic.recv_object_check"); let object_check_label = ctx.block_label(object_check_idx); - let is_map = ctx.block().icmp_eq(I8, &gc_type, "8"); // GC_TYPE_MAP - let is_set = ctx.block().icmp_eq(I8, &gc_type, "12"); // GC_TYPE_SET + let kind = gc_type + .clone() + .expect("the GC-kind byte is materialised whenever `.size` is inlined"); + let is_map = ctx.block().icmp_eq(I8, &kind, "8"); // GC_TYPE_MAP + let is_set = ctx.block().icmp_eq(I8, &kind, "12"); // GC_TYPE_SET let is_collection = ctx.block().or(I1, &is_map, &is_set); ctx.block() .cond_br(&is_collection, &collection_label, &object_check_label); ctx.current_block = object_check_idx; - } - let is_object_kind = ctx.block().icmp_eq(I8, &gc_type, "2"); + collection_idx + }); // Closures and RegExp values have distinct GC kinds. Every // `GC_TYPE_OBJECT` payload is therefore an ObjectHeader and its ShapeId is // the remaining exact layout discriminator. - + // // #6080: a receiver that has ever had a property/accessor descriptor // installed (`Object.defineProperty`) needs descriptor-aware dispatch — // an accessor must fire on reads, a non-writable slot must reject stores. @@ -476,10 +492,18 @@ pub(crate) fn lower_generic_property_get( // hit path would return the raw slot and bypass the getter entirely. // OBJ_FLAG_HAS_DESCRIPTORS is bit 11 of reserved (bit 27 of the // little-endian header word). Ignore gc_flags and every other flag. + // A descriptor-bearing receiver leaves for the slow exit, which keeps the + // Array-subclass named-prefix proof that used to be a second inline ladder + // here: it is the one case where an unrelated `length` descriptor must not + // make every declared field permanently generic. let is_plain_kind = if let Some(word) = &packed_header { let kind_and_desc = ctx.block().and(I32, word, "134217983"); // 0x080000ff ctx.block().icmp_eq(I32, &kind_and_desc, "2") } else { + let kind = gc_type + .as_ref() + .expect("the GC-kind byte is materialised on native-endian targets"); + let is_object_kind = ctx.block().icmp_eq(I8, kind, "2"); let reserved_addr = ctx.block().sub(I64, &obj_handle, "6"); let reserved_ptr = ctx.block().inttoptr(I64, &reserved_addr); let reserved = ctx.block().load(crate::types::I16, &reserved_ptr); @@ -487,33 +511,12 @@ pub(crate) fn lower_generic_property_get( let no_desc = ctx.block().icmp_eq(crate::types::I16, &has_desc, "0"); ctx.block().and(I1, &is_object_kind, &no_desc) }; - // Resolve the full cache only after the compact MRU declines a read. - // Loading it here keeps an unused global load on every successful hit. - // Each cold entry retains its own non-null proof before dereferencing. - let cache_slot_ref = format!("@{cache_name}"); let is_plain_object = ctx.block().and(I1, &is_plain_kind, &packed_present); // Validate kind, descriptor policy, and initialized MRU before reading - // ObjectHeader's ShapeId. Header failures bypass the shape ways; the - // descriptor-aware named-prefix path keeps its own full-cache guard. + // ObjectHeader's ShapeId. ctx.block() - .cond_br(&is_plain_object, &tok_label, &desc_classify_label); - - // A descriptor-bearing GC_TYPE_OBJECT normally goes cold. Array-subclass - // `length` is the important exception: runtime can prove that descriptor - // is unrelated to all class-declared named fields and arm word 2. Keep - // this classification off the ordinary descriptor-free hit path. - ctx.current_block = desc_classify_idx; - // #9708: the descriptor prefix path reads cache word 2, so it needs the - // full-cache non-null proof as the shape-miss path; without a cache it is - // simply a cold miss. - let desc_cache = crate::expr::emit_inline_cache_slot(ctx, &cache_name); - let desc_object_with_cache = ctx.block().and(I1, &is_object_kind, &desc_cache.present); - ctx.block().cond_br( - &desc_object_with_cache, - &desc_prefix_guard_label, - &cold_label, - ); + .cond_br(&is_plain_object, &tok_label, &cold_label); ctx.current_block = tok_idx; // The receiver token is derived solely from its authoritative ShapeId. @@ -539,11 +542,14 @@ pub(crate) fn lower_generic_property_get( let token_miss_label = ctx.block_label(token_miss_idx); ctx.block() .cond_br(&token_eq, &hit_label, &token_miss_label); + + // Every way load still requires a resolved full cache. A site that has + // never primed has no cache, so there is nothing to compare against and + // the read goes straight out. ctx.current_block = token_miss_idx; - // Every subsequent prefix/way load still requires a resolved full cache. let token_cache = crate::expr::emit_inline_cache_slot(ctx, &cache_name); ctx.block() - .cond_br(&token_cache.present, &prefix_guard_label, &cold_label); + .cond_br(&token_cache.present, &miss_label, &cold_label); // `js_object_get_field_ic_miss` primes only slots below the descriptor's // exact `live_inline_slot_count`. ShapeIds are never reused, so an exact @@ -555,216 +561,86 @@ pub(crate) fn lower_generic_property_get( // #9287: the primed slot word may carry IC_SLOT_OVERFLOW_BIT (1 << 30) — // the field lives past the inline region, in the object's spill buffer, // and the inline `obj + header + slot*8` arithmetic below must not run on - // it. Such hits route through `js_object_get_field_ic_overflow_load`, - // which loads through `overflow_get` and falls back to the full miss - // handler on a tombstoned slot. Sites whose field is inline never see the - // bit, so this branch predicts perfectly for them. The polymorphic WAYS - // never hold an encoded slot (`pic_prime_get` refuses to cascade one), so - // only this MRU path needs the check. - let ovf_idx = ctx.new_block("pic.hit.overflow"); + // it. Such hits leave for the slow entry, which re-derives the same MRU + // pair and performs `js_object_get_field_ic_overflow_load`'s `overflow_get` + // (falling back to the full miss handler on a tombstoned slot, without the + // packed republication that helper also did not do). Sites whose field is + // inline never see the bit, so this branch predicts perfectly for them. + // The polymorphic WAYS never hold an encoded slot (`pic_prime_get` refuses + // to cascade one), so only this MRU path needs the check. + // + // Tested as `== 0` rather than `!= 0` so the guard-PASSING edge is the TRUE + // edge, exactly like every other link in this chain. That is not cosmetic: + // `generic_property_get_slot_load_is_reached_only_through_every_guard` + // walks the CFG backwards from the slot load and requires every edge on the + // way to be a true edge, which is what makes a swapped `cond_br` — running + // the raw load when a guard FAILS — turn it red. Before T1 the walk reached + // the load through `pic.hit.overflow` (whose key-handle load also matched + // "load double") and never evaluated this branch's polarity at all. let inline_hit_idx = ctx.new_block("pic.hit.inline"); - let ovf_label = ctx.block_label(ovf_idx); let inline_hit_label = ctx.block_label(inline_hit_idx); let ovf_bits = ctx.block().and(I64, &slot, "1073741824"); // 1 << 30 - let is_ovf = ctx.block().icmp_ne(I64, &ovf_bits, "0"); - ctx.block().cond_br(&is_ovf, &ovf_label, &inline_hit_label); - - ctx.current_block = ovf_idx; - let ovf_key_handle = emit_key_handle(ctx, &key_handle_global); - let ovf_slot_i32 = ctx.block().trunc(I64, &slot, I32); - let val_ovf = ctx.block().call( - DOUBLE, - "js_object_get_field_ic_overflow_load", - &[ - (I64, &obj_handle), - (I64, &ovf_key_handle), - (I32, &ovf_slot_i32), - (PTR, &cache_slot_ref), - ], - ); - let ovf_end_label = ctx.block().label.clone(); - ctx.block().br(&merge_label); + let is_inline_slot = ctx.block().icmp_eq(I64, &ovf_bits, "0"); + ctx.block() + .cond_br(&is_inline_slot, &inline_hit_label, &call_label); ctx.current_block = inline_hit_idx; - let offset = ctx.block().shl(I64, &slot, "3"); // arm64_32 watchOS: the object fields region begins at // `size_of::()` past the user pointer — 16 on LP64 and // padded ILP32 since #8047. Derive it from the target triple. let obj_header_size = crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string(); let base = ctx.block().add(I64, &obj_handle, &obj_header_size); - let field_addr = ctx.block().add(I64, &base, &offset); - let field_ptr = ctx.block().inttoptr(I64, &field_addr); + let base_ptr = ctx.block().inttoptr(I64, &base); + // A typed GEP rather than `shl 3` + `add`: the slot is the LAST use of the + // packed word here, so with an explicit `shl` InstCombine folds + // `(packed >> 32) << 3` into `(packed >> 29) & 0x5fffffff8` and isel pays a + // 10-byte `movabs` plus an `and` for the mask. As a GEP index there is no + // `shl` to fold, so the scaled addressing mode survives — `shr $32` plus + // `(base,idx,8)`, which is what the pre-T1 tower emitted (it kept the plain + // `shr` only because its overflow block consumed the same value). + let field_ptr = ctx.block().gep(DOUBLE, &base_ptr, &[(I64, &slot)]); let val_hit = ctx.block().load(DOUBLE, &field_ptr); let val_hit_bits = ctx.block().bitcast_double_to_i64(&val_hit); let hit_deleted = ctx .block() .icmp_eq(I64, &val_hit_bits, crate::nanbox::TAG_HOLE_I64); - let deleted_idx = ctx.new_block("pic.hit.deleted"); - let deleted_label = ctx.block_label(deleted_idx); + // A hole is a field deleted since priming. It took the ordinary miss + // before (via `pic.miss`, whose way compares can never match a token the + // MRU entry still holds — `pic_prime_get` evicts a duplicate before it + // writes one), and it takes the same ordinary miss now, recording the same + // guard-fail/fallback-call pair on the way. + let hit_live_label = hit_live_idx + .map(|idx| ctx.block_label(idx)) + .unwrap_or_else(|| merge_label.clone()); ctx.block() .cond_br(&hit_deleted, &deleted_label, &hit_live_label); - ctx.current_block = deleted_idx; - let deleted_cache = crate::expr::emit_inline_cache_slot(ctx, &cache_name); - ctx.block() - .cond_br(&deleted_cache.present, &miss_label, &cold_label); - - ctx.current_block = hit_live_idx; - crate::expr::emit_typed_feedback_record_call( - ctx.block(), - "js_typed_feedback_record_guard_pass", - &[(I64, &feedback_site_id)], - ); - let hit_end_label = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - // An object-backed Array subclass changes exact ShapeId on every numeric - // push/pop because its elements live in ordinary object slots. Its class- - // declared named prefix does not move. Runtime miss handling proves the - // complete registered prefix plus dense numeric suffix once and publishes - // a nonzero token in cache word 2 and ObjectMeta. Generic structural or - // descriptor transitions clear the object token; only the exact learned - // numeric-tail installer preserves it. - // - // Keep the ordinary miss path cheap: test the cache word first. Every - // non-Array-subclass site reads zero and leaves without touching the - // receiver's meta pointer. - ctx.current_block = prefix_guard_idx; - let cached_prefix_ptr = ctx.block().gep( - I64, - &token_cache.cache, - &[(I64, &PIC_NAMED_PREFIX_TOKEN.to_string())], - ); - let cached_prefix = ctx.block().load(I64, &cached_prefix_ptr); - let prefix_armed = ctx.block().icmp_ne(I64, &cached_prefix, "0"); - ctx.block() - .cond_br(&prefix_armed, &prefix_meta_label, &miss_label); - - // ObjectHeader::meta is the final header field: offset 8 on LP64, 12 on - // ILP32. Load it with the target pointer width, then branch before reading - // ObjectMeta so a null metadata pointer remains harmless. - ctx.current_block = prefix_meta_idx; - let meta_ptr_size: u64 = if crate::target_layout::target_is_ilp32(ctx.target_triple) { - 4 - } else { - 8 - }; - let meta_offset = - crate::target_layout::object_meta_slot_offset_bytes(ctx.target_triple).to_string(); - let meta_addr = ctx.block().add(I64, &obj_handle, &meta_offset); - let meta_slot = ctx.block().inttoptr(I64, &meta_addr); - let meta_load_ty = if meta_ptr_size == 4 { I32 } else { I64 }; - let meta_raw = ctx.block().load(meta_load_ty, &meta_slot); - let meta = if meta_ptr_size == 4 { - ctx.block().zext(I32, &meta_raw, I64) - } else { - meta_raw - }; - let meta_nonnull = ctx.block().icmp_ne(I64, &meta, "0"); - ctx.block() - .cond_br(&meta_nonnull, &prefix_token_label, &miss_label); - - ctx.current_block = prefix_token_idx; - let meta_ptr = ctx.block().inttoptr(I64, &meta); - // repr(C) ObjectMeta word 6. The first six u64 words are prototype, - // descriptor blooms, flags, spill, and private brand. Runtime has an - // offset assertion paired with the IR test below. - let object_prefix_ptr = ctx.block().gep(I64, &meta_ptr, &[(I64, "6")]); - let object_prefix = ctx.block().load(I64, &object_prefix_ptr); - let prefix_match = ctx.block().icmp_eq(I64, &object_prefix, &cached_prefix); - ctx.block() - .cond_br(&prefix_match, &prefix_hit_label, &miss_label); - - ctx.current_block = prefix_hit_idx; - // The exact ShapeId guard did fail, so preserve typed-feedback accounting - // just like a polymorphic-way hit: the site remains structurally - // polymorphic even though no runtime fallback call is needed. - 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)], - ); - let prefix_slot_ptr = ctx.block().gep(I64, &token_cache.cache, &[(I64, "1")]); - let prefix_slot = ctx.block().load(I64, &prefix_slot_ptr); - let prefix_offset = ctx.block().shl(I64, &prefix_slot, "3"); - let prefix_base = ctx.block().add(I64, &obj_handle, &obj_header_size); - let prefix_field_addr = ctx.block().add(I64, &prefix_base, &prefix_offset); - let prefix_field_ptr = ctx.block().inttoptr(I64, &prefix_field_addr); - let val_prefix = ctx.block().load(DOUBLE, &prefix_field_ptr); - let prefix_end_label = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - // Descriptor-bearing Array subclasses reach this duplicate of the family - // guard without ever entering `pic.token`: the exact raw-load PIC remains - // forbidden, but a runtime-proved data-only declared prefix is still safe. - // Every failure goes to the cold handler because the shape token values - // required by `pic.miss` do not dominate this path. - ctx.current_block = desc_prefix_guard_idx; - let desc_cached_prefix_ptr = ctx.block().gep( - I64, - &desc_cache.cache, - &[(I64, &PIC_NAMED_PREFIX_TOKEN.to_string())], - ); - let desc_cached_prefix = ctx.block().load(I64, &desc_cached_prefix_ptr); - let desc_prefix_armed = ctx.block().icmp_ne(I64, &desc_cached_prefix, "0"); - ctx.block() - .cond_br(&desc_prefix_armed, &desc_prefix_meta_label, &cold_label); - - ctx.current_block = desc_prefix_meta_idx; - let desc_meta_addr = ctx.block().add(I64, &obj_handle, &meta_offset); - let desc_meta_slot = ctx.block().inttoptr(I64, &desc_meta_addr); - let desc_meta_raw = ctx.block().load(meta_load_ty, &desc_meta_slot); - let desc_meta = if meta_ptr_size == 4 { - ctx.block().zext(I32, &desc_meta_raw, I64) - } else { - desc_meta_raw + let hit_end_label = match hit_live_idx { + None => ctx.block().label.clone(), + Some(idx) => { + ctx.current_block = idx; + crate::expr::emit_typed_feedback_record_call( + ctx.block(), + "js_typed_feedback_record_guard_pass", + &[(I64, &feedback_site_id)], + ); + let label = ctx.block().label.clone(); + ctx.block().br(&merge_label); + label + } }; - let desc_meta_nonnull = ctx.block().icmp_ne(I64, &desc_meta, "0"); - ctx.block() - .cond_br(&desc_meta_nonnull, &desc_prefix_token_label, &cold_label); - ctx.current_block = desc_prefix_token_idx; - let desc_meta_ptr = ctx.block().inttoptr(I64, &desc_meta); - let desc_object_prefix_ptr = ctx.block().gep(I64, &desc_meta_ptr, &[(I64, "6")]); - let desc_object_prefix = ctx.block().load(I64, &desc_object_prefix_ptr); - let desc_prefix_match = ctx - .block() - .icmp_eq(I64, &desc_object_prefix, &desc_cached_prefix); - ctx.block() - .cond_br(&desc_prefix_match, &desc_prefix_hit_label, &cold_label); - - ctx.current_block = desc_prefix_hit_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)], - ); - let desc_prefix_slot_ptr = ctx.block().gep(I64, &desc_cache.cache, &[(I64, "1")]); - let desc_prefix_slot = ctx.block().load(I64, &desc_prefix_slot_ptr); - let desc_prefix_offset = ctx.block().shl(I64, &desc_prefix_slot, "3"); - let desc_prefix_base = ctx.block().add(I64, &obj_handle, &obj_header_size); - let desc_prefix_field_addr = ctx.block().add(I64, &desc_prefix_base, &desc_prefix_offset); - let desc_prefix_field_ptr = ctx.block().inttoptr(I64, &desc_prefix_field_addr); - let val_desc_prefix = ctx.block().load(DOUBLE, &desc_prefix_field_ptr); - let desc_prefix_end_label = ctx.block().label.clone(); - ctx.block().br(&merge_label); + // The hit's hole lands here rather than on the shared exit — see the + // congruence note where the block is minted. + ctx.current_block = deleted_idx; + ctx.block().br(&cold_label); // PIC miss on the MRU entry — before paying for the call, try the // polymorphic ways (#7753). // - // `js_object_get_field_ic_miss` is not a cheap fallback: it re-derives the - // receiver kind from scratch (proxy band, closure magic, registered-buffer - // and typed-array registries, small-handle dispatch), reads the + // The slow entry is not a cheap fallback: it re-derives the receiver kind + // from scratch (proxy band, closure magic, registered-buffer and + // typed-array registries, small-handle dispatch), reads the // accessors-in-use thread-local, then linear-scans the keys array with a // `js_string_equals` per key. On a site whose receiver alternates between a // handful of shapes — the shape of every discriminated-union dispatch — @@ -781,9 +657,10 @@ pub(crate) fn lower_generic_property_get( // // # Why this block is DOMINATED by `pic.token` (#7907) // - // Its only predecessor is `pic.token` after the MRU token did not match. - // The exact descriptor identity proves cached-slot bounds, so `token` - // is everything the way compares need. + // Its only predecessor is `pic.token.miss`, which is `pic.token`'s. The + // exact descriptor identity proves cached-slot bounds, so `token` is + // everything the way compares need, and the cache pointer arrives on one + // edge rather than through a phi. // // #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 @@ -795,24 +672,8 @@ pub(crate) fn lower_generic_property_get( // the duplicate ladder sat on the hot path. Measured on `interp.ts`'s // `evalNode`, the single hottest instruction in the whole program was the // redundant receiver reconstruction inside this block. - // - // 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; - // All incoming cache values have passed their own presence guard. The - // merge preserves that proof without resolving the full cache on a hit. - let cache_ref = ctx.block().phi( - PTR, - &[ - (&token_cache.cache, &prefix_guard_label), - (&token_cache.cache, &prefix_meta_label), - (&token_cache.cache, &prefix_token_label), - (&deleted_cache.cache, &deleted_label), - ], - ); + let cache_ref = token_cache.cache.clone(); crate::expr::emit_typed_feedback_record_call( ctx.block(), "js_typed_feedback_record_guard_fail", @@ -915,33 +776,36 @@ 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); + // #7907: receiver-validation failure. A receiver that gets here can never + // match a way — the compares require a real pointer to a plain + // descriptor-free `ObjectHeader` — so 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 records on the same edges, so the feedback + // signal is byte-identical to what the pre-T1 blocks reported. + if let Some(cold_idx) = cold_idx { + 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. + // The object exit: one call reproducing every pointer-path arm this tower + // used to expand. ctx.current_block = call_idx; crate::expr::emit_versioned_loop_callback_deopt(ctx); let miss_key_handle = emit_key_handle(ctx, &key_handle_global); let val_miss = ctx.block().call( DOUBLE, - "js_object_get_field_ic_miss_packed", + "js_object_get_field_ic_slow", &[ (I64, &obj_handle), (I64, &miss_key_handle), @@ -952,149 +816,43 @@ pub(crate) fn lower_generic_property_get( let miss_end_label = ctx.block().label.clone(); ctx.block().br(&merge_label); - // Merge PIC hit + way hit + miss, then jump to the outer recv-valid merge. - ctx.current_block = merge_idx; - let pic_val = ctx.block().phi( - DOUBLE, - &[ - (&val_hit, &hit_end_label), - (&val_ovf, &ovf_end_label), - (&val_prefix, &prefix_end_label), - (&val_desc_prefix, &desc_prefix_end_label), - (&val_way, &way_end_label), - (&val_miss, &miss_end_label), - ], - ); - let pic_end_label = ctx.block().label.clone(); - ctx.block().br(&final_merge_label); - // Native Map/Set `.size`: their common leading field was admitted only by // the exact live GC-kind checks above. Keep the read inline; calling // `js_map_size` / `js_set_size` would reclassify the same receiver again. - let collection_size_arm = if let Some(collection_idx) = collection_size_idx { + let collection_size_arm = collection_size_idx.map(|collection_idx| { ctx.current_block = collection_idx; let size_i32 = ctx.block().safe_load_i32_from_ptr(&obj_handle); let size = ctx.block().uitofp(I32, &size_i32, DOUBLE); let collection_end_label = ctx.block().label.clone(); - ctx.block().br(&final_merge_label); - Some((size, collection_end_label)) - } else { - None - }; - - // Invalid receiver: per JS spec, `undefined` and `null` - // throw a TypeError; other non-pointer tags (int32, bool, - // plain f64, bigint) should auto-box and look up via the - // primitive's prototype. Perry doesn't implement primitive - // auto-boxing yet, so non-nullish primitives continue to - // return `undefined` to preserve existing behavior. - // - // Issue #462: bare `obj.foo` against TAG_UNDEFINED / - // TAG_NULL silently returned undefined, which masked - // unimplemented-API bugs (e.g. `crypto.subtle.encrypt(...)` - // ran to completion as a chain of no-ops). Funnel the - // nullish receiver into the runtime helper which prints a - // node-shaped diagnostic and aborts. - ctx.current_block = invalid_idx; - let is_undef = ctx - .block() - .icmp_eq(I64, &obj_bits, crate::nanbox::TAG_UNDEFINED_I64); - let is_null = ctx - .block() - .icmp_eq(I64, &obj_bits, crate::nanbox::TAG_NULL_I64); - let is_nullish = ctx.block().or(I1, &is_undef, &is_null); - let throw_idx = ctx.new_block("pget.throw_nullish"); - let undef_idx = ctx.new_block("pget.recv_undef_return"); - let throw_label = ctx.block_label(throw_idx); - let undef_label = ctx.block_label(undef_idx); - ctx.block().cond_br(&is_nullish, &throw_label, &undef_label); - - // Throw path: helper aborts the process; block ends with - // `unreachable` because the helper's `-> !` return is - // not visible to LLVM. - ctx.current_block = throw_idx; - let prop_entry = ctx.strings.entry(key_idx); - let prop_bytes_global = format!("@{}", prop_entry.bytes_global); - let prop_len_str = prop_entry.byte_len.to_string(); - let is_null_i32 = ctx.block().zext(I1, &is_null, I32); - ctx.block().call_void( - "js_throw_type_error_property_access", - &[ - (I32, &is_null_i32), - (PTR, &prop_bytes_global), - (I64, &prop_len_str), - ], - ); - ctx.block().unreachable(); - - // Undef-return path: existing fall-through for non-nullish - // invalid receivers. Route through the runtime helper first - // so non-pointer typed shapes can still report a sensible - // value when the runtime knows what they are. Today this - // unblocks Date `.constructor` (Date stores as a raw f64 - // timestamp, so the codegen receiver-tag check at line ~4212 - // rejects it as non-pointer — yet the runtime's - // `js_object_get_field_by_name_f64` recognizes the bit - // pattern via `DATE_REGISTRY` and returns the global Date - // constructor closure). Date-fns `constructFrom` blocker. - ctx.current_block = undef_idx; - let undef_key_handle = emit_key_handle(ctx, &key_handle_global); - let undef_val = ctx.block().call( - DOUBLE, - "js_object_get_field_by_name_f64", - &[(I64, &obj_bits), (I64, &undef_key_handle)], - ); - let invalid_end_label = ctx.block().label.clone(); - ctx.block().br(&final_merge_label); - - // SSO receiver: dispatch directly to the runtime by-name - // helper, which reads `.length` inline from the NaN-box - // payload and returns `undefined` for other keys. Bypasses - // the PIC entirely (PIC would read garbage memory). The - // key handle has already been extracted above. - ctx.current_block = sso_idx; - let sso_val = if inline_string_length { - // `.length` of an SSO string is the length byte in bits 40..47 of the - // NaN-box itself — the same extract `js_object_get_field_by_name_f64` - // performs, minus the call and the key decode. - let len_shifted = ctx.block().lshr(I64, &obj_bits, "40"); - let len_byte = ctx.block().and(I64, &len_shifted, "255"); - ctx.block().uitofp(I64, &len_byte, DOUBLE) - } else { - let sso_key_handle = emit_key_handle(ctx, &key_handle_global); - ctx.block().call( - DOUBLE, - "js_object_get_field_by_name_f64", - &[(I64, &obj_bits), (I64, &sso_key_handle)], - ) - }; - let sso_end_label = ctx.block().label.clone(); - ctx.block().br(&final_merge_label); + ctx.block().br(&merge_label); + (size, collection_end_label) + }); // Heap string `.length`: `utf16_len` is the leading `u32` of // `StringHeader` — the identical load the proven-string lowering in // `property_get.rs` emits (`strlen.heap`). `safe_load_i32_from_ptr` // keeps a sub-page handle off the load. - let strlen_heap_arm = if let Some(heap_idx) = strlen_heap_idx { + let strlen_heap_arm = strlen_heap_idx.map(|heap_idx| { ctx.current_block = heap_idx; let len_i32 = ctx.block().safe_load_i32_from_ptr(&obj_handle); let heap_len = ctx.block().uitofp(I32, &len_i32, DOUBLE); let heap_end_label = ctx.block().label.clone(); - ctx.block().br(&final_merge_label); - Some((heap_len, heap_end_label)) - } else { - None - }; + ctx.block().br(&merge_label); + (heap_len, heap_end_label) + }); - // Outer merge joins PIC result + invalid-receiver undefined - // + SSO result + class-ref dispatch result (+ heap-string `.length`). - ctx.current_block = final_merge_idx; + // One merge for the whole tower: the inline hit, a way hit, the two slow + // calls, and whichever short-circuit arms this key grew. + ctx.current_block = merge_idx; let mut incoming: Vec<(&str, &str)> = vec![ - (&pic_val, &pic_end_label), - (&undef_val, &invalid_end_label), - (&sso_val, &sso_end_label), - (&class_ref_result, &class_ref_end_label), + (&val_hit, &hit_end_label), + (&val_way, &way_end_label), + (&val_miss, &miss_end_label), + (&val_nonptr, &nonptr_end_label), ]; + if let Some((sso_val, sso_end_label)) = sso_arm.as_ref() { + incoming.push((sso_val, sso_end_label)); + } if let Some((heap_len, heap_end_label)) = strlen_heap_arm.as_ref() { incoming.push((heap_len, heap_end_label)); } diff --git a/crates/perry-codegen/src/expr/property_get/tests.rs b/crates/perry-codegen/src/expr/property_get/tests.rs index 59f23e97e4..e9a3b0f654 100644 --- a/crates/perry-codegen/src/expr/property_get/tests.rs +++ b/crates/perry-codegen/src/expr/property_get/tests.rs @@ -136,10 +136,15 @@ fn imported_variable_read_preserves_class_tags_and_calls_the_live_getter_once() (operand == format!("{value} to i64")).then_some(result) }) .expect("getter result must be classified by its intact value tag"); + // #9366's invariant, one indirection later: T1 moved the INT32 class-ref + // arm (and its `js_typed_feedback_object_get_field_by_name_f64` call) into + // `js_object_get_field_ic_nonptr`, which routes on the tag — so the bits it + // receives must still be the getter's UNMASKED value. A masked handle here + // would lose the 0x7FFE tag and the class would dispatch as an object. assert!( ir.lines().any(|line| { - line.contains("call double @js_typed_feedback_object_get_field_by_name_f64(") - && line.contains(&format!(", i64 {bits},")) + line.contains("call double @js_object_get_field_ic_nonptr(") + && line.contains(&format!("i64 {bits},")) }), "class dispatch must receive the getter's unmasked value bits:\n{ir}" ); @@ -391,40 +396,56 @@ fn pic_cache_layout_matches_runtime() { ); } -/// Object-backed Array subclasses mint one ShapeId per numeric tail length. -/// A named-field site on such a receiver must try the independently proved -/// class prefix before falling into the bounded shape ways / runtime miss. +/// Object-backed Array subclasses mint one ShapeId per numeric tail length, so +/// a named-field site on such a receiver is served from the independently +/// proved class prefix rather than the exact ShapeId. +/// +/// Renamed from `generic_property_get_emits_array_subclass_named_prefix_guard` +/// (T1): the proof itself is unchanged and still runs on exactly the same two +/// words, but it runs in `js_object_get_field_ic_slow` instead of in FOUR +/// emitted blocks per site (`pic.prefix.*`) plus FOUR more for the +/// descriptor-bearing twin (`pic.desc.prefix.*`). Its behaviour is pinned by +/// `an_armed_named_prefix_serves_the_cached_slot` in +/// `perry-runtime/src/object/field_get_set/ic_miss/ic_slow.rs`; what this test +/// keeps is the CODEGEN half of the contract — the emitted site must hand the +/// runtime the two operands that proof reads, and must not have grown its own +/// copy back. #[test] -fn generic_property_get_emits_array_subclass_named_prefix_guard() { - use crate::expr::property_get::generic_dispatch::PIC_NAMED_PREFIX_TOKEN; - +fn array_subclass_named_prefix_proof_is_reached_through_the_one_exit() { let ir = emit(false, None); - let guard = ir - .find("\npic.prefix.guard") - .unwrap_or_else(|| panic!("expected a named-prefix guard block:\n{ir}")); - let token = ir - .find("\npic.prefix.token") - .unwrap_or_else(|| panic!("expected a named-prefix token block:\n{ir}")); - let hit = ir - .find("\npic.prefix.hit") - .unwrap_or_else(|| panic!("expected a named-prefix hit block:\n{ir}")); - let miss = ir - .find("\npic.miss") - .unwrap_or_else(|| panic!("expected the ordinary PIC miss block:\n{ir}")); - assert!( - guard < token && token < hit && hit < miss, - "prefix guard must precede the ordinary miss path:\n{ir}" - ); - - let guard_body = &ir[guard..token]; - assert!( - guard_body.contains(&format!("i64 {PIC_NAMED_PREFIX_TOKEN}\n")), - "the cheap first gate must read cache word 2 before touching ObjectMeta:\n{guard_body}" - ); - let token_body = &ir[token..hit]; + for gone in [ + "pic.prefix.guard", + "pic.prefix.meta", + "pic.prefix.token", + "pic.prefix.hit", + "pic.desc.classify", + "pic.desc.prefix.guard", + "pic.desc.prefix.meta", + "pic.desc.prefix.token", + "pic.desc.prefix.hit", + ] { + assert!( + !ir.contains(gone), + "the named-prefix ladder must not be emitted per site any more, \ + found `{gone}`:\n{ir}" + ); + } + // The runtime half reads cache word 2 against ObjectMeta word 6 and then + // the cached slot, so the emitted site has to hand it both per-site + // globals — a call that lost either operand would silently stop serving + // Array-subclass named fields and fall back to the full lookup. + let call = ir + .find("\npic.miss.call") + .unwrap_or_else(|| panic!("expected the single slow-exit block:\n{ir}")); + let call_line = ir[call..] + .lines() + .find(|l| l.contains("@js_object_get_field_ic_slow(")) + .unwrap_or_else(|| panic!("expected the one slow call:\n{ir}")); assert!( - token_body.contains("getelementptr i64") && token_body.contains("i64 6\n"), - "ObjectMeta word 6 must carry the runtime-paired prefix token:\n{token_body}" + call_line.contains("ptr @perry_ic_") && call_line.contains("_packed_get"), + "the slow exit must receive BOTH the cache slot (which holds the \ + named-prefix token in word 2) and the packed MRU word, or the runtime \ + cannot reproduce the arms this site stopped emitting:\n{call_line}" ); } @@ -471,8 +492,8 @@ fn generic_property_get_tries_ways_before_calling_the_miss_handler() { miss call — otherwise the compares are not gating anything:\n{ways_body}" ); assert!( - !ways_body.contains("call double @js_object_get_field_ic_miss"), - "the miss call must not sit inside the way block:\n{ways_body}" + !ways_body.contains("call double @js_object_get_field_ic"), + "the slow call must not sit inside the way block:\n{ways_body}" ); // The way compares read (token, slot) pairs at words PIC_WAY_BASE.. and the // gate reads the state word — all inside pic.ways, none anywhere else. @@ -520,10 +541,43 @@ fn pic_miss_reuses_the_token_blocks_values_instead_of_re_deriving_them() { main.contains("@perry_ic_"), "test premise: the generic read reaches the inline PIC:\n{ir}" ); + // T1: the landing block is now the single slow exit itself, and the + // dominance is structural — `pic.miss` has exactly ONE predecessor, + // `pic.token.miss`, which `pic.token` dominates. Assert that directly: + // routing any receiver-validation failure back into `pic.miss` would add a + // predecessor and immediately re-introduce the phis #7907 removed. + // `pic.miss` carries a numeric suffix and `pic.miss.call` starts with the + // same text, so match the block's own label exactly and then count the + // branches whose TARGET is that label (a `br i1` naming both blocks counts + // once, for the right one). + let miss_label = main + .lines() + .filter(|l| !l.starts_with(' ') && l.ends_with(':')) + .map(|l| l.trim_end_matches(':')) + .find(|l| { + l.strip_prefix("pic.miss.") + .is_some_and(|tail| tail.chars().all(|c| c.is_ascii_digit())) + }) + .unwrap_or_else(|| panic!("expected a pic.miss block:\n{ir}")) + .to_string(); + let preds = main + .lines() + .filter(|l| l.trim_start().starts_with("br ")) + .filter(|l| { + l.split("label %") + .skip(1) + .any(|t| t.trim_end_matches(&[',', ' '][..]) == miss_label) + }) + .count(); + assert_eq!( + preds, 1, + "pic.miss must have exactly one predecessor (pic.token.miss), or it is \ + no longer dominated by pic.token:\n{ir}" + ); assert!( - main.contains("\npic.miss.cold"), - "the two receiver-validation failures need their own landing block, \ - otherwise pic.miss is not dominated by pic.token:\n{ir}" + main.contains("label %pic.miss.call"), + "every receiver-validation failure must land on the single slow \ + exit:\n{ir}" ); assert!( !main.contains("@PERRY_IC_EPOCH"), @@ -963,28 +1017,56 @@ fn generic_length_read_serves_a_string_inline() { ); // Everything that is NOT a string keeps the tower. assert!( - ir.contains("@perry_ic_") && ir.contains("js_object_get_field_ic_miss"), - "non-string receivers must still reach the inline PIC and its miss \ - handler:\n{ir}" + ir.contains("@perry_ic_") && ir.contains("js_object_get_field_ic_slow"), + "non-string receivers must still reach the inline PIC and its slow \ + exit:\n{ir}" ); } /// The short-circuit is keyed on the property name: any other key on a string -/// receiver (`s.charCodeAt`, `s.constructor`) still needs the runtime, so no -/// other read may grow the string blocks. +/// receiver (`s.charCodeAt`, `s.constructor`) still needs the runtime. +/// +/// T1 moved the SSO arm itself behind the one exit — an SSO receiver with any +/// key but `length` is served by `js_object_get_field_ic_slow`'s tag ladder +/// (`sso_receiver_routes_to_the_by_name_helper` in +/// `ic_miss/ic_slow.rs` pins that it still reaches the by-name helper). What +/// codegen must guarantee is that such a receiver LEAVES: it must never fall +/// into the PIC, whose header loads would read the SSO payload as an address. #[test] fn generic_non_length_read_keeps_the_whole_tower() { let ir = emit_read("charCodeAt"); + for gone in ["pget.strlen_heap", "pget.recv_sso"] { + assert!( + !ir.contains(gone), + "only `.length` may grow an inline string arm, found `{gone}`:\n{ir}" + ); + } + // 32765 = (STRING_TAG|POINTER_TAG) & 0xFFFD: the one test that decides + // whether the receiver may be dereferenced at all. Its false edge must be + // the NON-POINTER exit — a distinct block with a distinct callee, which is + // what stops SimplifyCFG folding this guard into the next one. + let tag_branch = ir + .lines() + .find(|l| l.contains("icmp eq i64") && l.contains("32765")) + .unwrap_or_else(|| panic!("expected the receiver-tag test:\n{ir}")); + let cond = tag_branch + .trim() + .split_once(" = ") + .map(|(lhs, _)| lhs.to_string()) + .unwrap_or_else(|| panic!("malformed tag test: {tag_branch}")); + let branch = ir + .lines() + .find(|l| l.trim_start().starts_with(&format!("br i1 {cond},"))) + .unwrap_or_else(|| panic!("expected a branch on the receiver tag:\n{ir}")); assert!( - !ir.contains("pget.strlen_heap"), - "only `.length` may take the inline string arm:\n{ir}" + branch.contains("label %pget.recv_other") && !branch.contains("label %pic.miss.call"), + "a non-pointer receiver must leave for its OWN exit — sharing the \ + object exit's block is what cost +4 instructions per hit:\n{branch}" ); - let sso = ir - .find("\npget.recv_sso") - .unwrap_or_else(|| panic!("expected an SSO receiver block:\n{ir}")); assert!( - ir[sso..].contains("js_object_get_field_by_name_f64"), - "a non-`length` SSO read must still call the by-name helper:\n{ir}" + ir.contains("@js_object_get_field_ic_slow(") + && ir.contains("@js_object_get_field_ic_nonptr("), + "the tower must still reach both slow entries:\n{ir}" ); } @@ -1015,7 +1097,7 @@ fn generic_size_read_serves_native_collections_inline() { {collection_body}" ); assert!( - ir.contains("@perry_ic_") && ir.contains("js_object_get_field_ic_miss"), + ir.contains("@perry_ic_") && ir.contains("js_object_get_field_ic_slow"), "non-collection receivers must retain the generic property tower:\n{ir}" ); } @@ -1069,10 +1151,18 @@ fn packed_pic_header_guard_is_endianness_aware() { ir.contains("load i16"), "descriptor guard must retain native endianness: {ir}" ); + assert!( + ir.contains(", 2048"), + "the native-endian arm must still test OBJ_FLAG_HAS_DESCRIPTORS: {ir}" + ); } + // T1: the descriptor-bearing fallback is the one exit. It must still + // be a distinct EDGE — a descriptor-bearing receiver may never take + // the raw-slot hit — and the runtime keeps the Array-subclass + // named-prefix exception behind it. assert!( - ir.contains("pic.desc.prefix.guard"), - "descriptor fallback must remain: {ir}" + ir.contains("@js_object_get_field_ic_slow(") && ir.contains("\npic.miss.call"), + "descriptor fallback must remain, through the one exit: {ir}" ); } } @@ -1088,7 +1178,7 @@ fn compact_get_mru_is_atomic_and_full_cache_remains_lazy() { ir.contains("load atomic i64") && ir.contains("monotonic, align 8"), "{ir}" ); - assert!(ir.contains("@js_object_get_field_ic_miss_packed("), "{ir}"); + assert!(ir.contains("@js_object_get_field_ic_slow("), "{ir}"); assert!( ir.contains("trunc i64") && ir.contains("icmp ne i64"), "{ir}" @@ -1098,3 +1188,104 @@ fn compact_get_mru_is_atomic_and_full_cache_remains_lazy() { "a full-cache dereference must still guard a null site: {ir}" ); } + +/// T1: the whole point — per untyped `obj.prop` the emitted tower is TWO calls +/// and a handful of blocks, with the inline hit and the polymorphic ways kept. +/// +/// This is a ratchet, so it is an EXACT count in both dimensions. The tower it +/// replaced expanded 33 tower blocks and SIX runtime call sites per site +/// (`js_object_get_field_by_name_f64` twice, the feedback-wrapped class-ref +/// helper, `js_throw_type_error_property_access`, +/// `js_object_get_field_ic_overflow_load`, `js_object_get_field_ic_miss_packed`), +/// each one a statepoint whose live GC values are written into +/// `.perry_gcmap`. On @babel/parser that was 29% of all emitted IR over 6,487 +/// sites; a single arm creeping back inline is a regression measured in +/// megabytes of `.text`, and nothing else in the suite would report it. +/// +/// Two and not one: a single shared exit let SimplifyCFG fold the receiver-tag +/// test and the small-handle test into one flat predicate, costing +4.00 +/// instructions on every HIT (measured). The separate non-pointer callee is +/// what keeps that guard chain branchy, so the count below is 2 — and a change +/// that makes it 1 is a hit-path regression, not a size win. +#[test] +fn the_generic_tower_is_two_calls_and_a_bounded_number_of_blocks() { + let ir = emit(false, None); + let func = ir + .split("\ndefine ") + .find(|f| f.contains("\npic.miss.call")) + .unwrap_or_else(|| panic!("no function contains the generic tower:\n{ir}")); + + // Every call/invoke in the whole function, by callee. Feedback records are + // compile-time gated and absent from this build; anything else must be the + // one exit (the fixture's module init contributes its own calls, so match + // on the property-GET family rather than on a total). + let pget_calls: Vec<&str> = func + .lines() + .filter(|l| l.contains(" call ") || l.contains(" invoke ")) + .filter_map(|l| l.split(" @").nth(1)) + .filter_map(|c| c.split('(').next()) + .filter(|c| { + c.starts_with("js_object_get_field") + || c.starts_with("js_typed_feedback_object_get_field") + || *c == "js_throw_type_error_property_access" + }) + .collect(); + let mut sorted = pget_calls.clone(); + sorted.sort(); + assert_eq!( + sorted, + vec![ + "js_object_get_field_ic_nonptr", + "js_object_get_field_ic_slow" + ], + "the tower must expand exactly two property-GET call sites:\n{func}" + ); + + let blocks: Vec<&str> = func + .lines() + .filter(|l| !l.starts_with(' ') && l.ends_with(':')) + .map(|l| l.trim_end_matches(':')) + .filter(|l| l.starts_with("pget.") || l.starts_with("pic.")) + .collect(); + let mut expected = vec![ + // the guard chain and the inline hit + "pget.recv_ok", + // the non-pointer exit, off the tag test's false edge + "pget.recv_other", + "pic.recv_hdr", + "pic.token", + "pic.token.miss", + "pic.hit", + "pic.hit.inline", + // The hit's hole edge keeps its own landing block so its tail is not + // congruent with `pic.way.load`'s; `pic.hit.live` exists only when + // typed feedback has something to record on the live edge. + "pic.hit.deleted", + // the polymorphic ways, deliberately still inline (#7753) + "pic.miss", + "pic.ways", + "pic.way.load", + "pic.way.live", + // the one exit, and the join + "pic.miss.call", + "pget.recv_merge", + ]; + // Labels carry a numeric suffix (`pic.ways.16`); strip it for comparison. + let mut normalized: Vec = blocks + .iter() + .map(|b| { + let mut parts: Vec<&str> = b.split('.').collect(); + if parts.last().is_some_and(|p| p.parse::().is_ok()) { + parts.pop(); + } + parts.join(".") + }) + .collect(); + normalized.sort(); + expected.sort(); + assert_eq!( + normalized, + expected.iter().map(|s| s.to_string()).collect::>(), + "the emitted tower's block set changed:\n{func}" + ); +} diff --git a/crates/perry-codegen/src/gc_call_effects.rs b/crates/perry-codegen/src/gc_call_effects.rs index e1e8d17fe4..265da914e7 100644 --- a/crates/perry-codegen/src/gc_call_effects.rs +++ b/crates/perry-codegen/src/gc_call_effects.rs @@ -750,6 +750,11 @@ mod tests { "js_rel_lt", "js_rel_gt", "js_object_get_field_ic_miss_packed", + // T1: the generic-get tower's two slow exits. One reaches the same + // `get_field_ic_miss_impl`, the other the by-name helper, so both + // allocate and can run a user getter. + "js_object_get_field_ic_slow", + "js_object_get_field_ic_nonptr", ] { assert_ne!( classify_direct_callee(name), diff --git a/crates/perry-codegen/src/module/linkage.rs b/crates/perry-codegen/src/module/linkage.rs index 531516b8ef..823f7525e9 100644 --- a/crates/perry-codegen/src/module/linkage.rs +++ b/crates/perry-codegen/src/module/linkage.rs @@ -251,7 +251,10 @@ pub(crate) fn helper_decl_attrs(name: &str) -> &'static str { // register saves and code layout focused on the inline continuation. // `cold` is only a profitability hint: both calls remain fully // memory-clobbering and the cache miss remains GC-capable/throwing. - "js_object_get_field_ic_miss_packed" | "js_write_barrier_root_nanbox" => " cold", + "js_object_get_field_ic_miss_packed" + | "js_object_get_field_ic_slow" + | "js_object_get_field_ic_nonptr" + | "js_write_barrier_root_nanbox" => " cold", // PURE — each verified: pure bit tests/masking on the f64/i64 args, // total over arbitrary bits, no memory access anywhere in the body. // js_nanbox_pointer value/nanbox.rs — tag ladder, 0 → TAG_NULL diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index e909ba79a7..f35aa1b628 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -366,6 +366,17 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { // single call for oversized modules. Args: (obj_bits, key_handle, site_id, // per-site IC cache global) -> field value. module.declare_function("js_object_get_field_ic", DOUBLE, &[I64, I64, I64, PTR]); + // T1: the two exits of the inline generic-get tower. Every guard failure — + // SSO / INT32 class ref / nullish / non-object receiver / overflow slot / + // deleted slot / named prefix / miss+prime — branches to one of these + // instead of expanding its own arm and its own call. + // + // Non-pointer receiver (the emitted tag test failed): (obj_bits, key_handle, + // site_id) -> field value. No cache: none of its arms can prime one. + module.declare_function("js_object_get_field_ic_nonptr", DOUBLE, &[I64, I64, I64]); + // Heap-pointer receiver: (masked obj_handle, key_handle, per-site IC cache + // SLOT, per-site packed MRU word) -> field value. + module.declare_function("js_object_get_field_ic_slow", DOUBLE, &[I64, I64, PTR, PTR]); // Object rest destructuring: copy all properties from src except excluded keys. // Takes a src object ptr and an array of NaN-boxed strings (the excluded keys), // returns a new object pointer. diff --git a/crates/perry-codegen/src/stmt/cached_field_index_return.rs b/crates/perry-codegen/src/stmt/cached_field_index_return.rs index 30339319ab..07759705a5 100644 --- a/crates/perry-codegen/src/stmt/cached_field_index_return.rs +++ b/crates/perry-codegen/src/stmt/cached_field_index_return.rs @@ -207,7 +207,11 @@ pub(super) fn try_emit_cached_field_index_return( .cond_br(&exact, &field_load_label, &prefix_meta_label); ctx.current_block = prefix_meta_idx; - let cached_prefix_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "2")]); + // The same cache word the generic property-get tower's named-prefix proof + // uses, and the same one the runtime publishes it in. + let prefix_word = + crate::expr::property_get::generic_dispatch::PIC_NAMED_PREFIX_TOKEN.to_string(); + let cached_prefix_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, &prefix_word)]); let cached_prefix = ctx.block().load(I64, &cached_prefix_ptr); let prefix_armed = ctx.block().icmp_ne(I64, &cached_prefix, "0"); let pointer_bytes = if crate::target_layout::target_is_ilp32(ctx.target_triple) { diff --git a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs index 3961e289d8..9004f10c92 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -992,7 +992,10 @@ fn object_literal_element_resolution_does_not_escape_the_clone() { .expect("the merge block should be DEFINED in the emitted IR")..]; assert!( after.contains("js_object_get_field_by_name_f64") - || after.contains("js_object_get_field_ic_miss"), + || after.contains("js_object_get_field_ic_miss") + // T1: the generic tower's cold arms are behind these two entries now. + || after.contains("js_object_get_field_ic_slow") + || after.contains("js_object_get_field_ic_nonptr"), "the post-loop read must stay on the by-name path; emitted:\n{after}" ); } diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index b14de19aa9..d5128a648b 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -14866,7 +14866,11 @@ fn annotated_class_method_value_uses_generic_lookup() { // fallback, which is the exact regression #8033 exists to prevent. let generic = ir_function_body(&ir, "__probe$generic("); assert!( - generic.contains("call double @js_object_get_field_ic_miss"), + // T1 renamed the tower's cold exits; this assertion is about the + // unguarded body keeping GENERIC lookup, not about which symbol + // serves it. + generic.contains("call double @js_object_get_field_ic_slow") + || generic.contains("call double @js_object_get_field_ic_miss"), "an annotation-only class receiver must preserve generic property \ lookup in the unguarded body:\n{generic}" ); diff --git a/crates/perry-codegen/tests/native_proof_regressions/pod_manifest.rs b/crates/perry-codegen/tests/native_proof_regressions/pod_manifest.rs index 77222de320..d119a37e2c 100644 --- a/crates/perry-codegen/tests/native_proof_regressions/pod_manifest.rs +++ b/crates/perry-codegen/tests/native_proof_regressions/pod_manifest.rs @@ -652,7 +652,11 @@ fn native_pod_view_length_survives_immutable_local_alias() { "an immutable PodView alias must use the validating length helper:\n{ir}" ); assert!( - !ir.contains("call double @js_object_get_field_ic_miss"), + // T1 renamed the tower's exits; a negative assertion that names only + // the retired symbol can no longer fail, so it names the live ones. + !ir.contains("call double @js_object_get_field_ic_slow") + && !ir.contains("call double @js_object_get_field_ic_nonptr") + && !ir.contains("call double @js_object_get_field_ic_miss"), "a PodView alias must not enter the ordinary object-property PIC:\n{ir}" ); } diff --git a/crates/perry-codegen/tests/typed_feedback.rs b/crates/perry-codegen/tests/typed_feedback.rs index 3ee8f733eb..d093fc39e5 100644 --- a/crates/perry-codegen/tests/typed_feedback.rs +++ b/crates/perry-codegen/tests/typed_feedback.rs @@ -581,8 +581,9 @@ fn a_default_build_emits_no_typed_feedback_recording_calls() { // empty string. The property boundaries themselves must still be here — // this test proves the RECORDING is gone, not the program. assert!( - ir.contains("js_object_get_field_by_name_f64") - || ir.contains("js_object_get_field_ic_miss"), + ir.contains("call double @js_object_get_field_ic_slow(") + || ir.contains("call double @js_object_get_field_by_name_f64(") + || ir.contains("call double @js_object_get_field_ic_miss"), "the property reads themselves must still be lowered; emitted:\n{ir}" ); // And the helpers that DECIDE something, rather than merely counting, are @@ -598,8 +599,15 @@ fn a_default_build_emits_no_typed_feedback_recording_calls() { // the two dispatchers this same fixture still emits -- the property GET // (the set dispatcher's twin) and the method call -- and as CALLS, since // the old symbol match was satisfied by the `declare` line alone. + // T1: the property GET's dispatching wrapper is one indirection further + // out. `js_typed_feedback_object_get_field_by_name_f64` is no longer + // emitted per site — it is the INT32 class-ref arm of + // `js_object_get_field_ic_nonptr`, which the site calls with the same + // `site_id`. The line this assertion draws is unchanged: a dispatcher that + // DECIDES something is emitted in a default build, a helper that merely + // counts is not (all six are asserted absent above). assert!( - ir.contains("call double @js_typed_feedback_object_get_field_by_name_f64("), + ir.contains("call double @js_object_get_field_ic_nonptr("), "dispatching feedback wrappers must still be emitted in a default build \ (property get):\n{ir}" ); diff --git a/crates/perry-runtime/src/object/field_get_set.rs b/crates/perry-runtime/src/object/field_get_set.rs index 318bb493d9..dc9438fcd3 100644 --- a/crates/perry-runtime/src/object/field_get_set.rs +++ b/crates/perry-runtime/src/object/field_get_set.rs @@ -309,8 +309,14 @@ pub use ic_miss::{ js_object_set_field_by_property_id, js_private_brand_add, js_private_brand_check, js_private_field_add, js_private_guard, PicCache, PicCacheSlot, PIC_CACHE_WORDS, }; +/// The one slow exit of the emitted generic property-get tower. Declared here +/// rather than inside `ic_miss.rs` only because that file sits at the +/// 2000-line cap; the source lives next to its sibling entries. +#[path = "field_get_set/ic_miss/ic_slow.rs"] +mod ic_slow; pub(crate) use ic_slot::pic_slot_census; pub use ic_slot::{pic_arena_bytes, pic_slot_peek, pic_slot_resolve, pic_slots_resolved}; +pub use ic_slow::{js_object_get_field_ic_nonptr, js_object_get_field_ic_slow}; #[cfg(test)] mod buffer_ic_miss_tests { diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs index 41a847b423..9cedc25adb 100644 --- a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs @@ -537,7 +537,7 @@ fn ic_diag_note( mod packed_get; pub use packed_get::{js_object_get_field_ic_miss, js_object_get_field_ic_miss_packed}; -fn get_field_ic_miss_impl( +pub(super) fn get_field_ic_miss_impl( obj: *const ObjectHeader, key: *const crate::StringHeader, cache_slot: *mut PicCacheSlot, diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss/ic_slow.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss/ic_slow.rs new file mode 100644 index 0000000000..4f36ea44aa --- /dev/null +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss/ic_slow.rs @@ -0,0 +1,582 @@ +//! The two slow exits of the emitted generic property-get tower. +//! +//! # Why this exists +//! +//! `lower_generic_property_get` used to emit ~37 basic blocks and 6-7 runtime +//! call sites per untyped `obj.prop`: an SSO arm, an INT32 class-ref arm, a +//! nullish-throw arm, a non-object-receiver arm, an overflow-slot load, a +//! deleted-slot miss, two Array-subclass named-prefix ladders, and the +//! miss+prime. Every one of those calls is a statepoint, so both the emitted +//! `.text` and `.perry_gcmap` scale with call sites x live GC values — on +//! @babel/parser that tower was 29% of all emitted IR over 6,487 sites. +//! +//! The inline *hit* is worth its bytes; the arms around it are not. These two +//! entries are what the site branches to instead of each of them, reproducing +//! the arms they replaced in the same ORDER and with the same cache-priming +//! decisions. The emitted site keeps, byte for byte, the receiver-tag test, the +//! small-handle test, the packed header kind/descriptor test, the packed-MRU +//! compare, the overflow-bit test, the inline field load with its hole check, +//! and the polymorphic ways. +//! +//! # Why TWO exits and not one +//! +//! The first cut had one entry taking the unmasked NaN-box, reached from every +//! guard failure including the receiver-tag test. It cost **+4.00 instructions +//! on every property-read HIT**, measured on a monomorphic 10M-read loop +//! (1,231,521,261 -> 1,271,522,669 retired instructions, +3.2%), and the +//! disassembly says exactly why: with the tag test and the small-handle test +//! failing to the SAME block, LLVM's SimplifyCFG folds them into one flat +//! predicate — +//! +//! ```text +//! cmp …; sete %dl; cmp …; setae %dil; test %dil,%dl; je (6 instructions) +//! ``` +//! +//! — where the branchy chain was `cmp; jne; cmp; ja` (4). That is #7883's +//! finding re-introduced from the other side: there codegen flattened the +//! guards, here the optimiser does it because the shared exit made the two +//! branches congruent. Giving the tag test its own callee restores the chain +//! (and, as a bonus, lets the unmasked `obj_bits` die in the entry block +//! instead of staying live across the whole hit path for a call it might make). +//! +//! The split is along the one line that matters: [`js_object_get_field_ic_nonptr`] +//! serves receivers that are NOT heap pointers (and needs the tag, not the +//! cache), [`js_object_get_field_ic_slow`] serves the ones that are (and needs +//! the cache, not the tag). Two call sites per site instead of six. +//! +//! # The arms, in the order the emitted tower took them +//! +//! 1. Receiver-tag routing ([`js_object_get_field_ic_nonptr`]), exactly as +//! `js_object_get_field_ic` does it (SSO -> the SSO-aware by-name helper; +//! INT32 class ref -> the feedback-wrapped by-name helper; nullish -> +//! `TypeError`; any other non-pointer -> the by-name helper, which still +//! resolves typed shapes such as Date `.constructor`). +//! 2. A packed-MRU token HIT that the emitted hit path declined: +//! * the slot carries `IC_SLOT_OVERFLOW_BIT` — the field lives in the spill +//! buffer, so the inline `obj + header + slot*8` arithmetic must not run +//! on it. This is `js_object_get_field_ic_overflow_load`'s body, including +//! its fallback to the miss handler **without** the packed republication +//! (that helper called the three-argument `js_object_get_field_ic_miss`, +//! and re-publishing the packed word here would change which arm the next +//! read of the same site takes); +//! * otherwise the inline load found `TAG_HOLE` — a field deleted since +//! priming — and takes the ordinary miss, with the packed word, exactly +//! as the emitted `pic.hit.deleted` -> `pic.miss.call` edge did. +//! 3. The Array-subclass named-prefix proof (cache word 2 against `ObjectMeta` +//! word 6, then the cached slot). The emitted tower had this twice, once for +//! a descriptor-free receiver whose exact ShapeId missed and once for a +//! descriptor-bearing one that could never take the raw-load PIC at all; +//! both reduce to the same three loads on a `GC_TYPE_OBJECT` receiver with a +//! resolved cache, so they are one arm here. +//! 4. Everything else: `get_field_ic_miss_impl`, which owns the priming policy +//! (`prime_get`, the megamorphic countdown, the `PERRY_IC_DIAG` rows). +//! +//! # What is deliberately NOT here +//! +//! The polymorphic ways stay inline. They are read-only compares against a +//! cache the site already resolved, they serve the receiver rotation that is +//! the whole point of #7753, and moving them behind a call is a separate +//! measurement. +//! +//! Typed feedback is unchanged: the OBSERVE call still sits inline in +//! `pget.recv_ok` and the guard-pass / guard-fail / fallback-call records still +//! sit on the emitted edges, all under the same compile-time gate. The one +//! feedback call that lives here is the class-ref arm's, which was a +//! feedback-wrapped helper on that edge before and still is. +//! +//! # Rooting +//! +//! Every call these functions make is in TAIL position: the tag-routing arms, +//! the overflow fallback, and `get_field_ic_miss_impl` are all `return`s, and +//! the two probes above them (`overflow_get`, the named-prefix loads) neither +//! allocate nor enter user code. There is therefore no window in which a raw +//! `obj`/`key` argument is named after a collection point, which is the +//! condition a `RuntimeHandleScope` exists to cover (cf. `js_put_value_set_ic_miss`, +//! which re-reads its key to prime AFTER the allocating `js_put_value_set`). +//! Adding a scope here would root nothing and would put a TLS push/pop on every +//! megamorphic read. If an arm ever grows work after a call, it needs the scope +//! at that moment. + +use crate::object::{ObjectHeader, PicCacheSlot}; +use std::sync::atomic::{AtomicU64, Ordering}; + +/// Cache word 2: the optional Array-subclass class-declared named-prefix +/// token. Mirrors `PIC_NAMED_PREFIX_TOKEN` in +/// `perry-codegen/src/expr/property_get/generic_dispatch.rs`. +const PIC_NAMED_PREFIX_TOKEN: usize = 2; + +/// The exit for a receiver that is NOT a heap pointer: the emitted site's +/// `(tag & 0xFFFD) == 0x7FFD` test just failed. +/// +/// It takes no cache: none of these arms can prime one, which is exactly why +/// splitting them off costs nothing and buys the emitted guard chain back (see +/// the module header). +/// +/// * `obj_bits` — the receiver's full, UNMASKED NaN-box bits. +/// * `key` — the interned property-name `StringHeader`, already masked. +/// * `site_id` — the typed-feedback site id, used only by the class-ref arm. +#[no_mangle] +pub extern "C" fn js_object_get_field_ic_nonptr( + obj_bits: i64, + key: *const crate::StringHeader, + site_id: u64, +) -> f64 { + let bits = obj_bits as u64; + let tag = bits >> 48; + let obj_unmasked = bits as usize as *const ObjectHeader; + + // SSO receiver (SHORT_STRING_TAG): the SSO-aware by-name helper reads + // `.length` from the NaN-box payload and answers undefined otherwise. + // A `.length` site keeps serving this inline and never gets here. + if tag == 0x7FF9 { + return super::js_object_get_field_by_name_f64(obj_unmasked, key); + } + // INT32-tagged class ref: static field / dynamic IIFE-set property / + // synthetic `constructor`. Passes the UNMASKED bits so the runtime can + // detect the tag, exactly as the emitted `pget.recv_class_ref` did. + if tag == 0x7FFE { + return crate::typed_feedback::js_typed_feedback_object_get_field_by_name_f64( + site_id, + obj_unmasked, + key, + ); + } + // `undefined`/`null` throw a node-shaped TypeError (#462). The emitted + // arm passed the property's static bytes and ended in `unreachable`; + // this reads the same bytes off the interned key and the helper is + // `-> !`, so the two are the same divergence with the same message. + if bits == crate::value::TAG_UNDEFINED || bits == crate::value::TAG_NULL { + let is_null = u32::from(bits == crate::value::TAG_NULL); + let (ptr, len) = unsafe { + match super::super::has_own_helpers::str_from_string_header(key) { + Some(s) => (s.as_ptr(), s.len()), + None => (std::ptr::null(), 0), + } + }; + crate::error::js_throw_type_error_property_access(is_null, ptr, len); + } + // Every other tag: no auto-boxing, but the by-name helper still recognizes + // typed shapes (Date `.constructor` via DATE_REGISTRY). A POINTER/STRING + // receiver never reaches this entry from emitted code; if one ever did, + // this arm is the correct — merely non-priming — answer for it too, so + // there is no unverified mode behind the split. + super::js_object_get_field_by_name_f64(obj_unmasked, key) +} + +/// The spill-buffer read, out of line. +/// +/// Kept in its own `#[cold] #[inline(never)]` function for a code-shape reason +/// that is worth 12 instructions on EVERY slow call: inlined, its +/// `overflow_get` call forces `js_object_get_field_ic_slow` to save +/// callee-saved registers across it, which gives that function a real frame +/// (`push rbp/r15/r14/rbx` + the matching pops) and stops the miss handler from +/// being a sibling call. Out of line, the entry's every call is in tail +/// position and it needs no frame at all. +/// +/// Semantics are `js_object_get_field_ic_overflow_load`'s, unchanged: read +/// through `overflow_get`, and on a tombstoned slot fall back to the +/// THREE-argument miss entry — i.e. with a null `packed`, because +/// republishing the packed word here would re-decide which arm the next read +/// of this site takes. +/// +/// # Safety +/// Same contract as the caller: `obj` is a live `GC_TYPE_OBJECT` above the +/// handle band whose shape stamp matched the packed word, and `slot` is that +/// word's high half with `IC_SLOT_OVERFLOW_BIT` set. +#[cold] +#[inline(never)] +unsafe fn overflow_arm( + obj: *const ObjectHeader, + key: *const crate::StringHeader, + cache_slot: *mut PicCacheSlot, + slot: u32, +) -> f64 { + let idx = (slot & !crate::proxy::IC_SLOT_OVERFLOW_BIT) as usize; + if let Some(v) = crate::object::overflow_get(obj as usize, idx) { + if v != crate::value::TAG_HOLE { + return f64::from_bits(v); + } + } + super::ic_miss::get_field_ic_miss_impl(obj, key, cache_slot, std::ptr::null()) +} + +/// The exit for a receiver that IS a heap pointer — every failing guard on the +/// emitted site's object path lands here. +/// +/// * `obj_handle` — the receiver with the NaN-box tag already masked off. The +/// caller has established the POINTER/STRING tag; this entry re-establishes +/// everything below it, because a guard failure says nothing about WHICH +/// guard failed. +/// * `key` — the interned property-name `StringHeader`, already masked. +/// * `cache_slot` — the site's [`PicCacheSlot`] (`@perry_ic_N`), possibly still +/// null: `pic_slot_peek` answers null and the miss handler resolves it when +/// it actually primes. +/// * `packed` — the site's compact MRU word (`@perry_ic_N_packed_get`). +#[no_mangle] +pub extern "C" fn js_object_get_field_ic_slow( + obj_handle: i64, + key: *const crate::StringHeader, + cache_slot: *mut PicCacheSlot, + packed: *const AtomicU64, +) -> f64 { + let obj = obj_handle as usize as *const ObjectHeader; + let addr = obj as usize; + // Below the handle band the emitted code never dereferenced, and neither + // does this: the miss handler routes registry ids through + // HANDLE_PROPERTY_DISPATCH. + if crate::value::addr_class::is_above_handle_band(addr) { + // SAFETY: the caller's emitted `(tag & 0xFFFD) == 0x7FFD` test + // established a POINTER/STRING NaN-box and the address is above the + // handle band — the same licence under which the emitted tower loaded + // this header word inline. + unsafe { + let header = &*((addr - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader); + if header.obj_type == crate::gc::GC_TYPE_OBJECT { + let plain = header._reserved & crate::gc::OBJ_FLAG_HAS_DESCRIPTORS == 0; + // --- 2. the MRU token hit the emitted hit path declined ----- + if plain && !packed.is_null() { + // The emitted guard is `icmp eq i32 %pcid, trunc(%packed)` + // against the RAW header word, with no range check, and + // this is that test: a nonzero packed word always carries a + // valid ShapeId (`prime_get` publishes nothing else), and a + // receiver whose word is an ordinary class id can never + // equal one, so `object_shape_stamp`'s range check would + // only re-derive what the equality already proves (#809's + // keyless receiver fails the equality, not the range test). + let word = (*packed).load(Ordering::Relaxed); + if word != 0 && (*obj).parent_class_id == word as u32 { + let slot = (word >> 32) as u32; + if slot & crate::proxy::IC_SLOT_OVERFLOW_BIT != 0 { + return overflow_arm(obj, key, cache_slot, slot); + } + // An inline slot that reached this entry on a token hit + // was a `TAG_HOLE` — the field was deleted since + // priming. The emitted `pic.hit.deleted` edge took the + // ordinary miss WITH the packed word. + return super::ic_miss::get_field_ic_miss_impl( + obj, key, cache_slot, packed, + ); + } + } + // --- 3. the Array-subclass named-prefix proof -------------- + // An object-backed Array subclass mints a new exact ShapeId on + // every numeric push/pop, but its class-declared named prefix + // does not move. The runtime proves the complete prefix once + // and publishes one token in cache word 2 and in ObjectMeta; + // any structural or descriptor transition clears the owner's. + // + // The conjunction is ordered by what it costs to ASK, not by + // which half the emitted tower read first: `meta` is one load + // from the header line this function has already touched + // (`obj - 8` and `obj + 4`), while the cache word is two + // dependent loads through the site's slot. An object with no + // metadata record cannot carry the token, so testing it first + // retires the overwhelmingly common non-subclass receiver in + // two instructions instead of eight — worth 6 on every miss, + // and the same conjunction either way. + let meta = (*obj).meta; + if !meta.is_null() { + let cache = crate::object::pic_slot_peek(cache_slot); + if !cache.is_null() + && (*cache)[PIC_NAMED_PREFIX_TOKEN] != 0 + && (*meta).array_subclass_named_prefix_token + == (*cache)[PIC_NAMED_PREFIX_TOKEN] as u64 + { + // Word 1 is an INLINE slot whenever word 2 is armed: an + // overflow prime writes word 2 = 0 precisely so this + // address arithmetic can never see one. + let slot = (*cache)[1] as usize; + let field = (obj as *const u8) + .add(std::mem::size_of::() + slot * 8) + as *const f64; + return *field; + } + } + } + } + } + + // --- 4. everything else: the miss handler owns the priming policy ------- + super::ic_miss::get_field_ic_miss_impl(obj, key, cache_slot, packed) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::object::{PicCache, PIC_CACHE_WORDS}; + + fn key_of(bytes: &[u8]) -> *const crate::StringHeader { + crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) + } + + /// What the emitted site passes: the NaN box with its tag masked off. The + /// tag itself was already tested inline, which is why this entry never + /// sees it. + fn handle(obj: *mut ObjectHeader) -> i64 { + (obj as u64 & 0x0000_FFFF_FFFF_FFFF) as i64 + } + + /// A plain own data read that has never primed: the entry must fall all the + /// way through to the miss handler, answer the field, and leave the site + /// primed exactly as the old `js_object_get_field_ic_miss_packed` edge did. + #[test] + fn plain_miss_answers_the_field_and_primes_both_caches() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let obj = scope.root_raw_mut_ptr(crate::object::js_object_alloc(0, 8)); + let key = scope.root_string_ptr(key_of(b"ic_slow_plain")); + obj.with_mut_ptr(|o| { + key.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 7.0)) + }); + + let mut cache: PicCache = [0; PIC_CACHE_WORDS]; + let mut slot: PicCacheSlot = &mut cache; + let packed = AtomicU64::new(0); + let v = obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| js_object_get_field_ic_slow(handle(o), k, &mut slot, &packed)) + }); + assert_eq!(v, 7.0); + assert_ne!(cache[0], 0, "the full cache must be primed by the miss"); + assert_eq!(cache[1], 0, "the first own data field lives in slot 0"); + let word = packed.load(Ordering::Relaxed); + assert_ne!(word, 0, "the compact MRU must be published too"); + assert_eq!(word >> 32, 0, "slot 0, with no overflow bit"); + } + + /// An SSO receiver can never be dereferenced as an ObjectHeader. `.length` + /// is served from the NaN-box payload; any other key answers undefined. + #[test] + fn sso_receiver_routes_to_the_by_name_helper() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let sso = crate::value::JSValue::try_short_string(b"hey").unwrap(); + let key = scope.root_string_ptr(key_of(b"length")); + let v = key.with_const_ptr(|k| js_object_get_field_ic_nonptr(sso.bits() as i64, k, 0)); + assert_eq!(v, 3.0, "the SSO length byte"); + let other = scope.root_string_ptr(key_of(b"nope_not_here")); + let v = other.with_const_ptr(|k| js_object_get_field_ic_nonptr(sso.bits() as i64, k, 0)); + assert_eq!(v.to_bits(), crate::value::TAG_UNDEFINED); + } + + /// The overflow arm, both directions. + /// + /// A field past the inline region primes with `IC_SLOT_OVERFLOW_BIT` + /// (#9287), which is the exact state that sends the emitted MRU hit here + /// instead of doing the `obj + header + slot*8` arithmetic — that address + /// is not where the value lives. The positive direction is the spill read; + /// the negative one is the tombstone, where this entry must fall back to + /// the full miss handler rather than answer the hole. + #[test] + fn an_overflow_slot_reads_through_the_spill_and_falls_back_when_tombstoned() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let obj = scope.root_raw_mut_ptr(crate::object::js_object_alloc(0, 0)); + // INLINE_SLOT_FLOOR is 2, so the fourth key is past the inline region. + let mut keys = Vec::new(); + for (i, name) in [ + b"ic_slow_ovf_a".as_slice(), + b"ic_slow_ovf_b".as_slice(), + b"ic_slow_ovf_c".as_slice(), + b"ic_slow_ovf_d".as_slice(), + ] + .iter() + .enumerate() + { + let k = scope.root_string_ptr(key_of(name)); + obj.with_mut_ptr(|o| { + k.with_const_ptr(|kp| crate::object::js_object_set_field_by_name(o, kp, i as f64)) + }); + keys.push(k); + } + let key = keys.last().expect("four keys"); + + let mut cache: PicCache = [0; PIC_CACHE_WORDS]; + let mut slot: PicCacheSlot = &mut cache; + let packed = AtomicU64::new(0); + let read = |slot: &mut PicCacheSlot, packed: &AtomicU64| { + obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| js_object_get_field_ic_slow(handle(o), k, slot, packed)) + }) + }; + assert_eq!(read(&mut slot, &packed), 3.0, "the priming read"); + let word = packed.load(Ordering::Relaxed); + assert_ne!(word, 0, "test premise: the site primed"); + assert_ne!( + (word >> 32) as u32 & crate::proxy::IC_SLOT_OVERFLOW_BIT, + 0, + "test premise: the fourth field must live past the inline region, \ + or this test never reaches the overflow arm (packed word {word:#x})" + ); + // The hit the emitted code now delegates: same packed pair, read + // through `overflow_get`. + assert_eq!(read(&mut slot, &packed), 3.0, "the overflow-arm read"); + + // Tombstone it. The spill slot no longer holds a value, so the arm must + // decline and let the full miss handler answer for the prototype chain. + obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| { + assert_eq!(crate::object::js_object_delete_field(o, k), 1); + }) + }); + assert_eq!( + read(&mut slot, &packed).to_bits(), + crate::value::TAG_UNDEFINED, + "a tombstoned overflow slot must miss, not answer the hole" + ); + } + + /// A non-nullish, non-pointer receiver keeps the old fall-through: no + /// throw, no cache, `undefined` unless the runtime knows the typed shape. + #[test] + fn plain_number_receiver_returns_undefined_without_priming() { + let _lock = crate::gc::global_side_table_test_lock(); + let key = key_of(b"ic_slow_on_a_double"); + let v = js_object_get_field_ic_nonptr(3.5f64.to_bits() as i64, key, 0); + assert_eq!(v.to_bits(), crate::value::TAG_UNDEFINED); + } + + /// The nullish arm must reach `js_throw_type_error_property_access` with + /// the key's bytes. `js_throw` exits the process when `TRY_DEPTH == 0`, so + /// the assertion here is on the pre-throw routing decision: `undefined` and + /// `null` are the only two bit patterns that take it, which is what the + /// emitted `pget.throw_nullish` predicate tested. + #[test] + fn only_undefined_and_null_take_the_throwing_arm() { + for bits in [crate::value::TAG_UNDEFINED, crate::value::TAG_NULL] { + assert!( + (bits >> 48) & 0xFFFD != 0x7FFD, + "a nullish tag must not look like a heap pointer" + ); + } + // TAG_FALSE/TAG_TRUE sit in the same 0x7FFC band and must NOT throw. + let key = key_of(b"ic_slow_on_a_bool"); + let v = js_object_get_field_ic_nonptr(crate::value::TAG_TRUE as i64, key, 0); + assert_eq!(v.to_bits(), crate::value::TAG_UNDEFINED); + } + + /// A hole in the primed inline slot (the field was deleted) must take the + /// ordinary miss and answer what the full lookup answers — never the raw + /// `TAG_HOLE` word. + #[test] + fn a_deleted_inline_slot_takes_the_ordinary_miss() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let obj = scope.root_raw_mut_ptr(crate::object::js_object_alloc(0, 8)); + let key = scope.root_string_ptr(key_of(b"ic_slow_deleted")); + obj.with_mut_ptr(|o| { + key.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 5.0)) + }); + + let mut cache: PicCache = [0; PIC_CACHE_WORDS]; + let mut slot: PicCacheSlot = &mut cache; + let packed = AtomicU64::new(0); + let first = obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| js_object_get_field_ic_slow(handle(o), k, &mut slot, &packed)) + }); + assert_eq!(first, 5.0); + assert_ne!(packed.load(Ordering::Relaxed), 0, "test premise: primed"); + + obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| { + assert_eq!(crate::object::js_object_delete_field(o, k), 1); + }) + }); + let after = obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| js_object_get_field_ic_slow(handle(o), k, &mut slot, &packed)) + }); + assert_eq!( + after.to_bits(), + crate::value::TAG_UNDEFINED, + "a deleted field reads undefined, not the hole word" + ); + } + + /// The named-prefix arm: an armed cache word 2 that matches the receiver's + /// ObjectMeta token serves the cached slot WITHOUT consulting the exact + /// ShapeId — that is the whole point of the proof, since an object-backed + /// Array subclass re-shapes on every numeric write. + /// + /// Driven directly against the two words the emitted ladder read, so it + /// pins this entry's contract with codegen rather than the subclass + /// machinery that publishes the token. + #[test] + fn an_armed_named_prefix_serves_the_cached_slot() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let obj = scope.root_raw_mut_ptr(crate::object::js_object_alloc(0, 8)); + let key = scope.root_string_ptr(key_of(b"ic_slow_prefix")); + obj.with_mut_ptr(|o| { + key.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 11.0)) + }); + // Give the receiver a meta record and an arbitrary prefix token. + const TOKEN: u64 = 0xA11CE; + let (meta, _) = obj.across_mut::(|| unsafe { + crate::object::object_meta_ensure(obj.get_raw_mut_ptr::()) + }); + assert!( + !meta.is_null(), + "test premise: the receiver has a meta record" + ); + unsafe { (*meta).array_subclass_named_prefix_token = TOKEN }; + + let mut cache: PicCache = [0; PIC_CACHE_WORDS]; + cache[PIC_NAMED_PREFIX_TOKEN] = TOKEN as i64; + cache[1] = 0; // the field's inline slot + // Word 0 stays 0 and `packed` stays 0, so the exact-ShapeId + // arm cannot serve this read: only the prefix proof can. + let mut slot: PicCacheSlot = &mut cache; + let packed = AtomicU64::new(0); + let v = obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| js_object_get_field_ic_slow(handle(o), k, &mut slot, &packed)) + }); + assert_eq!(v, 11.0); + assert_eq!( + packed.load(Ordering::Relaxed), + 0, + "a prefix hit answers from the cache and must not prime" + ); + + // A token that no longer matches the receiver's must fall through to + // the ordinary lookup (which still answers, and primes). + cache[PIC_NAMED_PREFIX_TOKEN] = (TOKEN + 1) as i64; + let v = obj.with_mut_ptr(|o: *mut ObjectHeader| { + key.with_const_ptr(|k| js_object_get_field_ic_slow(handle(o), k, &mut slot, &packed)) + }); + assert_eq!(v, 11.0); + assert_ne!( + packed.load(Ordering::Relaxed), + 0, + "a stale prefix token must reach the priming miss handler" + ); + } + + /// A site whose cache slot has not been resolved yet reads as a null + /// cache, and the named-prefix arm must not dereference it. + /// + /// The read is of a key the receiver does NOT own, so the miss handler + /// answers from the prototype chain without priming — which is what keeps + /// this test on the arm it is about. (Codegen always passes the address of + /// a real `@perry_ic_N` global; a genuinely null slot only ever reaches a + /// runtime entry from a test or from the write PIC's poly tail.) + #[test] + fn an_unresolved_cache_slot_is_never_dereferenced() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let obj = scope.root_raw_mut_ptr(crate::object::js_object_alloc(0, 8)); + let own = scope.root_string_ptr(key_of(b"ic_slow_present")); + obj.with_mut_ptr(|o| { + own.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 3.0)) + }); + let absent = scope.root_string_ptr(key_of(b"ic_slow_absent")); + let packed = AtomicU64::new(0); + // A never-published slot: `pic_slot_peek` must answer null and the + // prefix arm must skip rather than read word 2 out of nothing. + let mut slot: PicCacheSlot = std::ptr::null_mut(); + let v = obj.with_mut_ptr(|o: *mut ObjectHeader| { + absent.with_const_ptr(|k| js_object_get_field_ic_slow(handle(o), k, &mut slot, &packed)) + }); + assert_eq!(v.to_bits(), crate::value::TAG_UNDEFINED); + assert!(slot.is_null(), "an absent key must not resolve a cache"); + assert_eq!(packed.load(Ordering::Relaxed), 0, "and must not prime"); + } +} diff --git a/crates/perry-transform/src/prop_cse.rs b/crates/perry-transform/src/prop_cse.rs index 7678c2d65f..3b0089fd9f 100644 --- a/crates/perry-transform/src/prop_cse.rs +++ b/crates/perry-transform/src/prop_cse.rs @@ -15,18 +15,18 @@ //! ``` //! //! Every one of those `n.kind` reads compiles to a **full generic -//! property-get diamond** — `pget.recv_sso` / `pget.check_class_ref` / -//! `pget.recv_ok` / the monomorphic-IC guard ladder / `pic.hit` / `pic.miss` / -//! `pget.recv_merge` — about fifty instructions and six branches per site +//! property-get tower** — `pget.recv_ok` / the monomorphic-IC guard ladder / +//! `pic.hit` / `pic.miss` / the polymorphic ways / `pic.miss.call` / +//! `pget.recv_merge` — about forty instructions and six branches per site //! (`expr/property_get/generic_dispatch.rs`). `gc-handoff/apps/interp.ts`'s //! `evalNode` has **53 such sites**, of which `n.kind` accounts for seven and //! `n.op` for seven more. //! //! LLVM `-O3` cannot remove them. GVN dedupes *instructions*, not congruent //! control-flow regions, and every arm of the diamond that is not the inline -//! fast load ends in an opaque call (`js_object_get_field_ic_miss`, -//! `js_object_get_field_by_name_f64`) whose memory effects license nothing. So -//! the redundancy has to be removed before it becomes a diamond — here. +//! fast load ends in an opaque call (`js_object_get_field_ic_slow`) whose +//! memory effects license nothing. So +//! the redundancy has to be removed before it becomes a tower — here. //! //! # The rewrite //! diff --git a/docs/src/internals/gc-rooting-invariant.md b/docs/src/internals/gc-rooting-invariant.md index 78dbcf7ff9..20aab1cb9f 100644 --- a/docs/src/internals/gc-rooting-invariant.md +++ b/docs/src/internals/gc-rooting-invariant.md @@ -275,6 +275,8 @@ helper called from a loop body is in-loop too. `movers` and `UnrootedAlloca`) counts `js_gc_loop_safepoint`, anything in `poll_reaching`, **and** anything in `POLL_CAPABLE_RUNTIME` — the runtime helpers that can re-enter JS, such as `js_object_set_field_by_name`, +`js_object_get_field_ic_slow` (the generic property-GET tower's single slow +exit, and since T1 the only GET symbol most emitted sites carry), `js_object_get_field_ic_miss` and `js_closure_call1`. Those are moving with no poll anywhere near them. @@ -293,7 +295,11 @@ like coverage while doing it. Two rounds of this have now been measured: SET verbatim but lowers every GET to `js_object_get_field_by_name_f64`, `js_object_get_field_ic_miss` or `js_typed_feedback_object_get_field_by_name_f64`, none of which were in the - set. Property sets classified `MOVING: YES`, property gets classified + set. (T1 later collapsed the generic tower's six cold arms into one + `js_object_get_field_ic_slow` call, which is in the set for the same reason + and is now the GET symbol a stale-register window is most likely to span — + a size optimisation is exactly the kind of change that can silently retire + the symbol an audit is keyed on.) Property sets classified `MOVING: YES`, property gets classified `MOVING: no`, and 31 `--stale-registers` hits on the gate corpus were dropped by `--moving-only` as a result — including the shape that faults deterministically under `PERRY_GC_PROTECT_FROMSPACE=1 diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py index d84642e652..015a5f9590 100755 --- a/scripts/gc_root_dominance_check.py +++ b/scripts/gc_root_dominance_check.py @@ -1261,6 +1261,16 @@ def is_collecting(callee): # Same getter/Proxy-capable implementation as the retained three-arg ABI; # the fourth argument only adds a numeric packed cache publication. "js_object_get_field_ic_miss_packed", + # T1: the two slow exits of the inline generic-get tower, and now the ONLY + # property-GET symbols most emitted sites carry. `_slow` reaches + # `get_field_ic_miss_impl` (hence `js_object_get_field_by_name`, hence a + # user getter and `js_proxy_get`); `_nonptr` reaches + # `js_object_get_field_by_name_f64` on every arm. A register held across + # either is exactly as stale as one held across the entries they replaced. + # Omitting them would silently re-open the #7154 GET hole that this set + # exists to close, because the calls they replaced no longer appear at + # those sites. + "js_object_get_field_ic_slow", "js_object_get_field_ic_nonptr", "js_typed_feedback_object_get_field_by_name_f64", "js_array_sort_default", "js_array_sort_with_comparator", "js_array_map", "js_array_filter", "js_typed_array_for_each", @@ -5057,6 +5067,8 @@ def self_test(): _pget_family = {"js_object_get_field_by_name_f64", "js_object_get_field_ic_miss", "js_object_get_field_ic_miss_packed", + "js_object_get_field_ic_slow", + "js_object_get_field_ic_nonptr", "js_typed_feedback_object_get_field_by_name_f64"} if not _pget_family <= POLL_CAPABLE_RUNTIME: print("self-test FAIL: the emitted property-GET dispatch " @@ -5165,27 +5177,36 @@ def self_test(): "code shape rather than on the staleness.", file=sys.stderr) ok = False - # The packed entry is the name emitted by generic property reads now. - # Keeping only the old ABI above made --moving-only silently discard - # the same stale-slot hazard even though both call the same getter. - for label, source, want in ( - ("stale", _SELFTEST_PROPERTY_GET_WINDOW, 1), - ("reloaded", _SELFTEST_PROPERTY_GET_RELOADED, 0)): - packed = os.path.join(td, f"property_get_packed_{label}.ll") - source = source.replace( - "@js_object_get_field_ic_miss(", - "@js_object_get_field_ic_miss_packed(").replace( - "ptr @perry_ic_1)", - "ptr @perry_ic_1, ptr @perry_ic_packed_1)") - with open(packed, "w") as fh: - fh.write(source) - for moving_only in (False, True): - found = _stale_kinds_probe(packed, moving_only=moving_only) - if found.get("slotload", 0) != want: - print(f"self-test FAIL: packed GET {label}, " - f"moving_only={moving_only}: expected {want} " - f"slotload hazards, got {found!r}", file=sys.stderr) - ok = False + # The packed entry is the name emitted by generic property reads now, + # and since T1 the SLOW entry is the only one most sites carry at all: + # the tower's six other calls collapsed into it, so a set that named + # only the older ABIs would classify almost every real property-GET + # window `MOVING: no` — the exact #7154 hole, reopened by a size + # optimisation rather than by a misspelling. Both are asserted in both + # directions against the same fixture. + for entry, extra_args in ( + ("js_object_get_field_ic_miss_packed", + "ptr @perry_ic_1, ptr @perry_ic_packed_1)"), + ("js_object_get_field_ic_slow", + "ptr @perry_ic_1, ptr @perry_ic_packed_1)"), + ("js_object_get_field_ic_nonptr", "i64 4242)")): + for label, source, want in ( + ("stale", _SELFTEST_PROPERTY_GET_WINDOW, 1), + ("reloaded", _SELFTEST_PROPERTY_GET_RELOADED, 0)): + packed = os.path.join(td, f"property_get_{entry}_{label}.ll") + source = source.replace( + "@js_object_get_field_ic_miss(", + "@%s(" % entry).replace( + "ptr @perry_ic_1)", extra_args) + with open(packed, "w") as fh: + fh.write(source) + for moving_only in (False, True): + found = _stale_kinds_probe(packed, moving_only=moving_only) + if found.get("slotload", 0) != want: + print(f"self-test FAIL: {entry} GET {label}, " + f"moving_only={moving_only}: expected {want} " + f"slotload hazards, got {found!r}", file=sys.stderr) + ok = False if (not is_collecting("js_string_compare_value") or is_collecting("js_string_compare")): print("self-test FAIL: only the raw primitive string comparison " diff --git a/scripts/shape_descriptor_census.py b/scripts/shape_descriptor_census.py index 0626294170..9815d926e0 100644 --- a/scripts/shape_descriptor_census.py +++ b/scripts/shape_descriptor_census.py @@ -690,7 +690,7 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: for fragment in ( 'letpacked_present=ctx.block().icmp_ne(I64,&packed_word,"0");', 'letis_plain_object=ctx.block().and(I1,&is_plain_kind,&packed_present);', - 'cond_br(&is_plain_object,&tok_label,&desc_classify_label)', + 'cond_br(&is_plain_object,&tok_label,&cold_label)', 'letpacked_stamp=ctx.block().trunc(I64,&packed_word,I32);', 'lettoken_eq=ctx.block().icmp_eq(I32,&pcid,&packed_stamp);', 'cond_br(&token_eq,&hit_label,&token_miss_label)', diff --git a/test-files/test_gap_generic_get_one_exit_arms.ts b/test-files/test_gap_generic_get_one_exit_arms.ts new file mode 100644 index 0000000000..9c831842bd --- /dev/null +++ b/test-files/test_gap_generic_get_one_exit_arms.ts @@ -0,0 +1,131 @@ +// Every arm of the generic property-get tower, against node's own output. +// +// `lower_generic_property_get` used to expand each of these as its own inline +// block group with its own runtime call: the SSO receiver, the heap-string +// receiver, the INT32 class ref, the nullish TypeError, the non-object +// receiver, the native Map/Set `.size`, the overflow slot, the deleted slot, +// and the Array-subclass named-prefix proof. They now live behind two runtime +// entries (`js_object_get_field_ic_nonptr` for a receiver that is not a heap +// pointer, `js_object_get_field_ic_slow` for one that is), so a defect in any +// of them is no longer a defect in emitted code that some other test might +// notice — it is a defect in one shared function, and this is the witness that +// each arm still answers what node answers. +// +// Every read below goes through `read()`, whose parameter is `any`: that is +// what denies the front end a type proof and lands the access in the generic +// tower rather than in one of the specialised lowerings. + +function read(o: any, k: string): any { + // One generic `obj.prop` site per arm. + if (k === "a") return o.a; + if (k === "length") return o.length; + if (k === "size") return o.size; + if (k === "kind") return o.kind; + if (k === "charCodeAt") return o.charCodeAt; + return o.zzz; +} + +// --- receiver tags that are not heap pointers ------------------------------- + +// SSO string (short enough to live inside the NaN box) and a heap string. +const sso: any = "hi"; +const heap: any = "a considerably longer string"; +console.log("sso.length", read(sso, "length")); +console.log("heap.length", read(heap, "length")); +console.log("sso.charCodeAt is fn", typeof read(sso, "charCodeAt")); +console.log("heap.zzz", read(heap, "zzz")); + +// A class object read as a value: its static field, not an instance field. +class C { + static kind = "static-c"; + x = 1; +} +const cref: any = C; +console.log("C.kind", read(cref, "kind")); + +// Non-nullish primitives: no auto-boxing, no throw, just `undefined`. +console.log("3.5.zzz", read(3.5 as any, "zzz")); +console.log("true.zzz", read(true as any, "zzz")); + +// --- heap receivers that are not ordinary objects --------------------------- + +const m: any = new Map(); +m.set(1, 2); +const st: any = new Set(); +st.add(9); +console.log("map.size", read(m, "size"), "set.size", read(st, "size")); + +const arr: any = [1, 2, 3]; +console.log("arr.length", read(arr, "length")); + +// --- ordinary objects: hit, hole, re-add ------------------------------------ + +const o: any = { a: 1, b: 2 }; +console.log("o.a", read(o, "a")); +delete o.a; +console.log("o.a deleted", read(o, "a")); +o.a = 7; +console.log("o.a again", read(o, "a")); + +// A field past the inline region (the overflow slot), read repeatedly so the +// site primes and then takes the primed path, then tombstoned. +const big: any = {}; +big.p0 = 0; +big.p1 = 1; +big.p2 = 2; +big.p3 = 3; +big.p4 = 4; +for (let i = 0; i < 3; i++) console.log("big.p4", big.p4); +delete big.p4; +console.log("big.p4 deleted", big.p4); + +// Absent own key: the prototype chain answers. +const proto = { inherited: "yes" }; +const child: any = Object.create(proto); +console.log("child.inherited", child.inherited); + +// A descriptor-bearing receiver: the accessor must fire on EVERY read, which +// is what forbids a raw-slot hit from ever being primed for it. +let calls = 0; +const acc: any = {}; +Object.defineProperty(acc, "g", { + get() { + calls += 1; + return calls; + }, +}); +console.log("acc.g", acc.g, acc.g, acc.g, "calls", calls); + +// --- nullish receivers throw a node-shaped TypeError ------------------------ + +try { + const n: any = null; + console.log(n.foo); +} catch (e: any) { + console.log("caught null:", e.message); +} +try { + const u: any = undefined; + console.log(u.bar); +} catch (e: any) { + console.log("caught undefined:", e.message); +} + +// --- shape rotation: the bounded polymorphic ways --------------------------- + +class S1 { + x = 1; +} +class S2 { + a = 0; + x = 2; +} +class S3 { + a = 0; + b = 0; + x = 3; +} +const rot: any[] = [new S1(), new S2(), new S3()]; +let sum = 0; +for (let i = 0; i < 300; i++) sum += rot[i % rot.length].x; +console.log("poly sum", sum); From fe2a9cfd94c5555ea11df88c2d81063298fa1425 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 14:00:05 +0200 Subject: [PATCH 2/2] changelog: fragment for #10196 --- changelog.d/10196-generic-get-two-exits.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 changelog.d/10196-generic-get-two-exits.md diff --git a/changelog.d/10196-generic-get-two-exits.md b/changelog.d/10196-generic-get-two-exits.md new file mode 100644 index 0000000000..9ba6cff0dc --- /dev/null +++ b/changelog.d/10196-generic-get-two-exits.md @@ -0,0 +1,12 @@ +Collapse the generic (untyped) property-get inline cache from a 33-block diamond +with six runtime call sites to the inline hit path plus the polymorphic ways and +two out-of-line exits: `js_object_get_field_ic_nonptr` for non-pointer receivers +(SSO, class refs, nullish throw, primitives) and `js_object_get_field_ic_slow` +for a pointer receiver that failed a guard (overflow slot, Array-subclass +named-prefix proofs, miss and prime). Per site: 191 → 106 IR instructions, +6 → 2 call sites. The packed-MRU hit path, the ways and every cache decision are +unchanged; the field address is emitted as a typed `gep` so instruction selection +keeps the scaled addressing mode. On @babel/parser `.text` shrinks 14.4 % and +`.perry_gcmap` 11.7 %; total benchmark instructions drop 0.5 % (`interp.ts` +−1.5 %) with RSS unchanged. A new gap test exercises every arm that moved out of +line against Node.