From 84ee63ff10d51f86c322bda46f4583539e3fbf4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 08:31:25 +0200 Subject: [PATCH 1/7] feat(runtime): prove the element shape of class-0 record arrays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-array homogeneous element-shape invariant (#7480) keyed every proof on a class id, and `element_identity_of_bits` refused `class_id == 0` outright. Every `JSON.parse`'d record is class 0 with an ordinary birth ShapeId (`object/json_construction.rs`), so no parsed record array could ever carry a proof — the one array shape the invariant's consumer most wants to reason about was structurally excluded. Admit class 0, keyed on the EXACT ordinary ShapeId the descriptor probe already validated. That identity is strictly narrower than the class-level one it replaces, and it has to be: "same class" is vacuous when the class is 0, so `element_matches_record` now declines the two class-level fallbacks for a class-0 record rather than relying on both of them failing closed by coincidence. Two new generated-code-facing entry points: * `js_array_ensure_element_shape_ordinary` — establish-or-confirm, returning the proven ordinary ShapeId for a class-0 proof and 0 for a class-keyed one. The two proofs are deliberately not interchangeable: a class-keyed record matches at class level, so its `ordinary_shape_id` is the first element's shape and not a per-element guarantee. * `js_shape_ordinary_inline_slot_for_key` — the inline slot a PLAIN ordinary shape assigns to a key, or -1. Four conjuncts make "slot k == key position k" true (ordinary kind, generation 0, no holes, every key inline); dropping any one would produce a wrong offset rather than a missed optimization, so each is asserted separately. The key arrives as the whole NaN-box rather than a masked pointer, because a short property name reaches the string pool as an SSO immediate whose masked low bits are packed characters and not an address. `js_array_ensure_element_shape` still returns the class id, so every existing consumer reads a class-0 proof exactly as "no proof". --- .../perry-runtime/src/array/element_shape.rs | 80 +++++- .../src/array/element_shape_tests.rs | 262 ++++++++++++++++++ crates/perry-runtime/src/object/shapes.rs | 81 ++++++ .../perry-runtime/src/object/shapes_tests.rs | 50 ++++ 4 files changed, 462 insertions(+), 11 deletions(-) diff --git a/crates/perry-runtime/src/array/element_shape.rs b/crates/perry-runtime/src/array/element_shape.rs index 29f0f245cc..1fa9316ca2 100644 --- a/crates/perry-runtime/src/array/element_shape.rs +++ b/crates/perry-runtime/src/array/element_shape.rs @@ -143,6 +143,11 @@ struct ElementShapeRecord { #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) struct ElementShapeProof { pub(crate) class_id: u32, + /// The exact ordinary ShapeId every element carries. Meaningful for a + /// class-keyed proof too, but it is the WHOLE identity of a `class_id == 0` + /// one (#10123) — the element-shape loop clone keys its per-element + /// residual check on it. + pub(crate) ordinary_shape_id: u32, pub(crate) verified_len: u32, pub(crate) epoch: u64, } @@ -289,11 +294,23 @@ fn element_identity_of_bits(value_bits: u64) -> Option<(u32, u32)> { }) { return None; } - let class_id = (*obj).class_id; - if class_id == 0 { - return None; - } - Some((class_id, shape_id)) + // #10123: a class-0 ordinary object is admitted, keyed on the exact + // ShapeId the descriptor probe above just validated. Every + // `JSON.parse`'d record is one (`object/json_construction.rs` stamps + // `class_id = 0` and an ordinary birth shape), and refusing them here + // is the reason no element-shape proof was ever established for a + // parsed record array. + // + // The identity is never `(0, 0)`: `shape_descriptor_by_id` answers + // `None` for id 0, so the `Ordinary` test above already rejected it. + // And a class-0 proof cannot be mistaken for a class-keyed one by an + // existing consumer, because every one of them compares against a + // NONZERO class id — `js_array_ensure_element_shape` keeps returning + // `class_id`, so a class-0 proof reads to them exactly as "no proof". + // What makes the identity usable is that a class-0 record's shape is + // compared EXACTLY (see `element_matches_record`), which is strictly + // narrower than the class-level match. + Some(((*obj).class_id, shape_id)) } } @@ -326,6 +343,18 @@ fn element_matches_record(value_bits: u64, record: ElementShapeRecord) -> bool { EXACT_SHAPE_STORE_HITS.with(|hits| hits.set(hits.get().wrapping_add(1))); return true; } + // #10123: a class-0 proof is the exact ShapeId and NOTHING else. The + // two fallbacks below are class-level, and "same class" is vacuous for + // class 0 — every plain object shares it — so reaching them would + // widen a shape proof into no proof at all. Both already fail closed + // for class 0 (`array_subclass_named_prefix_token_matches_class` + // returns false, and `element_identity_of_validated_object` requires a + // nonzero class), and `class_zero_record_declines_a_different_shape` + // pins that; this makes the rule local rather than a coincidence of + // two other functions. + if record.class_id == 0 { + return false; + } // Object-backed Array subclasses publish a move-stable, class-wide // ordinary-prefix proof precisely because their numeric tail mints a // different ShapeId on every push/pop. Once present, that token is a @@ -498,6 +527,7 @@ pub(crate) unsafe fn element_shape_proof(arr: *const ArrayHeader) -> Option i32 { unsafe { ensure_element_shape(arr).map_or(0, |p| p.class_id as i32) } } +/// #10123: establish-or-confirm for a **class-0** (plain-object) element +/// array, returning the exact ordinary ShapeId every element carries, or `0`. +/// +/// The sibling above answers the class question and therefore answers `0` for +/// a `JSON.parse`'d record array — correctly, since those records have no +/// class. This entry point answers the question that array CAN answer: they +/// all carry one exact ShapeId. A class-KEYED proof deliberately reports `0` +/// here rather than its shape id: the two proofs are not interchangeable +/// (`element_matches_record` matches a class-keyed record at class level, so +/// its `ordinary_shape_id` is the shape of the first element and not a +/// per-element guarantee), and handing one out would licence a shape-keyed +/// clone on a proof that never checked shapes. +#[no_mangle] +pub extern "C" fn js_array_ensure_element_shape_ordinary(arr: *mut ArrayHeader) -> i32 { + unsafe { + match ensure_element_shape(arr) { + Some(proof) if proof.class_id == 0 => proof.ordinary_shape_id as i32, + _ => 0, + } + } +} + /// The O(1) query with no scan: the proven `class_id`, or `0`. #[no_mangle] pub extern "C" fn js_array_element_shape_class(arr: *const ArrayHeader) -> i32 { @@ -776,19 +828,25 @@ pub extern "C" fn js_array_element_shape_check( } } -// NOTE — exactly ONE `keepalive-anchors` `#[used]` static, deliberately. +// NOTE — anchor EXACTLY the entries codegen emits a call to, and no more. // `keepalive-anchors` is a DEFAULT feature, so an anchor pins its symbol into -// every shipped binary: anchoring all five would be the dead-strip defeat the +// every shipped binary: anchoring all six would be the dead-strip defeat the // hello-size campaign traced its regression to. #5093's element-shape -// versioned-loop clone emits a call to exactly one of them -// (`js_array_ensure_element_shape`, from the loop preheader), so that one — -// and only that one — is anchored. The other four stay unanchored and -// dead-strippable until something emits a call to them. +// versioned-loop clone emits a call from its preheader to +// `js_array_ensure_element_shape` (the class-keyed arm) or to +// `js_array_ensure_element_shape_ordinary` (#10123's shape-keyed arm), so +// those two — and only those two — are anchored. The other four stay +// unanchored and dead-strippable until something emits a call to them. #[cfg(feature = "keepalive-anchors")] #[used] static KEEP_ARRAY_ENSURE_ELEMENT_SHAPE: extern "C" fn(*mut ArrayHeader) -> i32 = js_array_ensure_element_shape; +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_ARRAY_ENSURE_ELEMENT_SHAPE_ORDINARY: extern "C" fn(*mut ArrayHeader) -> i32 = + js_array_ensure_element_shape_ordinary; + #[cfg(test)] pub(crate) fn test_element_shape_record_exists(owner: usize) -> bool { ELEMENT_SHAPES.with(|m| m.borrow().contains_key(&owner)) diff --git a/crates/perry-runtime/src/array/element_shape_tests.rs b/crates/perry-runtime/src/array/element_shape_tests.rs index 2fe5bc67e6..c90194d586 100644 --- a/crates/perry-runtime/src/array/element_shape_tests.rs +++ b/crates/perry-runtime/src/array/element_shape_tests.rs @@ -661,3 +661,265 @@ fn pruning_dead_owners_removes_their_records() { "pruning must not disturb a live array's proof" ); } + +// --------------------------------------------------------------------------- +// #10123 — CLASS-0 (plain-object) element arrays. +// +// A `JSON.parse`'d record has `class_id == 0` and an ordinary birth ShapeId +// (`object/json_construction.rs`), so the class-keyed invariant declined every +// parsed record array: `element_identity_of_bits` refused class 0 outright and +// no proof was ever established. The proof below is keyed on the EXACT ShapeId +// instead, which is strictly narrower than a class-level match — and it has to +// be, because "same class" is vacuous when the class is 0. +// --------------------------------------------------------------------------- + +/// A keys array of heap-allocated interned property names, the shape a +/// parser's canonical keys array has. +fn keys_array(names: &[&str]) -> *mut ArrayHeader { + let keys = crate::array::js_array_alloc_with_length(names.len() as u32); + for (index, name) in names.iter().enumerate() { + let string = crate::js_string_from_bytes(name.as_ptr(), name.len() as u32); + crate::array::js_array_set(keys, index as u32, crate::JSValue::string_ptr(string)); + } + keys +} + +/// `(keys, shape_id)` for a plain ordinary shape over `names`. +fn record_shape(names: &[&str]) -> (*mut ArrayHeader, u32) { + let keys = keys_array(names); + let shape_id = crate::object::shapes::shape_id_for_keys_ensure(keys, names.len() as u32); + assert_ne!(shape_id, 0, "test premise: the keys must mint a shape"); + (keys, shape_id) +} + +/// One class-0 ordinary record, birthed exactly the way +/// `object/json_construction.rs` births a parsed one: class 0, the shape +/// stamped from the canonical keys array, values in key order. +fn json_like_record(keys: *mut ArrayHeader, shape_id: u32, field_count: u32) -> f64 { + let obj = + crate::object::js_object_alloc_class_inline_keys_stamped(0, 0, field_count, keys, shape_id); + unsafe { + assert_eq!( + (*obj).class_id, + 0, + "test premise: the fixture record must be class 0" + ); + assert_eq!( + (*obj).parent_class_id, + shape_id, + "test premise: the fixture record must carry the requested shape" + ); + } + crate::value::js_nanbox_pointer(obj as i64) +} + +#[test] +fn a_homogeneous_class_zero_record_array_proves_on_its_shape_id() { + let _serialized = test_serialize(); + let (keys, shape_id) = record_shape(&["id", "name"]); + let mut arr = js_array_alloc(4); + for _ in 0..4 { + arr = push(arr, json_like_record(keys, shape_id, 2)); + } + + let established = unsafe { ensure_element_shape(arr) } + .expect("a homogeneous class-0 record array must prove (#10123)"); + assert_eq!( + established.class_id, 0, + "the proof stays class 0 — every class-keyed consumer must read it as `no class`" + ); + assert_eq!(established.ordinary_shape_id, shape_id); + assert_eq!(established.verified_len, 4); + + // The FFI the clone's shape-keyed preheader calls. + assert_eq!( + js_array_ensure_element_shape_ordinary(arr), + shape_id as i32, + "the shape-keyed entry point must hand back the exact ShapeId" + ); + // ... and the class-keyed one still answers 0, so a class-keyed preheader + // comparing against its own (nonzero) class id takes the slow clone. + assert_eq!( + js_array_ensure_element_shape(arr), + 0, + "a class-0 proof must never read as a class proof" + ); +} + +#[test] +fn a_mixed_shape_class_zero_record_array_declines() { + let _serialized = test_serialize(); + let (keys_a, shape_a) = record_shape(&["id", "name"]); + let (keys_b, shape_b) = record_shape(&["id", "name", "extra"]); + assert_ne!(shape_a, shape_b, "test premise: two distinct shapes"); + let mut arr = js_array_alloc(4); + arr = push(arr, json_like_record(keys_a, shape_a, 2)); + arr = push(arr, json_like_record(keys_a, shape_a, 2)); + arr = push(arr, json_like_record(keys_b, shape_b, 3)); + + assert!( + unsafe { ensure_element_shape(arr) }.is_none(), + "a record with a DIFFERENT key set must decline the whole array — this is \ + the heterogeneous JSON case, and admitting it would licence a field read \ + at a slot the second shape does not have" + ); + assert_eq!(js_array_ensure_element_shape_ordinary(arr), 0); +} + +#[test] +fn class_zero_record_declines_a_different_shape() { + // The load-bearing narrowing: `element_matches_record`'s class-level + // fallbacks are vacuous for class 0 (every plain object shares it), so a + // class-0 record is matched on its exact ShapeId and nothing else. + let _serialized = test_serialize(); + let (keys_a, shape_a) = record_shape(&["id", "name"]); + let (keys_b, shape_b) = record_shape(&["id", "flag"]); + assert_ne!(shape_a, shape_b); + let mut arr = js_array_alloc(4); + for _ in 0..3 { + arr = push(arr, json_like_record(keys_a, shape_a, 2)); + } + assert_eq!( + unsafe { ensure_element_shape(arr) } + .expect("proven") + .ordinary_shape_id, + shape_a + ); + + js_array_set_f64(arr, 1, json_like_record(keys_b, shape_b, 2)); + + assert!( + proof(arr).is_none(), + "a same-class (class 0) but different-SHAPE store must clear the proof" + ); + unsafe { assert!(!test_element_shape_bit_set(arr)) }; +} + +#[test] +fn class_zero_proof_keeps_a_same_shape_store_and_clears_a_primitive_one() { + let _serialized = test_serialize(); + let (keys, shape_id) = record_shape(&["id", "name"]); + let mut arr = js_array_alloc(4); + for _ in 0..3 { + arr = push(arr, json_like_record(keys, shape_id, 2)); + } + let established = unsafe { ensure_element_shape(arr) }.expect("proven"); + + js_array_set_f64(arr, 1, json_like_record(keys, shape_id, 2)); + let kept = proof(arr).expect("a same-shape store must keep the proof"); + assert_eq!(kept.epoch, established.epoch, "the proof identity survives"); + assert_eq!(kept.ordinary_shape_id, shape_id); + + js_array_set_f64(arr, 1, 42.0); + assert!( + proof(arr).is_none(), + "a primitive store must clear a class-0 proof like any other" + ); +} + +#[test] +fn a_class_keyed_proof_reports_no_ordinary_shape_id_to_the_shape_keyed_entry() { + // The two proofs are not interchangeable: a class-keyed record matches at + // CLASS level, so its `ordinary_shape_id` is the first element's shape and + // not a per-element guarantee. Handing it to the shape-keyed clone would + // licence an exact-shape read on a proof that never checked shapes. + let _serialized = test_serialize(); + let arr = built_from_pushes(CLASS_A, 3); + assert_eq!(js_array_ensure_element_shape(arr), CLASS_A as i32); + assert_eq!( + js_array_ensure_element_shape_ordinary(arr), + 0, + "a class-keyed proof must not hand its shape id to the shape-keyed clone" + ); + assert_eq!( + proof(arr).expect("proven").class_id, + CLASS_A, + "and the class-keyed proof itself is unchanged" + ); +} + +#[test] +fn class_zero_admission_does_not_disturb_class_keyed_proofs() { + let _serialized = test_serialize(); + let arr = built_from_pushes(CLASS_A, 4); + let established = proof(arr).expect("proven"); + assert_eq!(established.class_id, CLASS_A); + assert_ne!( + established.ordinary_shape_id, 0, + "a class instance still carries an exact ordinary shape id" + ); + + // A class-B store still clears; a class-A store with a DIFFERENT shape + // still keeps through the class-level fallback (the behaviour #10123 must + // not narrow for a nonzero class). + js_array_set_f64(arr, 1, instance(CLASS_A)); + assert!(proof(arr).is_some(), "a same-class store must still keep"); + js_array_set_f64(arr, 1, instance(CLASS_B)); + assert!(proof(arr).is_none(), "a different-class store must clear"); +} + +// --------------------------------------------------------------------------- +// #10123 — the shape's key -> inline slot query the clone's preheader asks. +// --------------------------------------------------------------------------- + +#[test] +fn the_ordinary_slot_query_answers_the_key_position() { + let _serialized = test_serialize(); + let (_keys, shape_id) = record_shape(&["id", "name", "score"]); + for (index, name) in ["id", "name", "score"].iter().enumerate() { + let key = crate::js_string_from_bytes(name.as_ptr(), name.len() as u32); + assert_eq!( + crate::object::shapes::js_shape_ordinary_inline_slot_for_key( + shape_id, + crate::JSValue::string_ptr(key).bits(), + ), + index as i32, + "slot k must be key position k for a plain birth-stamped shape" + ); + } +} + +#[test] +fn the_ordinary_slot_query_declines_an_absent_key_and_an_unknown_shape() { + let _serialized = test_serialize(); + let (_keys, shape_id) = record_shape(&["id", "name"]); + let missing = crate::js_string_from_bytes(b"nope".as_ptr(), 4); + assert_eq!( + crate::object::shapes::js_shape_ordinary_inline_slot_for_key( + shape_id, + crate::JSValue::string_ptr(missing).bits(), + ), + -1 + ); + assert_eq!( + crate::object::shapes::js_shape_ordinary_inline_slot_for_key(0, { + let key = crate::js_string_from_bytes(b"id".as_ptr(), 2); + crate::JSValue::string_ptr(key).bits() + }), + -1, + "shape id 0 names no descriptor" + ); +} + +#[test] +fn the_ordinary_slot_query_matches_an_sso_immediate_against_a_heap_key() { + // Codegen hands the key over as the whole NaN-boxed pool value, and a + // short property name ("id") reaches the pool as an SSO IMMEDIATE whose + // masked low bits are packed characters, not an address. A pointer-only + // comparison would answer -1 for exactly the key names this optimization + // exists for. + let _serialized = test_serialize(); + let (_keys, shape_id) = record_shape(&["id", "name"]); + let heap = crate::js_string_from_bytes(b"id".as_ptr(), 2); + let sso = unsafe { crate::string::short_ascii_sso_bits(heap) } + .expect("test premise: `id` fits the SSO immediate form"); + assert_ne!( + sso, + crate::JSValue::string_ptr(heap).bits(), + "test premise: the two representations really are different bits" + ); + assert_eq!( + crate::object::shapes::js_shape_ordinary_inline_slot_for_key(shape_id, sso), + 0 + ); +} diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index bab64b561b..5ba4d64f54 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -863,6 +863,87 @@ pub extern "C" fn js_object_shape_id_for_keys(keys: u64, key_count: u32) -> u32 id } +/// #10123: the inline slot a PLAIN ordinary shape assigns to `key`, or `-1`. +/// +/// The element-shape loop clone's shape-keyed arm asks this once per tracked +/// property, in the preheader, and the answer replaces the compile-time packed +/// field index a class-keyed clone bakes in. `key_bits` is the whole NaN-boxed +/// key as codegen loaded it from the string pool — NOT a masked +/// `StringHeader*`, because a short property name ("id") reaches the pool as an +/// SSO immediate whose masked low bits are packed characters, not an address. +/// +/// **"Plain" is what makes slot k == key position k.** The four conjuncts +/// below are that claim, and dropping any one of them turns this into a wrong +/// offset rather than a missed optimization: +/// +/// * `object_kind == Ordinary` — a class shape's slots are the class's +/// layout, which this function knows nothing about; +/// * `semantic_generation == 0` — a descriptor/prototype mutation minted this +/// layout, so the keys array no longer describes the live slots; +/// * `hole_count == 0` — an O(1) delete tombstones a key IN PLACE, so a later +/// key's position in the keys array is no longer its slot; +/// * `live_inline_slot_count == logical_key_count` — every key is inline; a +/// shape with spilled keys would put later ones outside the inline block. +/// +/// Allocation-free and side-effect-free: it reads the descriptor, walks the +/// keys array, and returns. The walk is bounded by the physically present key +/// slots (`length.min(capacity)`), which is why a corrupted or forwarded keys +/// array costs a short scan and a `-1` rather than a spin. +#[no_mangle] +pub extern "C" fn js_shape_ordinary_inline_slot_for_key(shape_id: u32, key_bits: u64) -> i32 { + let Some(descriptor) = shape_descriptor_by_id(shape_id) else { + return -1; + }; + if descriptor.object_kind != ShapeObjectKind::Ordinary + || descriptor.semantic_generation != 0 + || descriptor.hole_count != 0 + || descriptor.live_inline_slot_count != descriptor.logical_key_count + { + return -1; + } + let mut wanted_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let mut stored_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + unsafe { + let Some(wanted) = crate::string::js_string_key_bytes( + crate::JSValue::from_bits(key_bits), + &mut wanted_buf, + ) else { + return -1; + }; + let (slots, slot_len) = + super::keys_array_dense_slots(descriptor.keys as usize as *const ArrayHeader); + if slots.is_null() { + return -1; + } + let bound = slot_len.min(descriptor.logical_key_count as usize); + for index in 0..bound { + let stored = crate::JSValue::from_bits((*slots.add(index)).to_bits()); + // Identical bits is the overwhelmingly common answer for a pooled + // key against a canonical keys array (both interned), and it is + // correct for either representation — an SSO immediate and a heap + // pointer each compare equal to themselves. The byte compare below + // is what makes a MIXED pair (pool immediate vs heap key, or two + // separately allocated heap keys) still match. + if stored.bits() == key_bits { + return index as i32; + } + if crate::string::js_string_key_bytes(stored, &mut stored_buf) == Some(wanted) { + return index as i32; + } + } + } + -1 +} + +/// Keepalive anchor — `js_shape_ordinary_inline_slot_for_key` is a +/// generated-code-only callee (the element-shape loop clone's shape-keyed +/// preheader), so the auto-optimize whole-program build would otherwise +/// dead-strip it (see the FFI-symbol-link-break class). +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_SHAPE_ORDINARY_INLINE_SLOT_FOR_KEY: extern "C" fn(u32, u64) -> i32 = + js_shape_ordinary_inline_slot_for_key; + /// Mint a process-global ShapeId for a codegen-registered typed layout and /// install its structural descriptor in the current agent. Unlike /// [`shape_id_for_keys_ensure`], this deliberately does not canonicalise by diff --git a/crates/perry-runtime/src/object/shapes_tests.rs b/crates/perry-runtime/src/object/shapes_tests.rs index dd06428773..4325b432f9 100644 --- a/crates/perry-runtime/src/object/shapes_tests.rs +++ b/crates/perry-runtime/src/object/shapes_tests.rs @@ -1132,3 +1132,53 @@ fn fresh_shape_creation_does_not_flush_the_lookup_cache() { ); } } + +// --------------------------------------------------------------------------- +// #10123: `js_shape_ordinary_inline_slot_for_key` — the element-shape loop +// clone's "which inline slot holds this key?" preheader query. +// +// The positive cases and the SSO-vs-heap representation case live with the +// invariant they serve (`array/element_shape_tests.rs`). What belongs HERE is +// the conjunct that is a property of the shape TABLE: a shape whose kind is +// `Class` describes a class layout, not "slot k == key position k", and +// answering a slot for one would hand the clone a wrong offset rather than a +// missed optimization. +// --------------------------------------------------------------------------- + +#[test] +fn the_ordinary_slot_query_declines_a_class_kind_shape() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let keys = crate::array::js_array_alloc_with_length(1); + let name = crate::string::js_string_from_bytes(b"id".as_ptr(), 2); + crate::array::js_array_set(keys, 0, crate::JSValue::string_ptr(name)); + let key_bits = crate::JSValue::string_ptr(name).bits(); + let ordinary = shape_id_for_keys_ensure(keys, 1); + assert_eq!( + js_shape_ordinary_inline_slot_for_key(ordinary, key_bits), + 0, + "test premise: the ORDINARY shape answers slot 0 for its only key — \ + otherwise the negative below is vacuous" + ); + + // `transition_object_shape_to_class` keeps the keys array and both + // counts and changes ONLY the kind, so the pair below differs in + // exactly the conjunct under test. + let obj = crate::object::js_object_alloc_class_inline_keys_stamped(0, 0, 1, keys, ordinary); + assert_eq!((*obj).parent_class_id, ordinary, "test premise: stamped"); + let class_kind = transition_object_shape_to_class(obj); + assert_ne!( + ordinary, class_kind, + "test premise: the kind really changed" + ); + assert_eq!( + shape_object_kind_by_id(class_kind), + Some(ShapeObjectKind::Class) + ); + assert_eq!( + js_shape_ordinary_inline_slot_for_key(class_kind, key_bits), + -1, + "a class-kind shape names a class layout, not key positions" + ); + } +} From a02e11ff7bfe96c73385ad140942ef99a912d7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 08:48:17 +0200 Subject: [PATCH 2/7] feat(codegen): key the element-shape loop clone on a runtime shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `for (let i = 0; i < count; i++) sum += rows[7].id` and `for (let i = 0; i < count; i++) { const index = i % length; sum += rows[index].id; }` over a `JSON.parse`'d record array now run in the #7480 call-free element-shape fast clone. They previously ran the full element-read + field-read diamond pair. The clone keyed every proof on a compile-time class, which is exactly what a parsed record array does not have. Four things kept it dark, each independently sufficient: * the matcher needed a resolvable element class, and `rows: any` has none; * the preheader's `GC_TYPE_ARRAY` brand ran BEFORE the growth-forwarding repair, so the `GC_TYPE_LAZY_ARRAY` header `JSON.parse` returns for a top-level array in [1 KB, 16 MB] was rejected before the repair that would have materialized it; * the residual per-element check required `GC_OBJ_TYPED_LAYOUT_INTACT`, which a parsed record never has — the clone would have been emitted, entered, and then side-exited on the first element of every loop, with every IR-census assertion still passing; * the index had to be exactly the counter, so neither `rows[7]` nor `const d = i % n; rows[d]` was admitted. The second arm proves the same thing about a different identity: the preheader asks `js_array_ensure_element_shape_ordinary` for the exact ordinary ShapeId every element carries, then `js_shape_ordinary_inline_slot_for_key` for each tracked property's inline slot in that shape. Both are loop-invariant, so the read stays one bare offset load. The repair now precedes the brand (the refresh is safe on an unbranded value by construction: it resolves through `clean_arr_ptr`, which returns null for every tracked non-array). The residual mask drops the typed-layout conjunct and the loaded word is tag-tested as a Number instead, side-exiting to the slow clone when it is not one — the same "this slot holds a raw double" claim, established from the value rather than from a layout declaration. `rows[k]` and `const d = i % m; rows[d]` each carry their own preheader bounds obligation (`length > k`, `1 <= m <= length`), so the clone still pays no per-read bounds test. The derived binding is virtual inside the clone: its `Let` emits one `srem i32`, because the generic `%` lowering is a runtime call and a call inside this clone deletes it rather than slowing it (#7690). The matcher admits exactly one index form per loop and the fact lookup re-checks the spelling, so a fact can never serve a read whose obligation was not discharged. A constant-index loop deliberately does not require the counter's canonical i32 slot: `stmt/let_stmt.rs` mints one only for an index-used or i32-bounded local, and the `repeat` shape's counter is neither. The matcher, the fact lookup and the field lowering all ask the same `needs_counter_i32_slot()` question. The revocation argument is unchanged — it never mentioned classes, and call-free remains the whole admission test. `fast_clone_slice` was widened to own the new `element_shape.number` blocks, or every negative assertion against it would have been partly vacuous. --- changelog.d/10123-json-record-loop-clone.md | 63 ++ .../src/expr/element_shape_guard.rs | 397 ++++++-- crates/perry-codegen/src/expr/mod.rs | 128 ++- .../src/expr/property_get/helpers.rs | 33 +- crates/perry-codegen/src/expr/shadow_slot.rs | 13 +- .../perry-codegen/src/runtime_decls/arrays.rs | 4 + .../src/runtime_decls/strings.rs | 4 + .../src/stmt/element_shape_loop.rs | 870 +++++++++++++----- .../src/stmt/element_shape_loop_tests.rs | 448 ++++++++- crates/perry-codegen/src/stmt/let_stmt.rs | 40 + .../src/type_analysis/numeric.rs | 17 +- test-files/test_gap_json_record_loop_clone.ts | 272 ++++++ 12 files changed, 1938 insertions(+), 351 deletions(-) create mode 100644 changelog.d/10123-json-record-loop-clone.md create mode 100644 test-files/test_gap_json_record_loop_clone.ts diff --git a/changelog.d/10123-json-record-loop-clone.md b/changelog.d/10123-json-record-loop-clone.md new file mode 100644 index 0000000000..0b04945888 --- /dev/null +++ b/changelog.d/10123-json-record-loop-clone.md @@ -0,0 +1,63 @@ +Make the element-shape versioned loop clone (#7480 / #5093) fire for +`JSON.parse`'d record arrays. Loops of the shape +`for (let i = 0; i < count; i++) sum += rows[7].id` and +`for (let i = 0; i < count; i++) { const index = i % length; sum += rows[index].id; }` +over an `any`-typed parsed array now run in the call-free fast clone instead of +the generic element-read + field-read diamonds. + +The clone keyed every proof on a compile-time class, which structurally +excluded the case record-processing code is written against: a parsed record is +`class_id == 0` with an ordinary birth ShapeId, and `rows: any` resolves to no +class at all. Four separate things had to change, each of which independently +kept the clone dark: + +* **Runtime.** `array/element_shape.rs` refused `class_id == 0` outright, so no + parsed record array could ever carry an element-shape proof. Class 0 is now + admitted keyed on the exact ordinary ShapeId — strictly narrower than the + class-level identity, and necessarily so, since "same class" is vacuous when + the class is 0. New FFI: `js_array_ensure_element_shape_ordinary` (the + ShapeId for a class-0 proof, 0 for a class-keyed one — the two are + deliberately not interchangeable) and `js_shape_ordinary_inline_slot_for_key` + (the inline slot a plain ordinary shape assigns to a key, or -1). The key + crosses as the whole NaN-box, not a masked pointer, because a short property + name reaches the string pool as an SSO immediate whose masked low bits are + packed characters rather than an address. +* **The brand test ran before the head repair.** `JSON.parse` of a top-level + array in [1 KB, 16 MB] hands back a `GC_TYPE_LAZY_ARRAY` header, which the + preheader's `GC_TYPE_ARRAY` brand rejected — before the repair step that + would have materialized it. The repair now runs first and the brand is + applied to the repaired head. `js_array_refresh_local_head` is safe on an + unbranded value by construction: it resolves through `clean_arr_ptr`, which + returns null for every tracked non-array. +* **The residual per-element check required `GC_OBJ_TYPED_LAYOUT_INTACT`.** A + parsed record never has that bit — `object/json_construction.rs` finishes + every record with `layout_init_pointer_free` or `layout_mark_unknown`, and + both clear it — so the clone would have been emitted, entered, and then + side-exited on the first element of every loop. The shape-keyed residual + drops that conjunct and buys the same claim per read, from the value: the + loaded word is tag-tested as a Number and a string / boolean / null `id` + side-exits to the slow clone. +* **The index had to be the counter.** `rows[7]` and + `const d = i % n; rows[d]` are now admitted, each with its own preheader + bounds obligation (`length > k`, `1 <= m <= length`) so the clone still pays + no per-read bounds test. The derived binding is virtual inside the clone — + its `Let` emits one `srem i32` rather than the generic `%` lowering, which is + a runtime call and would delete the clone rather than slow it. The matcher + admits exactly one index form per loop. + +The revocation argument is unchanged: it never mentioned classes, and every +funnel that retires a class-keyed proof retires a shape-keyed one. Call-free +is still the whole admission test, enforced by the matcher and by the +post-emission scan of every block the clone owns. + +Measured with `benchmarks/json_performance/.work/fixtures/records_array_*.json` +and a `rows: any` access worker, best of five, ns per iteration: + +| cell | before | after | node 26.5.1 | bun 1.3.14 | +|---|---|---|---|---| +| 16k repeat | 5.73 | BEFORE_AFTER | 3.06 | 3.63 | +| 16k sequential | 12.27 | BEFORE_AFTER | 4.39 | 4.29 | +| 1m repeat | 5.74 | BEFORE_AFTER | 3.49 | 4.66 | +| 1m sequential | 15.80 | BEFORE_AFTER | 9.51 | 6.76 | +| 20m repeat | 5.24 | BEFORE_AFTER | 3.59 | 5.17 | +| 20m sequential | 17.65 | BEFORE_AFTER | 7.22 | 8.37 | diff --git a/crates/perry-codegen/src/expr/element_shape_guard.rs b/crates/perry-codegen/src/expr/element_shape_guard.rs index 5db287d909..5d22777dbf 100644 --- a/crates/perry-codegen/src/expr/element_shape_guard.rs +++ b/crates/perry-codegen/src/expr/element_shape_guard.rs @@ -81,6 +81,27 @@ const ELEM_HEADER_MASK: &str = "402686207"; // 0x1800_80FF /// not forwarded, no per-object descriptors, typed layout intact. const ELEM_HEADER_EXPECT: &str = "268435458"; // 0x1000_0002 +/// #10123: [`ELEM_HEADER_MASK`] without the typed-layout conjunct. +/// +/// A `JSON.parse`'d record has no typed layout and never will: +/// `object/json_construction.rs` finishes it with `layout_init_pointer_free` +/// or `layout_mark_unknown`, and BOTH clear `GC_OBJ_TYPED_LAYOUT_INTACT` +/// explicitly. Keeping the bit in the mask would side-exit every element of +/// every parsed record array — the clone would be emitted, entered, and then +/// leave the loop on its first read. +/// +/// What the bit bought the class-keyed arm was "the slot holds a raw +/// `double`". The shape-keyed arm buys that differently and per read: the +/// loaded word is NaN-boxed, so it is tested with the same Number-tag range +/// check the preheader applies to the accumulator (`emit_js_value_is_number`), +/// and a non-Number (a string `id`, a `null`, a boxed INT32) side-exits to the +/// slow clone. That is not weaker — it is the same claim, established from the +/// value instead of from a layout declaration. +const ELEM_HEADER_SHAPE_MASK: &str = "134250751"; // 0x0800_80FF +/// The value [`ELEM_HEADER_SHAPE_MASK`] must produce: `obj_type == +/// GC_TYPE_OBJECT`, not forwarded, no per-object descriptors. +const ELEM_HEADER_SHAPE_EXPECT: &str = "2"; // 0x0000_0002 + /// Where the fast clone's trip count comes from. /// /// The two arms differ in *which* fact the preheader has to prove. A bound the @@ -104,6 +125,61 @@ pub(crate) enum ElementShapeLoopTripCount<'a> { ArrayLength, } +/// #10123: which identity the preheader proves for the elements. +#[derive(Clone, Copy, Debug)] +pub(crate) enum ElementShapeGuardKind<'a> { + /// The original arm: every element's `ObjectHeader::class_id` is + /// `expected_class_id`, and the expected ShapeId is the class's canonical + /// one, loaded from the global beside `keys_global_name`. + Class { + expected_class_id: &'a str, + keys_global_name: &'a str, + }, + /// The shape arm: every element carries the SAME exact ordinary ShapeId, + /// whatever it is. This is what a class-less record array can prove, and + /// the shape id itself becomes the expected one — there is no class, so + /// there is no canonical keys global to load it from. `properties` are the + /// names whose inline slots the preheader resolves against that shape. + Shape { + properties: &'a std::collections::BTreeSet, + }, +} + +/// #10123: the preheader obligation that makes every index the clone reads +/// in-bounds, WITHOUT a per-read test. +/// +/// Each arm is the whole bounds argument for one [`super::ElementShapeIndex`] +/// spelling, discharged once. Getting one wrong is an out-of-bounds read, not +/// a slow path, which is why they are named after the index form rather than +/// folded into a single "check the index" helper. +#[derive(Clone, Copy, Debug)] +pub(crate) enum ElementShapeIndexBound<'a> { + /// `arr[j]`: the counter IS the index, so the trip-count obligation + /// (`length >= bound`, or `bound == length`) already covers every read. + FromTripCount, + /// `arr[k]`: one constant index, in range iff `length > k`. + Constant(i64), + /// `arr[j % m]`: `srem` of a non-negative counter by `m` lands in + /// `[0, m)`, so `m <= length` covers every read. `m` was materialized as a + /// nonzero, non-negative i32 before the guard. + Modulus(&'a str), +} + +/// Everything the fast clone needs from the preheader. +pub(crate) struct ElementShapeGuardOutputs { + /// Elements base pointer, derived after the last call in the preheader. + pub elements_base: String, + /// i32 SSA of the ShapeId every element must still carry per read. + pub expected_shape_id: String, + /// The accumulated `i1` the caller ANDs its own conditions into. + pub shape_ok: String, + /// The clone's i32 trip count. + pub bound_i32: String, + /// Shape-keyed only: property -> i64 SSA of its inline slot index, each + /// proven non-negative before the clone is reachable. + pub field_slots: std::collections::BTreeMap, +} + /// Emit the once-per-loop element-shape guard into the current block chain. /// /// Leaves `ctx.current_block` on an UNTERMINATED block holding the accumulated @@ -113,47 +189,66 @@ pub(crate) enum ElementShapeLoopTripCount<'a> { /// terminates with `cond_br(shape_ok, fast, slow)`. Never entering a clone /// whose call-freeness is unproven is the whole revocation argument. /// -/// Sequencing is load-bearing, in four steps and this order: +/// Sequencing is load-bearing, in five steps and this order: /// /// 1. the receiver is a heap pointer at all; -/// 2. the **brand** test, so the pointer handed to the runtime is known to be -/// a real array and not an `extends Array` instance (#7573/#7603); -/// 3. the **growth-forwarding repair** (#7480) — the binding may hold a stale -/// head, and steps below read `length` and the elements base off the raw -/// pointer, where a forwarding stub is not merely wrong but *plausibly* +/// 2. the **growth-forwarding repair** (#7480) — the binding may hold a stale +/// head, and the steps below read `length` and the elements base off the +/// raw pointer, where a forwarding stub is not merely wrong but *plausibly* /// wrong (see the block comment). It goes before the guard call, not after, /// because the refresh can itself allocate; +/// 3. the **brand** test, so the pointer handed to the runtime is known to be +/// a real array and not an `extends Array` instance (#7573/#7603); /// 4. the runtime guard call when static construction did not already prove -/// the class, and only THEN the elements base — derived from a fresh load of -/// the array's rooted slot, because either preceding helper can allocate -/// and an allocation can move the array, so a base derived before it could -/// be a from-space address. +/// the identity, plus (shape-keyed) one inline-slot query per tracked +/// property; +/// 5. and only THEN the elements base — derived from a fresh load of the +/// array's rooted slot, because any preceding helper can allocate and an +/// allocation can move the array, so a base derived before it could be a +/// from-space address. /// -/// Returns `(elements_base, expected_shape_id, shape_ok, bound_i32)`. +/// **Why the repair now precedes the brand** (#10123; it used to follow it). +/// `JSON.parse` of a top-level array hands back a `GC_TYPE_LAZY_ARRAY` header, +/// which the brand test rejects — so the clone declined the single most common +/// record array in the language before the repair that would have materialized +/// it ever ran. `js_array_refresh_local_head` is safe on an unbranded value by +/// construction: it tests `POINTER_TAG`, then `is_plausible_heap_addr`, then +/// resolves through `clean_arr_ptr`, which returns null for every tracked +/// non-array (an `extends Array` instance included) and materializes a lazy +/// one. Anything it cannot resolve comes back unchanged, and the brand test — +/// now applied to the REPAIRED head — rejects it exactly as before. pub(crate) fn emit_element_shape_loop_preheader_check( ctx: &mut FnCtx, array_local_id: u32, - expected_class_id: &str, - keys_global_name: &str, + kind: ElementShapeGuardKind<'_>, trip_count: ElementShapeLoopTripCount<'_>, + index_bound: ElementShapeIndexBound<'_>, slow_label: &str, statically_proven: bool, -) -> anyhow::Result<(String, String, String, String)> { - let brand_idx = ctx.new_block("element_shape.loop.preheader.brand"); +) -> anyhow::Result { let repair_idx = ctx.new_block("element_shape.loop.preheader.repair"); + let brand_idx = ctx.new_block("element_shape.loop.preheader.brand"); let query_idx = (!statically_proven).then(|| ctx.new_block("element_shape.loop.preheader.query")); + let slots_idx = match kind { + ElementShapeGuardKind::Shape { properties } if !properties.is_empty() => { + Some(ctx.new_block("element_shape.loop.preheader.slots")) + } + _ => None, + }; let deref_idx = ctx.new_block("element_shape.loop.preheader.deref"); - let brand_label = ctx.block_label(brand_idx); let repair_label = ctx.block_label(repair_idx); + let brand_label = ctx.block_label(brand_idx); let query_label = query_idx.map(|idx| ctx.block_label(idx)); + let slots_label = slots_idx.map(|idx| ctx.block_label(idx)); let deref_label = ctx.block_label(deref_idx); - let post_repair_label = query_label.as_deref().unwrap_or(&deref_label); + let post_query_label = slots_label.as_deref().unwrap_or(&deref_label); + let post_brand_label = query_label.as_deref().unwrap_or(post_query_label); // (1) Receiver is a heap pointer at all. A basic block has no // short-circuit, so nothing may be dereferenced until this branch is taken. let arr0 = super::lower_expr(ctx, &perry_hir::Expr::LocalGet(array_local_id))?; - let handle0 = { + { let blk = ctx.block(); let bits0 = blk.bitcast_double_to_i64(&arr0); let tag0 = blk.lshr(I64, &bits0, "48"); @@ -161,28 +256,10 @@ pub(crate) fn emit_element_shape_loop_preheader_check( let handle0 = blk.and(I64, &bits0, crate::nanbox::POINTER_MASK_I64); let above0 = blk.icmp_ugt(I64, &handle0, HANDLE_BAND_TOP); let ok0 = blk.and(I1, &is_ptr0, &above0); - blk.cond_br(&ok0, &brand_label, slow_label); - handle0 - }; - - // (2) SUBCLASS BRAND (#7573/#7603). `class X extends Array` instances are - // plain `ObjectHeader`s that overlay `ArrayHeader` field for field, so - // `length`/`capacity`/`elements[0]` would read `class_id`/`parent_class_id` - // (the ShapeId)/`keys_array` (#8113). The runtime's `array_gc_header` makes the - // same test, but it is repeated here so the raw pointer handed across the - // call below is already branded, and so the emitted IR carries the brand - // where a reviewer (and the IR census) can see it. - ctx.current_block = brand_idx; - { - let blk = ctx.block(); - let gt_addr = blk.sub(I64, &handle0, "8"); - let gt_ptr = blk.inttoptr(I64, >_addr); - let gc_type = blk.load(I8, >_ptr); - let is_array = blk.icmp_eq(I8, &gc_type, GC_TYPE_ARRAY); - blk.cond_br(&is_array, &repair_label, slow_label); + blk.cond_br(&ok0, &repair_label, slow_label); } - // (2b) GROWTH-FORWARDING REPAIR (#7480). The binding may hold a *stale* + // (2) GROWTH-FORWARDING REPAIR (#7480). The binding may hold a *stale* // array head: `js_array_grow` allocates the larger array elsewhere and // leaves a forwarding stub at the old address, and only the bindings the // growing code itself wrote through are re-pointed. Every runtime entry @@ -204,7 +281,7 @@ pub(crate) fn emit_element_shape_loop_preheader_check( // BEFORE the query call, not after, because `js_array_refresh_local_head` // can allocate (a lazy array materializes inside `clean_arr_ptr`) — putting // it here keeps the "no call after the base is derived" invariant intact, - // and the write-back means step (4)'s re-load of the rooted slot picks up + // and the write-back means step (5)'s re-load of the rooted slot picks up // the repaired head no matter what the query call moved. ctx.current_block = repair_idx; { @@ -233,35 +310,118 @@ pub(crate) fn emit_element_shape_loop_preheader_check( let handler = blk.and(I64, &bitsr, crate::nanbox::POINTER_MASK_I64); let abover = blk.icmp_ugt(I64, &handler, HANDLE_BAND_TOP); let okr = blk.and(I1, &is_ptrr, &abover); - blk.cond_br(&okr, post_repair_label, slow_label); + blk.cond_br(&okr, &brand_label, slow_label); + } + + // (3) SUBCLASS BRAND (#7573/#7603). `class X extends Array` instances are + // plain `ObjectHeader`s that overlay `ArrayHeader` field for field, so + // `length`/`capacity`/`elements[0]` would read `class_id`/`parent_class_id` + // (the ShapeId)/`keys_array` (#8113). The runtime's `array_gc_header` makes the + // same test, but it is repeated here so the raw pointer handed across the + // call below is already branded, and so the emitted IR carries the brand + // where a reviewer (and the IR census) can see it. + // + // It reads the REPAIRED head: a lazy-array header is not `GC_TYPE_ARRAY` + // and would fail here, which is the whole reason step (2) now runs first. + ctx.current_block = brand_idx; + { + let arrb = super::lower_expr(ctx, &perry_hir::Expr::LocalGet(array_local_id))?; + let blk = ctx.block(); + let bitsb = blk.bitcast_double_to_i64(&arrb); + let handleb = blk.and(I64, &bitsb, crate::nanbox::POINTER_MASK_I64); + let gt_addr = blk.sub(I64, &handleb, "8"); + let gt_ptr = blk.inttoptr(I64, >_addr); + let gc_type = blk.load(I8, >_ptr); + let is_array = blk.icmp_eq(I8, &gc_type, GC_TYPE_ARRAY); + blk.cond_br(&is_array, post_brand_label, slow_label); } - // (3) The live-header query for arrays whose construction is not statically - // contained. `js_array_ensure_element_shape` establishes the invariant by - // scan on first visit and confirms it in O(1) afterwards; either way it - // reads the array's CURRENT `GcHeader` bit and its record, and self-heals - // (clearing the bit) when the record went stale. Type declarations are - // never sufficient — #7501's lesson. The static arm is stronger: E1--E5 - // proves every dense slot is a fresh exact-class allocation for the whole - // native region. + // (4) The live-header query for arrays whose construction is not statically + // contained. `js_array_ensure_element_shape[_ordinary]` establishes the + // invariant by scan on first visit and confirms it in O(1) afterwards; + // either way it reads the array's CURRENT `GcHeader` bit and its record, + // and self-heals (clearing the bit) when the record went stale. Type + // declarations are never sufficient — #7501's lesson. The static arm is + // stronger: E1--E5 proves every dense slot is a fresh exact-class + // allocation for the whole native region. // // Deliberately re-loads the (now repaired) binding rather than reusing the // repair block's handle: `js_array_refresh_local_head` can allocate, so a // handle derived before it is a pre-move address. + let mut queried_shape_id: Option = None; if let Some(query_idx) = query_idx { ctx.current_block = query_idx; let arrq = super::lower_expr(ctx, &perry_hir::Expr::LocalGet(array_local_id))?; - { + let blk = ctx.block(); + let bitsq = blk.bitcast_double_to_i64(&arrq); + let handleq = blk.and(I64, &bitsq, crate::nanbox::POINTER_MASK_I64); + match kind { + ElementShapeGuardKind::Class { + expected_class_id, .. + } => { + let class_id = blk.call(I32, "js_array_ensure_element_shape", &[(I64, &handleq)]); + let cid_ok = blk.icmp_eq(I32, &class_id, expected_class_id); + blk.cond_br(&cid_ok, post_query_label, slow_label); + } + // #10123: there is no compile-time id to compare against — the + // answer IS the expected ShapeId. Zero means "no class-0 proof", + // which covers both "not homogeneous" and "homogeneous but + // class-keyed", and both take the slow clone. + ElementShapeGuardKind::Shape { .. } => { + let shape_id = blk.call( + I32, + "js_array_ensure_element_shape_ordinary", + &[(I64, &handleq)], + ); + let ok = blk.icmp_ne(I32, &shape_id, "0"); + blk.cond_br(&ok, post_query_label, slow_label); + queried_shape_id = Some(shape_id); + } + } + } + + // (4b) #10123: resolve each tracked property to an inline slot ONCE, + // against the shape the query just proved. A class-keyed clone bakes this + // in at compile time from the class's field list; a record array has no + // class, so the shape table answers instead. A `-1` (absent key, a + // class-kind or mutated shape, a key the shape spilled) declines the clone + // — never a guess at an offset. + let mut field_slots: std::collections::BTreeMap = + std::collections::BTreeMap::new(); + if let (Some(slots_idx), ElementShapeGuardKind::Shape { properties }) = (slots_idx, kind) { + ctx.current_block = slots_idx; + let shape_id = queried_shape_id + .clone() + .expect("the shape-keyed arm always emits the runtime query"); + let mut all_ok: Option = None; + for property in properties { + let key_idx = ctx.strings.intern(property); + let key_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let blk = ctx.block(); - let bitsq = blk.bitcast_double_to_i64(&arrq); - let handleq = blk.and(I64, &bitsq, crate::nanbox::POINTER_MASK_I64); - let class_id = blk.call(I32, "js_array_ensure_element_shape", &[(I64, &handleq)]); - let cid_ok = blk.icmp_eq(I32, &class_id, expected_class_id); - blk.cond_br(&cid_ok, &deref_label, slow_label); + // The WHOLE NaN-box, not a masked pointer: a short property name + // ("id") reaches the pool as an SSO immediate whose masked low bits + // are packed characters rather than an address. The runtime + // compares by content across both representations. + let key_box = blk.load(DOUBLE, &key_global); + let key_bits = blk.bitcast_double_to_i64(&key_box); + let slot = blk.call( + I32, + "js_shape_ordinary_inline_slot_for_key", + &[(I32, &shape_id), (I64, &key_bits)], + ); + let ok = blk.icmp_sgt(I32, &slot, "-1"); + all_ok = Some(match all_ok { + Some(acc) => blk.and(I1, &acc, &ok), + None => ok, + }); + let slot64 = blk.sext(I32, &slot, I64); + field_slots.insert(property.clone(), slot64); } + let all_ok = all_ok.expect("a non-empty property set emits at least one query"); + ctx.block().cond_br(&all_ok, &deref_label, slow_label); } - // (4) Post-proof re-derivation. Everything from here to the end of the fast + // (5) Post-proof re-derivation. Everything from here to the end of the fast // clone is call-free, so THIS pointer is the pointer the clone uses. ctx.current_block = deref_idx; let arr1 = super::lower_expr(ctx, &perry_hir::Expr::LocalGet(array_local_id))?; @@ -306,6 +466,17 @@ pub(crate) fn emit_element_shape_loop_preheader_check( } }; + // #10123: the index obligation. `FromTripCount` adds nothing — the trip + // count already covers every read — and the other two are compared + // UNSIGNED for the same reason `Bound` is: `length` is a `u32` read into + // an i32, so a hypothetical 2^31-element array must not read as negative + // and pass. + let index_ok = match index_bound { + ElementShapeIndexBound::FromTripCount => None, + ElementShapeIndexBound::Constant(k) => Some(blk.icmp_ugt(I32, &length, &k.to_string())), + ElementShapeIndexBound::Modulus(modulus) => Some(blk.icmp_uge(I32, &length, modulus)), + }; + // Logical elements base, including a consumed queue prefix. let base_addr = blk.array_elements_addr(&handle1); let elements_base = blk.inttoptr(I64, &base_addr); @@ -314,40 +485,63 @@ pub(crate) fn emit_element_shape_loop_preheader_check( // load is hoistable here for the same reason the class-field preheader // check hoists it: flipping it requires a runtime call, and the fast clone // makes none. - let shape_global = crate::typed_shape::shape_id_global_name_from_keys_global(keys_global_name); - let expected_shape_id = blk.load(I32, &format!("@{shape_global}")); + let expected_shape_id = match kind { + ElementShapeGuardKind::Class { + keys_global_name, .. + } => { + let shape_global = + crate::typed_shape::shape_id_global_name_from_keys_global(keys_global_name); + blk.load(I32, &format!("@{shape_global}")) + } + // The query's answer, which dominates this block. + ElementShapeGuardKind::Shape { .. } => queried_shape_id + .clone() + .expect("the shape-keyed arm always emits the runtime query"), + }; let gate = blk.load_volatile(I8, "@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED"); let gate_ok = blk.icmp_eq(I8, &gate, "0"); let mut acc = blk.and(I1, &is_ptr1, &above1); acc = blk.and(I1, &acc, &is_array1); acc = blk.and(I1, &acc, &len_ok); + if let Some(index_ok) = index_ok { + acc = blk.and(I1, &acc, &index_ok); + } acc = blk.and(I1, &acc, &gate_ok); // No terminator: the caller branches after proving the clone call-free. - Ok((elements_base, expected_shape_id, acc, bound_i32)) + Ok(ElementShapeGuardOutputs { + elements_base, + expected_shape_id, + shape_ok: acc, + bound_i32, + field_slots, + }) } /// Emit one `arr[i].field` read inside the fast clone: bare element load, /// an optional residual per-element check, then a bare raw-f64 slot load. /// -/// `idx_i32` must be the loop counter's canonical i32 (non-negative, and -/// `< bound <= length` by the preheader), and `fact` must be the fact the -/// preheader installed for `(array, counter)`. +/// `idx_i32` must be an index the preheader discharged a bounds obligation for +/// (see [`ElementShapeIndexBound`]), and `fact` must be the fact the preheader +/// installed for this array. pub(crate) fn emit_element_shape_field_load( ctx: &mut FnCtx, fact: &super::ElementShapeLoopFact, idx_i32: &str, - field_index: u32, + field_slot: &super::ElementShapeFieldSlot, ) -> String { - let field_index_str = field_index.to_string(); let header_skip = crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string(); + let (slot_ty, slot_value) = match field_slot { + super::ElementShapeFieldSlot::Packed(index) => (I64, index.to_string()), + super::ElementShapeFieldSlot::Runtime(reg) => (I64, reg.clone()), + }; let elem_ptr = { let blk = ctx.block(); // The element-shape invariant proved every slot in the verified prefix - // is a POINTER_TAG object of the guarded class, so the unbox needs no - // tag test and no handle-band test — the two checks that make up the + // is a POINTER_TAG object of the guarded identity, so the unbox needs + // no tag test and no handle-band test — the two checks that make up the // element-read tier. let idx64 = blk.sext(I32, idx_i32, I64); let slot_ptr = blk.gep(I64, &fact.elements_base, &[(I64, &idx64)]); @@ -364,8 +558,13 @@ pub(crate) fn emit_element_shape_field_load( // cannot come from the runtime array-level invariant). let hdr_ptr = blk.gep(I8, &elem_ptr, &[(I64, "-8")]); let hdr = blk.load(I32, &hdr_ptr); - let hdr_masked = blk.and(I32, &hdr, ELEM_HEADER_MASK); - let hdr_ok = blk.icmp_eq(I32, &hdr_masked, ELEM_HEADER_EXPECT); + let (mask, expect) = if fact.shape_keyed { + (ELEM_HEADER_SHAPE_MASK, ELEM_HEADER_SHAPE_EXPECT) + } else { + (ELEM_HEADER_MASK, ELEM_HEADER_EXPECT) + }; + let hdr_masked = blk.and(I32, &hdr, mask); + let hdr_ok = blk.icmp_eq(I32, &hdr_masked, expect); // #8113: the ShapeId moved from header offset 8 to 4. let sid_ptr = blk.gep(I8, &elem_ptr, &[(I64, "4")]); @@ -383,8 +582,28 @@ pub(crate) fn emit_element_shape_field_load( let blk = ctx.block(); let fields_base = blk.gep(I8, &elem_ptr, &[(I64, &header_skip)]); - let field_ptr = blk.gep(DOUBLE, &fields_base, &[(I64, &field_index_str)]); - blk.load(DOUBLE, &field_ptr) + let field_ptr = blk.gep(DOUBLE, &fields_base, &[(slot_ty, &slot_value)]); + let value = blk.load(DOUBLE, &field_ptr); + + // #10123: the shape-keyed arm's representation check. + // + // A class-keyed clone reads a RAW double, licensed by + // `GC_OBJ_TYPED_LAYOUT_INTACT` in the residual mask above. A record's slot + // holds a NaN-boxed JSValue instead, and the two coincide exactly when the + // value is a Number — so the same tag-range test the preheader applies to + // the accumulator is applied here, per read, and a string / boolean / null + // `id` side-exits to the slow clone rather than being consumed as a raw + // double. Without it `sum += rows[i].id` over `{"id": "7"}` would add the + // bit pattern of a string pointer. + if fact.shape_keyed { + let is_number = crate::stmt::emit_js_value_is_number(ctx, &value); + let ok_idx = ctx.new_block("element_shape.number"); + let ok_label = ctx.block_label(ok_idx); + ctx.block() + .cond_br(&is_number, &ok_label, &fact.side_exit_label); + ctx.current_block = ok_idx; + } + value } #[cfg(test)] @@ -417,6 +636,46 @@ mod tests { "header expectation drifted" ); + // #10123's shape-keyed pair is the SAME three header facts with the + // typed-layout conjunct removed, derived here rather than restated so + // a drift in any shared constant moves both. + let shape_mask = obj_type_mask | forwarded | has_descriptors; + let shape_expect = u32::from(2u8) /* GC_TYPE_OBJECT */; + assert_eq!( + ELEM_HEADER_SHAPE_MASK, + shape_mask.to_string(), + "shape-keyed header mask drifted" + ); + assert_eq!( + ELEM_HEADER_SHAPE_EXPECT, + shape_expect.to_string(), + "shape-keyed header expectation drifted" + ); + // It must still reject the three facts it DOES cover, and must + // deliberately NOT depend on the typed-layout bit — a parsed record + // never has it, so a mask that kept it would side-exit every element. + assert_eq!(shape_expect & shape_mask, shape_expect); + assert_eq!( + (shape_expect | typed_intact) & shape_mask, + shape_expect, + "the shape-keyed mask must ignore the typed-layout bit" + ); + assert_ne!( + (shape_expect | forwarded) & shape_mask, + shape_expect, + "forwarded not rejected" + ); + assert_ne!( + (shape_expect | has_descriptors) & shape_mask, + shape_expect, + "descriptors not rejected" + ); + assert_ne!( + (shape_expect ^ 1) & shape_mask, + shape_expect, + "wrong obj_type not rejected" + ); + // Sabotage direction: the mask must actually reject each fact. let good = expect; assert_eq!(good & mask, expect); diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index bcc1eb6f32..aa6f68fb4e 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2096,6 +2096,70 @@ pub(crate) struct ClassFieldLoopFact { pub fields: std::collections::BTreeMap, } +/// #10123: where the fast clone's element index comes from. +/// +/// The class-keyed arm admits [`Self::Counter`] only — the preheader's +/// `length >= bound` check is then exactly "the verified prefix covers every +/// index the loop reads". The other two arms index somewhere the trip count +/// says nothing about, so each carries its OWN preheader obligation (see +/// `expr::element_shape_guard::ElementShapeIndexBound`) and the fast clone +/// still pays no per-read bounds test. +#[derive(Debug, Clone)] +pub(crate) enum ElementShapeIndex { + /// `arr[j]` — the counter, read from its canonical i32 slot. + Counter, + /// `arr[7]` — a compile-time constant in `0..=i32::MAX`. The preheader + /// proved `length > k`. + Constant(i64), + /// `const d = j % m; … arr[d]` — the shape every sequential-access loop + /// over a parsed record array is written in. + /// + /// `d` is VIRTUAL in the clone exactly like #7771's element binding: its + /// `Let` emits an `srem i32` into `slot` (`stmt/let_stmt.rs`) instead of + /// the generic `%` lowering, which would be a runtime call and would + /// therefore DELETE the clone rather than slow it. The preheader proved + /// `1 <= m <= length`, and the counter is non-negative, so + /// `srem(counter, m)` is in `[0, length)` with no per-read test. + DerivedMod { + /// The `const` local the body binds; nothing else may read it. + local_id: u32, + /// i32 SSA value of the modulus, materialized in the preheader. + modulus_i32: String, + /// Entry-block i32 alloca the clone writes the derived index to. + slot: String, + }, +} + +impl ElementShapeIndex { + /// Does computing this index read the counter's canonical i32 slot? + /// + /// `Constant` is a literal, so the clone never touches the counter at all + /// except for the trip test — which + /// `lower_for_after_init_with_i32_bound` already falls back to a double + /// compare for. This is why `for (i = 0; i < n; i++) sum += rows[7].id` + /// gets a clone: its counter is neither index-used nor i32-bounded, so + /// `stmt/let_stmt.rs` mints no i32 slot for it, and demanding one would + /// have declined the benchmark's own `repeat` shape. + /// + /// The matcher, the fact lookup and the field lowering must all ask this + /// same question: a lookup that answered `Some` for a read the lowering + /// then declined would hand `is_numeric_expr` a raw-double promise the + /// generic path does not keep. + pub(crate) fn needs_counter_i32_slot(&self) -> bool { + !matches!(self, ElementShapeIndex::Constant(_)) + } +} + +/// #10123: where one tracked property's value sits inside an element. +#[derive(Debug, Clone)] +pub(crate) enum ElementShapeFieldSlot { + /// Class-keyed: the compile-time packed field index. + Packed(u32), + /// Shape-keyed: an i64 SSA value produced once in the preheader by + /// `js_shape_ordinary_inline_slot_for_key`, proven non-negative there. + Runtime(String), +} + /// #5093 / repsel #7480: one fact per (array, counter, versioned loop) — the /// element-shape clone's licence to read `arr[i].field` with no guard. /// @@ -2111,12 +2175,32 @@ pub(crate) struct ClassFieldLoopFact { /// /// and the lowering proved the fast clone is call-free, so nothing can revoke /// the invariant or move the array while the clone runs. +/// +/// #10123 added a second, SHAPE-keyed arm. It proves the same thing about a +/// different identity: the preheader asks +/// `js_array_ensure_element_shape_ordinary` for the exact ordinary ShapeId +/// every element carries, which is what a `JSON.parse`'d record array (class +/// 0, an ordinary birth shape) can prove and a class id is not. The three +/// facts the clone then reads per element — the slot index, the expected +/// ShapeId, and that the loaded word really is a Number — all come from that +/// arm's preheader instead of from a compile-time class. #[derive(Debug, Clone)] pub(crate) struct ElementShapeLoopFact { /// LocalId of the loop-invariant array the preheader guarded. pub array_local_id: u32, - /// LocalId of the loop counter used as the element index. + /// LocalId of the loop counter. It is the element index only when + /// [`Self::index`] is [`ElementShapeIndex::Counter`]; it is always the + /// local whose canonical i32 slot the clone reads. pub index_local_id: u32, + /// #10123: how the clone computes the element index. + pub index: ElementShapeIndex, + /// #10123: true when the preheader proved an exact ordinary ShapeId rather + /// than a class id. The per-element residual check then drops the + /// typed-layout conjunct (a parsed record's layout is + /// `GC_LAYOUT_UNKNOWN`/pointer-free, never a typed layout) and adds a + /// Number-tag test on the loaded word, because a shape proves which slot + /// holds `field` but not what representation the value in it has. + pub shape_keyed: bool, pub scope_id: u32, /// Class the preheader proved every element in the verified prefix has. pub class_name: String, @@ -2134,9 +2218,11 @@ pub(crate) struct ElementShapeLoopFact { /// the exact ShapeId, descriptor-free state, and raw-f64 layout for every /// field this clone reads. When true, no per-object residual is needed. pub statically_layout_proven: bool, - /// property name -> packed slot index, every entry a declared raw-f64 - /// candidate validated by the matcher. - pub fields: std::collections::BTreeMap, + /// property name -> the slot the clone reads it from. Class-keyed entries + /// are compile-time packed indices validated by the matcher as declared + /// raw-f64 candidates; shape-keyed ones are the preheader's live query + /// results (#10123). + pub fields: std::collections::BTreeMap, /// #7771: the body's `const r = arr[counter]` binding, when the matcher /// admitted the element-binding form. Inside the fast clone the `Let` /// itself emits nothing (`stmt/let_stmt.rs`) and every `r.field` read @@ -2183,27 +2269,41 @@ pub(crate) fn element_shape_loop_fact_for_property_get<'f>( ctx: &'f FnCtx<'_>, object: &perry_hir::Expr, property: &str, -) -> Option<(&'f ElementShapeLoopFact, u32)> { +) -> Option<(&'f ElementShapeLoopFact, &'f ElementShapeFieldSlot)> { use perry_hir::Expr; if ctx.element_shape_loop_facts.is_empty() { return None; } match object { Expr::IndexGet { object, index } => { - let (Expr::LocalGet(array_local_id), Expr::LocalGet(index_local_id)) = - (object.as_ref(), index.as_ref()) - else { + let Expr::LocalGet(array_local_id) = object.as_ref() else { return None; }; - if !ctx.i32_counter_slots.contains_key(index_local_id) { - return None; - } ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { - if fact.array_local_id != *array_local_id || fact.index_local_id != *index_local_id + if fact.array_local_id != *array_local_id + || (fact.index.needs_counter_i32_slot() + && !ctx.i32_counter_slots.contains_key(&fact.index_local_id)) { return None; } - fact.fields.get(property).map(|idx| (fact, *idx)) + // #10123: the index SPELLING must be the one the fact's + // preheader discharged a bounds obligation for. A fact built + // for `arr[7]` says nothing about `arr[j]` in the same body, + // and the matcher's one-index-form rule means a body mixing + // them was never admitted — this is what keeps that true at + // the read. + let spelled = match (&fact.index, index.as_ref()) { + (ElementShapeIndex::Counter, Expr::LocalGet(id)) => *id == fact.index_local_id, + (ElementShapeIndex::Constant(k), Expr::Integer(n)) => *n == *k, + (ElementShapeIndex::DerivedMod { local_id, .. }, Expr::LocalGet(id)) => { + *id == *local_id + } + _ => false, + }; + if !spelled { + return None; + } + fact.fields.get(property).map(|slot| (fact, slot)) }) } // #7771: `r.field` through the clone's element binding. The matcher @@ -2217,7 +2317,7 @@ pub(crate) fn element_shape_loop_fact_for_property_get<'f>( { return None; } - fact.fields.get(property).map(|idx| (fact, *idx)) + fact.fields.get(property).map(|slot| (fact, slot)) }), _ => None, } diff --git a/crates/perry-codegen/src/expr/property_get/helpers.rs b/crates/perry-codegen/src/expr/property_get/helpers.rs index 3e3cb82e23..51a8e8dbe6 100644 --- a/crates/perry-codegen/src/expr/property_get/helpers.rs +++ b/crates/perry-codegen/src/expr/property_get/helpers.rs @@ -359,24 +359,43 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context( // candidate at the packed slot index carried here. So the lowering needs // nothing from the receiver's static type, and asking for it would have // made the whole clone dead IR. - if let Some((fact, field_index)) = + if let Some((fact, field_slot)) = crate::expr::element_shape_loop_fact_for_property_get(ctx, object, property) - .map(|(fact, idx)| (fact.clone(), idx)) + .map(|(fact, slot)| (fact.clone(), slot.clone())) { // Both receiver spellings — `arr[j].field` and #7771's `r.field` // through the clone's element binding — resolve to the fact's own // array; the report below must not re-derive it from the expression // shape, which the binding form does not carry. let arr_id = fact.array_local_id; - // The counter's canonical i32 slot is what the matcher required; - // without it there is nothing to index with. - if let Some(slot) = ctx.i32_counter_slots.get(&fact.index_local_id).cloned() { - let idx_i32 = ctx.block().load(I32, &slot); + // The counter's canonical i32 slot is what the matcher required for + // every index form that reads the counter; without it there is nothing + // to index with. A constant index reads no counter and needs none. + let counter_slot = ctx.i32_counter_slots.get(&fact.index_local_id).cloned(); + if counter_slot.is_some() || !fact.index.needs_counter_i32_slot() { + // #10123: the index the preheader discharged a bounds obligation + // for. All three forms are i32 and in `[0, length)` by the time + // they reach the GEP, which is why the fast clone pays no per-read + // bounds test in any of them. + let idx_i32 = match &fact.index { + crate::expr::ElementShapeIndex::Counter => { + let slot = counter_slot.expect("checked above"); + ctx.block().load(I32, &slot) + } + crate::expr::ElementShapeIndex::Constant(k) => k.to_string(), + // The derived `const d = j % m` binding's own slot, written by + // the `Let` arm in `stmt/let_stmt.rs` earlier in this same + // iteration. + crate::expr::ElementShapeIndex::DerivedMod { slot, .. } => { + let slot = slot.clone(); + ctx.block().load(I32, &slot) + } + }; let value = crate::expr::element_shape_guard::emit_element_shape_field_load( ctx, &fact, &idx_i32, - field_index, + &field_slot, ); let lowered = LoweredValue { semantic: SemanticKind::JsNumber, diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index 0aab0163ed..4f1ff8e459 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -172,10 +172,19 @@ pub(crate) fn emit_shadow_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) { // a moving collection rewrites like any root, and every later user of a // shared slot index binds before use. The slow clone, lowered after the // fact is popped, keeps its clear. + // + // #10123's derived index (`const d = j % m`) is the same case for the same + // reason: its `Let` emits one `srem` into a private i32 alloca, never a + // shadow bind, so a lexical-death clear would be the clone's only call. if ctx.element_shape_loop_facts.iter().any(|fact| { + let virtual_binding = match &fact.index { + crate::expr::ElementShapeIndex::DerivedMod { local_id, .. } => Some(*local_id), + _ => None, + }; fact.element_binding - .and_then(|id| ctx.shadow_slot_map.get(&id)) - == Some(&slot_idx) + .into_iter() + .chain(virtual_binding) + .any(|id| ctx.shadow_slot_map.get(&id) == Some(&slot_idx)) }) { return; } diff --git a/crates/perry-codegen/src/runtime_decls/arrays.rs b/crates/perry-codegen/src/runtime_decls/arrays.rs index f8b78d9708..72e9122781 100644 --- a/crates/perry-codegen/src/runtime_decls/arrays.rs +++ b/crates/perry-codegen/src/runtime_decls/arrays.rs @@ -60,6 +60,10 @@ pub fn declare_phase_b_arrays(module: &mut LlModule) { // invariant and returns the proven class id (0 = no proof). O(n) on the // first visit, O(1) after — see `array/element_shape.rs`. module.declare_function("js_array_ensure_element_shape", I32, &[I64]); + // #10123: the shape-keyed sibling — establish-or-confirm for a class-0 + // (plain-object) element array, returning the exact ordinary ShapeId every + // element carries, or 0. + module.declare_function("js_array_ensure_element_shape_ordinary", I32, &[I64]); module.declare_function("js_array_get_index_or_string", DOUBLE, &[I64, DOUBLE]); module.declare_function("js_array_numeric_get_f64_unboxed", DOUBLE, &[I64, I32]); module.declare_function("js_array_set_f64", VOID, &[I64, I32, DOUBLE]); diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 4a945389d5..0da4b188c1 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1153,6 +1153,10 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { ); module.declare_function("js_build_class_keys_array", I64, &[I32, I32, PTR, I32]); module.declare_function("js_object_shape_id_for_keys", I32, &[I64, I32]); + // #10123: (shape_id, NaN-boxed key) -> inline slot index, or -1. The + // element-shape loop clone's shape-keyed preheader resolves each tracked + // property once against the shape the runtime just proved. + module.declare_function("js_shape_ordinary_inline_slot_for_key", I32, &[I32, I64]); module.declare_function( "js_gc_typed_shape_id_for_keys", I32, diff --git a/crates/perry-codegen/src/stmt/element_shape_loop.rs b/crates/perry-codegen/src/stmt/element_shape_loop.rs index 95eab00544..3f9f9a8d9b 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop.rs @@ -40,8 +40,10 @@ //! binding whose only uses are tracked `r.field` reads (#7766: the shape //! the `for…of` desugar emits, and the form a parameter array reaches the //! clone through — the binding is virtual in the fast clone: its `Let` -//! emits nothing and the reads lower through the fact). No store of any -//! kind, no call, no closure, no `await`, no update other than the +//! emits nothing and the reads lower through the fact), or, since #10123, +//! by exactly one `const d = i % m` binding whose only use is as the +//! element index (equally virtual: its `Let` emits one `srem`). No store of +//! any kind, no call, no closure, no `await`, no update other than the //! counter's. //! 2. **By construction (the lowering).** After the fast clone is emitted, //! every one of its blocks is scanned for a GC-unsafe call @@ -88,12 +90,52 @@ //! runtime invalidation deopt needs an on-stack-replacement mechanism Perry //! does not have. //! +//! ## The SHAPE-keyed arm (#10123) +//! +//! Everything above keys on a compile-time CLASS, which excluded the one array +//! shape record-processing code is actually written against: `JSON.parse`'d +//! objects are `class_id == 0` with an ordinary birth ShapeId, and the class +//! resolver has nothing to resolve for a `rows: any` receiver. The clone +//! therefore never fired for a parsed record array — the measured case it +//! exists for. +//! +//! The second arm proves the same thing about a different identity: the +//! preheader asks `js_array_ensure_element_shape_ordinary` for the exact +//! ordinary ShapeId every element carries, then asks +//! `js_shape_ordinary_inline_slot_for_key` where each tracked property sits in +//! THAT shape. Both answers are loop-invariant values, so the clone's read is +//! still one bare offset load. +//! +//! **The revocation argument is unchanged**, because it never mentioned +//! classes: every funnel in the table above retires a shape-keyed proof +//! exactly as it retires a class-keyed one (`note_element_store` compares the +//! record, which for a class-0 proof is the exact ShapeId; a length change +//! fails `verified_len`; prototype surgery bumps the generation). Call-free is +//! still the whole admission test. +//! +//! Two things ARE different, and both are narrowings: +//! +//! * the per-element residual drops `GC_OBJ_TYPED_LAYOUT_INTACT`. A parsed +//! record never has that bit — `object/json_construction.rs` finishes every +//! record with `layout_init_pointer_free` or `layout_mark_unknown`, and both +//! clear it — so keeping it would have side-exited on the FIRST element of +//! every loop while every IR-census assertion still passed. What the bit +//! bought was "this slot holds a raw `double`"; the shape arm buys the same +//! claim per read, from the value, with the Number-tag test +//! `emit_js_value_is_number` and a side exit to the slow clone. +//! * the index grammar widens to `arr[k]` and `const d = j % m; arr[d]` — the +//! `repeat` and `sequential` shapes of real access code. Each carries its +//! own preheader bounds obligation (`length > k`, `m <= length`), so the +//! clone still pays no per-read bounds test, and the matcher admits exactly +//! ONE index form per loop so a fact can never be consulted for a spelling +//! whose obligation was not discharged. +//! //! ## Extension plan (write-up for #5093 / #7480) //! -//! The clone still pays a residual per-element check — `keys_array` identity, -//! `field_count`, the per-object descriptor flag and the typed-layout intact -//! bit — because the array-level invariant deliberately does not cover them. -//! Folding them into `element_class_of_bits` would make the reads bare, but it +//! The clone still pays a residual per-element check — the exact ShapeId, the +//! per-object descriptor flag and (class arm) the typed-layout intact bit — +//! because the array-level invariant deliberately does not cover them. Folding +//! them into `element_identity_of_bits` would make the reads bare, but it //! needs an invalidation surface for `delete elem.f`, `defineProperty(elem)` //! and typed-layout downgrade that does not exist today; #7496 kept the //! maintenance matrix small precisely by not opening that surface. That is the @@ -128,33 +170,95 @@ enum ElementShapeLoopBound { ArrayLength(u32), } +/// #10123: which identity the clone keys the elements on. +#[derive(Debug)] +enum ElementShapeIdentity { + /// The original arm. A compile-time class supplies the id the preheader + /// compares against, the canonical keys global the expected ShapeId is + /// loaded from, and a packed slot index per property. + Class { + class_name: String, + expected_class_id: u32, + keys_global_name: String, + /// property name -> packed slot index. + packed_fields: std::collections::BTreeMap, + /// The native-region E1--E5 proof already establishes every element's + /// exact class for this array's whole lifetime. When true, the + /// preheader need not rebuild the weaker runtime invariant by scanning + /// the array. + statically_class_proven: bool, + /// The same contained group proof also established that every requested + /// field remains a raw-f64 slot, so per-object residual checks are + /// redundant inside the call-free clone. + statically_layout_proven: bool, + }, + /// #10123: an `any`-typed array of plain objects — canonically the result + /// of `JSON.parse`. There is no class and no compile-time layout; the + /// preheader asks the runtime for the exact ordinary ShapeId every element + /// carries and for each tracked property's inline slot in that shape, and + /// the per-element residual adds a Number-tag test on the loaded word. + Shape, +} + +/// #10123: the index spelling the whole body uses. +/// +/// One form per loop, deliberately: each carries its own preheader bounds +/// obligation (`expr::element_shape_guard::ElementShapeIndexBound`), so a body +/// mixing `arr[j]` with `arr[7]` would need both discharged and both matched +/// at every read. The matcher declines such a body instead. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum MatchedIndex { + Counter, + Constant(i64), + DerivedMod { local_id: u32, modulus_id: u32 }, +} + +impl MatchedIndex { + /// Mirror of [`crate::expr::ElementShapeIndex::needs_counter_i32_slot`], + /// asked before the fact exists. + fn needs_counter_i32_slot(self) -> bool { + !matches!(self, MatchedIndex::Constant(_)) + } +} + #[derive(Debug)] struct ElementShapeVersionedLoop { counter_id: u32, bound: ElementShapeLoopBound, array_id: u32, - class_name: String, - expected_class_id: u32, - keys_global_name: String, - /// The native-region E1--E5 proof already establishes every element's - /// exact class for this array's whole lifetime. When true, the preheader - /// need not rebuild the weaker runtime invariant by scanning the array. - statically_class_proven: bool, - /// The same contained group proof also established that every requested - /// field remains a raw-f64 slot, so per-object residual checks are - /// redundant inside the call-free clone. - statically_layout_proven: bool, - /// property name -> packed slot index. - fields: std::collections::BTreeMap, + identity: ElementShapeIdentity, + /// Tracked property names, in both arms. The class arm additionally + /// carries a packed slot index per name inside its `identity`. + props: std::collections::BTreeSet, + index: MatchedIndex, /// #7771: the body's `const r = arr[counter]` binding in the two-statement /// form; `None` for the original single-statement accumulator body. element_binding: Option, accumulator_id: u32, } +/// The locals the pure-expression walk reasons about. +#[derive(Clone, Copy)] +struct PureExprScope { + counter_id: u32, + accumulator_id: u32, + element_binding: Option, + /// #10123: `(d, m)` for a body whose first statement is + /// `const d = counter % m`. + derived: Option<(u32, u32)>, +} + +/// What the walk collects. +#[derive(Default)] +struct PureExprFacts { + array: Option, + props: std::collections::BTreeSet, + index: Option, +} + /// Effect-free expression walk for the element-shape loop. /// -/// Admits exactly: tracked `arr[counter].prop` reads on ONE array, numeric +/// Admits exactly: tracked `arr[].prop` reads on ONE array, numeric /// locals, numeric literals, and pure arithmetic / `Math` (libm intrinsics /// cannot trigger a GC). Everything else bails the whole match — a catch-all /// that silently accepted an unknown expression would be the #6377 shape, and @@ -163,11 +267,8 @@ struct ElementShapeVersionedLoop { fn element_shape_loop_pure_expr_collect( ctx: &FnCtx<'_>, expr: &perry_hir::Expr, - counter_id: u32, - accumulator_id: u32, - element_binding: Option, - array: &mut Option, - props: &mut std::collections::BTreeSet, + scope: &PureExprScope, + out: &mut PureExprFacts, ) -> bool { use perry_hir::Expr; match expr { @@ -175,46 +276,56 @@ fn element_shape_loop_pure_expr_collect( object, property, .. } => match object.as_ref() { Expr::IndexGet { object, index } => { - let (Expr::LocalGet(arr_id), Expr::LocalGet(idx_id)) = - (object.as_ref(), index.as_ref()) - else { + let Expr::LocalGet(arr_id) = object.as_ref() else { return false; }; - // The index must be the loop counter itself. An offset index - // (`arr[j + 1]`) would need the preheader's `length >= bound` - // check widened; deliberately out of the first slice. - if *idx_id != counter_id || *arr_id == counter_id { + if *arr_id == scope.counter_id { return false; } - match array { - Some(a) if *a == *arr_id => {} + // #10123: the admitted index spellings. `Counter` is the + // original one, and the two additions are the shapes real + // record-access loops are written in (`rows[7]`, and + // `const d = i % n; rows[d]`). An offset index (`arr[j + 1]`) + // is still out: it would need a bounds obligation of its own. + let Some(form) = match_index_form(index.as_ref(), scope) else { + return false; + }; + match out.index { + Some(seen) if seen == form => {} + Some(_) => return false, + None => out.index = Some(form), + } + match out.array { + Some(a) if a == *arr_id => {} Some(_) => return false, // one array per loop - None => *array = Some(*arr_id), + None => out.array = Some(*arr_id), } - props.insert(property.clone()); + out.props.insert(property.clone()); true } // #7771: `r.field` through the body's `const r = arr[counter]` // binding is the same tracked read spelled through the Let the // body match admitted; the binding already pins (array, counter), // so only the property is left to record. - Expr::LocalGet(recv_id) if element_binding == Some(*recv_id) => { - props.insert(property.clone()); + Expr::LocalGet(recv_id) if scope.element_binding == Some(*recv_id) => { + out.props.insert(property.clone()); true } _ => false, }, - // A bare read of the array, the counter, or the element binding as a - // VALUE could flow it into arbitrary lowering; only scalar reads the - // analysis proves numeric are admitted. The element binding is - // excluded EXPLICITLY rather than via the numeric test: a bare `r` - // would hand out a reference the clone's skipped `Let` never bound - // (#7771), and betting that exclusion on a type predicate is the + // A bare read of the array, the counter, the element binding or the + // derived index as a VALUE could flow it into arbitrary lowering; only + // scalar reads the analysis proves numeric are admitted. The element + // binding and the derived index are excluded EXPLICITLY rather than via + // the numeric test: both are bound by a `Let` the fast clone does not + // lower generically, so a bare read would hand out a reference nothing + // bound (#7771), and betting that exclusion on a type predicate is the // #6377 shape this walk's docs warn about. Expr::LocalGet(id) => { - element_binding != Some(*id) - && array.is_none_or(|a| a != *id) - && (*id == accumulator_id || crate::type_analysis::is_numeric_expr(ctx, expr)) + scope.element_binding != Some(*id) + && scope.derived.map(|(d, _)| d) != Some(*id) + && out.array.is_none_or(|a| a != *id) + && (*id == scope.accumulator_id || crate::type_analysis::is_numeric_expr(ctx, expr)) } Expr::Number(_) | Expr::Integer(_) => true, // NOTE (#7480 step 3): deliberately NOT gated on @@ -225,7 +336,8 @@ fn element_shape_loop_pure_expr_collect( // locals and literals by their own arms, and tracked `arr[j].field` // reads because the caller rejects the whole loop unless every // collected property is a declared raw-f64 candidate on the resolved - // element class. + // element class (class arm) or the emitted read tag-tests the loaded + // word and side-exits when it is not a Number (#10123's shape arm). // // The gate had to go for the object-literal kernel: at match time no // fact is installed yet, so `is_numeric_expr` cannot see through @@ -234,63 +346,19 @@ fn element_shape_loop_pure_expr_collect( // object-literal element). Keeping it would have declined #7480's own // kernel before the class resolver was ever consulted. Expr::Binary { left, right, .. } => { - element_shape_loop_pure_expr_collect( - ctx, - left, - counter_id, - accumulator_id, - element_binding, - array, - props, - ) && element_shape_loop_pure_expr_collect( - ctx, - right, - counter_id, - accumulator_id, - element_binding, - array, - props, - ) + element_shape_loop_pure_expr_collect(ctx, left, scope, out) + && element_shape_loop_pure_expr_collect(ctx, right, scope, out) + } + Expr::NumberCoerce(operand) => { + element_shape_loop_pure_expr_collect(ctx, operand, scope, out) } - Expr::NumberCoerce(operand) => element_shape_loop_pure_expr_collect( - ctx, - operand, - counter_id, - accumulator_id, - element_binding, - array, - props, - ), Expr::MathImul(left, right) | Expr::MathPow(left, right) => { - element_shape_loop_pure_expr_collect( - ctx, - left, - counter_id, - accumulator_id, - element_binding, - array, - props, - ) && element_shape_loop_pure_expr_collect( - ctx, - right, - counter_id, - accumulator_id, - element_binding, - array, - props, - ) + element_shape_loop_pure_expr_collect(ctx, left, scope, out) + && element_shape_loop_pure_expr_collect(ctx, right, scope, out) } - Expr::MathMin(values) | Expr::MathMax(values) => values.iter().all(|e| { - element_shape_loop_pure_expr_collect( - ctx, - e, - counter_id, - accumulator_id, - element_binding, - array, - props, - ) - }), + Expr::MathMin(values) | Expr::MathMax(values) => values + .iter() + .all(|e| element_shape_loop_pure_expr_collect(ctx, e, scope, out)), Expr::MathAbs(value) | Expr::MathSqrt(value) | Expr::MathFloor(value) @@ -298,19 +366,32 @@ fn element_shape_loop_pure_expr_collect( | Expr::MathRound(value) | Expr::MathTrunc(value) | Expr::MathSign(value) - | Expr::MathF16round(value) => element_shape_loop_pure_expr_collect( - ctx, - value, - counter_id, - accumulator_id, - element_binding, - array, - props, - ), + | Expr::MathF16round(value) => element_shape_loop_pure_expr_collect(ctx, value, scope, out), _ => false, } } +/// Classify one `arr[]` subscript. `None` declines the whole match. +fn match_index_form(index: &perry_hir::Expr, scope: &PureExprScope) -> Option { + use perry_hir::Expr; + match index { + Expr::LocalGet(id) if *id == scope.counter_id => Some(MatchedIndex::Counter), + Expr::LocalGet(id) => match scope.derived { + Some((derived_id, modulus_id)) if derived_id == *id => Some(MatchedIndex::DerivedMod { + local_id: derived_id, + modulus_id, + }), + _ => None, + }, + // `0..=i32::MAX`, so the preheader's `length > k` compare is an i32 + // one and the emitted index needs no conversion. + Expr::Integer(k) if (0..=i64::from(i32::MAX)).contains(k) => { + Some(MatchedIndex::Constant(*k)) + } + _ => None, + } +} + /// Resolve the class every element of `array_id` must have for the clone to /// fire. /// @@ -509,7 +590,46 @@ fn anon_shape_field_type_is_compatible( } } -/// Match `for (let j = k0; j < B; j++) acc = `. +/// Is `array_id` an untyped receiver — the shape-keyed arm's entry condition? +/// +/// The class resolver having declined is not on its own enough: a `Node[]` +/// whose class the module does not define, or a shape the anon-shape resolver +/// found ambiguous, are both "declined" and both name a layout this arm would +/// be guessing about. Requiring the declared type to be absent / `any` / +/// `unknown` keeps #10123 to exactly the receivers that carry no layout claim +/// at all, which is what `JSON.parse` hands back. +fn array_is_untyped(ctx: &FnCtx<'_>, array_id: u32) -> bool { + use perry_hir::types::Type; + matches!( + ctx.local_type_hint(&array_id) + .map(|ty| resolve_type_alias(ctx, ty)), + None | Some(Type::Any) | Some(Type::Unknown) + ) +} + +/// A modulus local for #10123's `const d = counter % m` index. +/// +/// `m` must be readable and provably unchanged for the loop's duration: the +/// preheader materializes it ONCE and the clone's `srem` uses that value for +/// every iteration, so a body that could rewrite `m` would derive indices +/// against a stale bound. `local_bound_is_loop_invariant` answers exactly +/// that (it looks for WRITES, so the `const d = counter % m` read itself is +/// not a mutation). +fn modulus_local_is_admissible( + ctx: &FnCtx<'_>, + modulus_id: u32, + condition: &perry_hir::Expr, + update: Option<&perry_hir::Expr>, + body: &[Stmt], +) -> bool { + !ctx.boxed_vars.contains(&modulus_id) + && !ctx.closure_captures.contains_key(&modulus_id) + && (local_has_readable_slot(ctx, modulus_id) + || ctx.module_globals.contains_key(&modulus_id)) + && local_bound_is_loop_invariant(condition, update, body, modulus_id) +} + +/// Match `for (let j = k0; j < B; j++) acc = ].field>`. /// /// The single-statement, store-free body is the revocation argument (see the /// module docs) AND the side-exit protocol: the residual per-element check @@ -522,7 +642,7 @@ fn match_element_shape_versioned_loop( update: Option<&perry_hir::Expr>, body: &[Stmt], ) -> Option { - use perry_hir::{CompareOp, Expr, UpdateOp}; + use perry_hir::{BinaryOp, CompareOp, Expr, UpdateOp}; // Oversized modules full-outline the class-field diamonds for code size; // a clone that re-inlines them there would fight that decision. @@ -613,50 +733,87 @@ fn match_element_shape_versioned_loop( return None; } - // Store-free body, in one of two admitted shapes (see the module docs): + // Store-free body, in one of three admitted shapes (see the module docs): // - // 1. `acc = ` — the original single - // statement; + // 1. `acc = ].field>` — the original + // single statement; // 2. `const r = arr[j]; acc = ` — #7771's - // element-binding form, the shape real read loops are written in. - // The binding is VIRTUAL inside the fast clone: its `Let` emits - // nothing (`stmt/let_stmt.rs`) and every `r.field` lowers through - // the fact, so the revocation argument (no store, no call in the - // clone) is unchanged. `const`-only, deliberately: a `var` binding - // is function-scoped and observable after the loop, where the - // skipped `Let` would leave the slot holding its pre-loop value. + // element-binding form, the shape real read loops are written in; + // 3. `const d = j % m; acc = ` — + // #10123's derived-index form, the shape every sequential pass over a + // parsed record array is written in. + // + // In 2 and 3 the binding is VIRTUAL inside the fast clone: its `Let` + // emits nothing (form 2) or one `srem i32` (form 3) rather than the + // generic lowering (`stmt/let_stmt.rs`), so the revocation argument (no + // store, no call in the clone) is unchanged. `const`-only, deliberately: a + // `var` binding is function-scoped and observable after the loop, where + // the skipped `Let` would leave the slot holding its pre-loop value. // // NOTHING else is admitted. - let (element_binding, acc_id, value) = match body { - [Stmt::Expr(Expr::LocalSet(acc_id, value))] => (None, acc_id, value), + let mut element_binding: Option<(u32, u32)> = None; + let mut derived: Option<(u32, u32)> = None; + let (acc_id, value) = match body { + [Stmt::Expr(Expr::LocalSet(acc_id, value))] => (acc_id, value), [Stmt::Let { id, mutable: false, - init: Some(Expr::IndexGet { object, index }), + init: Some(binding_init), .. }, Stmt::Expr(Expr::LocalSet(acc_id, value))] => { - let (Expr::LocalGet(arr_id), Expr::LocalGet(idx_id)) = - (object.as_ref(), index.as_ref()) - else { - return None; - }; - // Same receiver/index discipline as the walk's IndexGet arm: the - // fetch must be `arr[counter]` exactly. - if *idx_id != counter_id || *arr_id == counter_id || *id == counter_id { - return None; - } - // The binding must be a plain, loop-owned const local. A boxed or - // captured binding lives in a cell the skipped `Let` would leave - // stale for an observer outside the clone; a module-global id is - // not a body-scoped binding at all. - if *id == *arr_id + // The binding must be a plain, loop-owned const local in either + // form. A boxed or captured binding lives in a cell the clone's + // replacement `Let` would leave stale for an observer outside the + // clone; a module-global id is not a body-scoped binding at all. + if *id == counter_id || ctx.boxed_vars.contains(id) || ctx.module_globals.contains_key(id) || ctx.closure_captures.contains_key(id) { return None; } - (Some((*id, *arr_id)), acc_id, value) + match binding_init { + Expr::IndexGet { object, index } => { + let (Expr::LocalGet(arr_id), Expr::LocalGet(idx_id)) = + (object.as_ref(), index.as_ref()) + else { + return None; + }; + // Same receiver/index discipline as the walk's IndexGet + // arm: the fetch must be `arr[counter]` exactly. + if *idx_id != counter_id || *arr_id == counter_id || *id == *arr_id { + return None; + } + element_binding = Some((*id, *arr_id)); + } + // #10123. `%` on two locals, the left one the counter. The + // modulus is validated below (it must be readable and + // loop-invariant), and the RANGE obligation — `1 <= m` so the + // `srem` cannot divide by zero, `m <= length` so every derived + // index is in bounds — is discharged in the preheader, which + // is also where a non-number / fractional `m` sends the loop + // to the slow clone. + Expr::Binary { + op: BinaryOp::Mod, + left, + right, + } => { + let (Expr::LocalGet(num_id), Expr::LocalGet(modulus_id)) = + (left.as_ref(), right.as_ref()) + else { + return None; + }; + if *num_id != counter_id || *modulus_id == counter_id || *id == *modulus_id { + return None; + } + if !modulus_local_is_admissible(ctx, *modulus_id, condition?, update, body) { + return None; + } + derived = Some((*id, *modulus_id)); + } + _ => return None, + } + (acc_id, value) } _ => return None, }; @@ -664,39 +821,53 @@ fn match_element_shape_versioned_loop( || !ctx.locals.contains_key(acc_id) || ctx.boxed_vars.contains(acc_id) || ctx.module_globals.contains_key(acc_id) - // The declared type is only a candidate. The lowering validates the - // accumulator's current NaN-box tag in the preheader before installing - // the numeric fact for the fast clone. - || !matches!(ctx.local_type_hint(acc_id), Some(perry_hir::types::Type::Number | perry_hir::types::Type::Int32)) { return None; } // The binding form pins the array before the walk runs, so a body mixing // `r.field` with `other[j].field` is declined by the walk's one-array rule. - let mut array: Option = element_binding.map(|(_, arr_id)| arr_id); - let element_binding = element_binding.map(|(id, _)| id); - if element_binding == Some(*acc_id) { + let scope = PureExprScope { + counter_id, + accumulator_id: *acc_id, + element_binding: element_binding.map(|(id, _)| id), + derived, + }; + if scope.element_binding == Some(*acc_id) || derived.map(|(d, _)| d) == Some(*acc_id) { return None; } - let mut props: std::collections::BTreeSet = std::collections::BTreeSet::new(); - if !element_shape_loop_pure_expr_collect( - ctx, - value, - counter_id, - *acc_id, - element_binding, - &mut array, - &mut props, - ) { + let mut facts = PureExprFacts { + array: element_binding.map(|(_, arr_id)| arr_id), + ..PureExprFacts::default() + }; + if !element_shape_loop_pure_expr_collect(ctx, value, &scope, &mut facts) { return None; } - let array_id = array?; - if props.is_empty() || array_id == *acc_id || array_id == counter_id { + let array_id = facts.array?; + if facts.props.is_empty() || array_id == *acc_id || array_id == counter_id { return None; } + // The element-binding form carries no subscript at the reads, so its index + // form comes from the binding rather than from the walk. + let index = match element_binding { + Some(_) => MatchedIndex::Counter, + None => facts.index?, + }; + if let MatchedIndex::DerivedMod { + local_id, + modulus_id, + } = index + { + if modulus_id == array_id || modulus_id == *acc_id || local_id == array_id { + return None; + } + } match bound { ElementShapeLoopBound::Local(bound_id) => { - if bound_id == array_id || bound_id == *acc_id || Some(bound_id) == element_binding { + if bound_id == array_id + || bound_id == *acc_id + || Some(bound_id) == scope.element_binding + || derived.map(|(d, _)| d) == Some(bound_id) + { return None; } } @@ -728,7 +899,7 @@ fn match_element_shape_versioned_loop( // #7480: the preheader must be able to write the growth-forwarding-repaired // head BACK into the binding (see // `expr::element_shape_guard::emit_element_shape_loop_preheader_check` - // step 2b). A closure-captured array lives in a capture cell that a plain + // step 2). A closure-captured array lives in a capture cell that a plain // slot store would not update, so the two views could disagree; decline // rather than repair only half of them. if ctx.closure_captures.contains_key(&array_id) { @@ -738,11 +909,72 @@ fn match_element_shape_versioned_loop( return None; } - let class_name = element_class_name(ctx, array_id, counter_id)?; - if CLASS_FIELD_LOOP_CLASS_DENYLIST.contains(&class_name.as_str()) { + for prop in &facts.props { + if CLASS_FIELD_LOOP_PROP_DENYLIST.contains(&prop.as_str()) { + return None; + } + } + + let identity = match element_class_name(ctx, array_id, counter_id) { + Some(class_name) => { + // The class arm keeps its ORIGINAL grammar. Its bounds argument is + // "the trip count covers every index", which only `Counter` + // satisfies, and widening it here would change the emitted code + // for receivers #10123 never measured. + if index != MatchedIndex::Counter { + return None; + } + match_class_identity(ctx, array_id, &class_name, &facts.props)? + } + // #10123. A receiver that declares no element type at all is the + // `JSON.parse` case: nothing is known statically, so everything is + // asked of the runtime in the preheader. + None if array_is_untyped(ctx, array_id) => ElementShapeIdentity::Shape, + None => return None, + }; + + // The declared accumulator type is only a candidate: the lowering + // validates the accumulator's current NaN-box tag in the preheader before + // installing the numeric fact for the fast clone. The class arm keeps its + // original Number/Int32 requirement; the shape arm also admits an + // untyped accumulator, because `let sum = 0; sum += rows[i].id` over an + // `any` array is the shape this optimization exists for and HIR widens + // `sum` to `Any` exactly when the element read is untyped. The preheader + // tag check is what makes that safe, and it is emitted either way. + let accumulator_hint_ok = match ctx.local_type_hint(acc_id) { + Some(perry_hir::types::Type::Number | perry_hir::types::Type::Int32) => true, + None | Some(perry_hir::types::Type::Any | perry_hir::types::Type::Unknown) => { + matches!(identity, ElementShapeIdentity::Shape) + } + _ => false, + }; + if !accumulator_hint_ok { return None; } - let class = ctx.classes.get(&class_name)?; + + Some(ElementShapeVersionedLoop { + counter_id, + bound, + array_id, + identity, + props: facts.props, + index, + element_binding: scope.element_binding, + accumulator_id: *acc_id, + }) +} + +/// Resolve the class arm's compile-time facts, or decline. +fn match_class_identity( + ctx: &FnCtx<'_>, + array_id: u32, + class_name: &str, + props: &std::collections::BTreeSet, +) -> Option { + if CLASS_FIELD_LOOP_CLASS_DENYLIST.contains(&class_name) { + return None; + } + let class = ctx.classes.get(class_name)?; if !class.computed_members.is_empty() { return None; } @@ -752,59 +984,97 @@ fn match_element_shape_versioned_loop( if class.extends_name.is_some() { return None; } - let expected_class_id = *ctx.class_ids.get(&class_name)?; - let keys_global_name = ctx.class_keys_globals.get(&class_name)?.clone(); + let expected_class_id = *ctx.class_ids.get(class_name)?; + let keys_global_name = ctx.class_keys_globals.get(class_name)?.clone(); let statically_class_proven = ctx .native_facts .exact_element_class(array_id) .is_some_and(|proven| proven == class_name); - let mut fields = std::collections::BTreeMap::new(); + let mut packed_fields = std::collections::BTreeMap::new(); for prop in props { - if CLASS_FIELD_LOOP_PROP_DENYLIST.contains(&prop.as_str()) { - return None; - } // Accessors route through synthesized __get_/__set_ methods before the // class-field diamond; mirror that dispatch gate exactly. if ctx .methods - .contains_key(&(class_name.clone(), format!("__get_{prop}"))) + .contains_key(&(class_name.to_string(), format!("__get_{prop}"))) || ctx .methods - .contains_key(&(class_name.clone(), format!("__set_{prop}"))) + .contains_key(&(class_name.to_string(), format!("__set_{prop}"))) { return None; } - let field_index = crate::type_analysis::class_field_global_index(ctx, &class_name, &prop)?; - let raw_f64 = crate::type_analysis::class_field_declared_type(ctx, &class_name, &prop) + let field_index = crate::type_analysis::class_field_global_index(ctx, class_name, prop)?; + let raw_f64 = crate::type_analysis::class_field_declared_type(ctx, class_name, prop) .as_ref() .is_some_and(crate::typed_shape::type_is_raw_f64_candidate); if !raw_f64 { return None; } - fields.insert(prop, field_index); + packed_fields.insert(prop.clone(), field_index); } let statically_layout_proven = statically_class_proven && ctx .native_facts .exact_numeric_element_fields(array_id) - .is_some_and(|proven| fields.keys().all(|field| proven.contains(field))); + .is_some_and(|proven| packed_fields.keys().all(|field| proven.contains(field))); - Some(ElementShapeVersionedLoop { - counter_id, - bound, - array_id, - class_name, + Some(ElementShapeIdentity::Class { + class_name: class_name.to_string(), expected_class_id, keys_global_name, + packed_fields, statically_class_proven, statically_layout_proven, - fields, - element_binding, - accumulator_id: *acc_id, }) } +/// Materialize a NaN-boxed local as an i32 in `[min, max]`, taking the slow +/// clone on a non-number, out-of-range or fractional value. +/// +/// Leaves `ctx.current_block` on a fresh block dominated by all three checks, +/// and returns the i32 SSA value. Call-free by construction — the whole point +/// is that the clone's hot path reads an i32 it can trust. +fn materialize_loop_i32( + ctx: &mut FnCtx<'_>, + local_id: u32, + min: i32, + max: i32, + slow_label: &str, + label_prefix: &str, +) -> Result { + let value = lower_expr(ctx, &perry_hir::Expr::LocalGet(local_id))?; + let is_number = emit_js_value_is_number(ctx, &value); + let range_idx = ctx.new_block(&format!("{label_prefix}.range")); + let convert_idx = ctx.new_block(&format!("{label_prefix}.convert")); + let done_idx = ctx.new_block(&format!("{label_prefix}.ok")); + let range_label = ctx.block_label(range_idx); + let convert_label = ctx.block_label(convert_idx); + let done_label = ctx.block_label(done_idx); + ctx.block().cond_br(&is_number, &range_label, slow_label); + + ctx.current_block = range_idx; + let ge_min = { + let min_literal = format!("{:.1}", f64::from(min)); + ctx.block().fcmp("oge", &value, &min_literal) + }; + let le_max = { + let max_literal = format!("{:.1}", f64::from(max)); + ctx.block().fcmp("ole", &value, &max_literal) + }; + let in_range = ctx.block().and(I1, &ge_min, &le_max); + ctx.block().cond_br(&in_range, &convert_label, slow_label); + + ctx.current_block = convert_idx; + let as_i32 = ctx.block().fptosi(DOUBLE, &value, I32); + let roundtrip = ctx.block().sitofp(I32, &as_i32, DOUBLE); + let is_integral = ctx.block().fcmp("oeq", &roundtrip, &value); + ctx.block().cond_br(&is_integral, &done_label, slow_label); + + ctx.current_block = done_idx; + Ok(as_i32) +} + /// Lower the matched loop as a guarded fast clone plus the unchanged generic /// body, modeled on `lower_class_field_versioned_for`. /// @@ -824,9 +1094,15 @@ pub(super) fn lower_element_shape_versioned_for( else { return Ok(false); }; - // The fast clone reads the counter through its canonical i32 slot; without - // one it would win nothing (and the element GEP would need an fptosi). - if !ctx.i32_counter_slots.contains_key(&matched.counter_id) { + // A counter- or modulo-derived index reads the counter through its + // canonical i32 slot; without one the element GEP would need an fptosi and + // the clone would win nothing. A CONSTANT index never reads the counter + // (#10123), and its loops are exactly the ones `stmt/let_stmt.rs` mints no + // i32 slot for — the counter is neither index-used nor i32-bounded — so + // demanding one there would decline the shape this arm was written for. + if matched.index.needs_counter_i32_slot() + && !ctx.i32_counter_slots.contains_key(&matched.counter_id) + { return Ok(false); } @@ -844,38 +1120,30 @@ pub(super) fn lower_element_shape_versioned_for( let materialized_bound: Option = match matched.bound { ElementShapeLoopBound::ArrayLength(_) => None, ElementShapeLoopBound::Constant(k) => Some(k.to_string()), - ElementShapeLoopBound::Local(bound_id) => Some({ - let bound_d = lower_expr(ctx, &perry_hir::Expr::LocalGet(bound_id))?; - let is_number = emit_js_value_is_number(ctx, &bound_d); - let range_idx = ctx.new_block("element_shape.loop.bound.range"); - let convert_idx = ctx.new_block("element_shape.loop.bound.convert"); - let check_idx = ctx.new_block("element_shape.loop.shape_check"); - let range_label = ctx.block_label(range_idx); - let convert_label = ctx.block_label(convert_idx); - let check_label = ctx.block_label(check_idx); - ctx.block() - .cond_br(&is_number, &range_label, &slow_pre_label); - - ctx.current_block = range_idx; - let ge_zero = ctx.block().fcmp("oge", &bound_d, "0.0"); - let le_max = { - let max_literal = format!("{:.1}", i32::MAX as f64); - ctx.block().fcmp("ole", &bound_d, &max_literal) - }; - let in_range = ctx.block().and(I1, &ge_zero, &le_max); - ctx.block() - .cond_br(&in_range, &convert_label, &slow_pre_label); - - ctx.current_block = convert_idx; - let bound_i32 = ctx.block().fptosi(DOUBLE, &bound_d, I32); - let roundtrip = ctx.block().sitofp(I32, &bound_i32, DOUBLE); - let is_integral = ctx.block().fcmp("oeq", &roundtrip, &bound_d); - ctx.block() - .cond_br(&is_integral, &check_label, &slow_pre_label); - - ctx.current_block = check_idx; - bound_i32 - }), + ElementShapeLoopBound::Local(bound_id) => Some(materialize_loop_i32( + ctx, + bound_id, + 0, + i32::MAX, + &slow_pre_label, + "element_shape.loop.bound", + )?), + }; + + // #10123: the same materialization for a derived index's modulus, with a + // floor of 1 — `srem` by zero is undefined behaviour, and `x % 0` is NaN + // in JS, so a zero modulus is a slow-clone case rather than something the + // clone may compute. + let modulus_i32: Option = match matched.index { + MatchedIndex::DerivedMod { modulus_id, .. } => Some(materialize_loop_i32( + ctx, + modulus_id, + 1, + i32::MAX, + &slow_pre_label, + "element_shape.loop.modulus", + )?), + _ => None, }; let trip_count = match &materialized_bound { @@ -884,24 +1152,113 @@ pub(super) fn lower_element_shape_versioned_for( } None => crate::expr::element_shape_guard::ElementShapeLoopTripCount::ArrayLength, }; - let expected_class_id_str = matched.expected_class_id.to_string(); - let (elements_base, expected_shape_id, shape_ok, bound_i32) = - crate::expr::element_shape_guard::emit_element_shape_loop_preheader_check( - ctx, - matched.array_id, - &expected_class_id_str, - &matched.keys_global_name, - trip_count, - &slow_pre_label, - matched.statically_class_proven, - )?; + let index_bound = match (&matched.index, &modulus_i32) { + (MatchedIndex::Counter, _) => { + crate::expr::element_shape_guard::ElementShapeIndexBound::FromTripCount + } + (MatchedIndex::Constant(k), _) => { + crate::expr::element_shape_guard::ElementShapeIndexBound::Constant(*k) + } + (MatchedIndex::DerivedMod { .. }, Some(modulus)) => { + crate::expr::element_shape_guard::ElementShapeIndexBound::Modulus(modulus.as_str()) + } + (MatchedIndex::DerivedMod { .. }, None) => { + unreachable!("a derived index always materializes its modulus") + } + }; + let expected_class_id_str = match &matched.identity { + ElementShapeIdentity::Class { + expected_class_id, .. + } => expected_class_id.to_string(), + ElementShapeIdentity::Shape => String::new(), + }; + let guard_kind = match &matched.identity { + ElementShapeIdentity::Class { + keys_global_name, .. + } => crate::expr::element_shape_guard::ElementShapeGuardKind::Class { + expected_class_id: expected_class_id_str.as_str(), + keys_global_name: keys_global_name.as_str(), + }, + ElementShapeIdentity::Shape => { + crate::expr::element_shape_guard::ElementShapeGuardKind::Shape { + properties: &matched.props, + } + } + }; + let statically_proven = matches!( + matched.identity, + ElementShapeIdentity::Class { + statically_class_proven: true, + .. + } + ); + let guard = crate::expr::element_shape_guard::emit_element_shape_loop_preheader_check( + ctx, + matched.array_id, + guard_kind, + trip_count, + index_bound, + &slow_pre_label, + statically_proven, + )?; let accumulator = lower_expr(ctx, &perry_hir::Expr::LocalGet(matched.accumulator_id))?; let accumulator_is_number = emit_js_value_is_number(ctx, &accumulator); - let fast_path_ok = ctx.block().and(I1, &shape_ok, &accumulator_is_number); + let fast_path_ok = ctx.block().and(I1, &guard.shape_ok, &accumulator_is_number); // Deliberately unterminated: it branches into the fast clone only after // the clone is PROVEN call-free below. let deref_idx = ctx.current_block; + let (shape_keyed, statically_layout_proven, fields, report_class) = match &matched.identity { + ElementShapeIdentity::Class { + class_name, + packed_fields, + statically_layout_proven, + .. + } => ( + false, + *statically_layout_proven, + packed_fields + .iter() + .map(|(prop, index)| { + ( + prop.clone(), + crate::expr::ElementShapeFieldSlot::Packed(*index), + ) + }) + .collect(), + class_name.clone(), + ), + ElementShapeIdentity::Shape => ( + true, + false, + guard + .field_slots + .iter() + .map(|(prop, slot)| { + ( + prop.clone(), + crate::expr::ElementShapeFieldSlot::Runtime(slot.clone()), + ) + }) + .collect(), + "".to_string(), + ), + }; + let fact_index = match matched.index { + MatchedIndex::Counter => crate::expr::ElementShapeIndex::Counter, + MatchedIndex::Constant(k) => crate::expr::ElementShapeIndex::Constant(k), + MatchedIndex::DerivedMod { local_id, .. } => crate::expr::ElementShapeIndex::DerivedMod { + local_id, + modulus_i32: modulus_i32 + .clone() + .expect("a derived index always materializes its modulus"), + // Entry-block alloca: LLVM lowers a non-entry `alloca` as a real + // stack bump with no restore, and this one is written once per + // iteration. + slot: ctx.func.alloca_entry(I32), + }, + }; + let scope_id = ctx.next_loop_proof_scope_id(); let fast_scan_start = ctx.func.num_blocks(); ctx.current_block = fast_pre_idx; @@ -909,13 +1266,15 @@ pub(super) fn lower_element_shape_versioned_for( .push(crate::expr::ElementShapeLoopFact { array_local_id: matched.array_id, index_local_id: matched.counter_id, + index: fact_index, + shape_keyed, scope_id, - class_name: matched.class_name.clone(), - elements_base, - expected_shape_id, + class_name: report_class, + elements_base: guard.elements_base, + expected_shape_id: guard.expected_shape_id, side_exit_label: slow_pre_label.clone(), - statically_layout_proven: matched.statically_layout_proven, - fields: matched.fields.clone(), + statically_layout_proven, + fields, element_binding: matched.element_binding, numeric_accumulator: matched.accumulator_id, }); @@ -926,7 +1285,7 @@ pub(super) fn lower_element_shape_versioned_for( update, body, "for.element_shape_fast", - Some((matched.counter_id, bound_i32)), + Some((matched.counter_id, guard.bound_i32)), ); ctx.element_shape_loop_facts .retain(|fact| fact.scope_id != scope_id); @@ -973,6 +1332,27 @@ pub(super) fn lower_element_shape_versioned_for( None, ), }; + let (mode, described) = match &matched.identity { + ElementShapeIdentity::Class { + class_name, + statically_class_proven, + statically_layout_proven, + .. + } => ( + if *statically_layout_proven { + "statically layout-proven" + } else if *statically_class_proven { + "statically proven" + } else { + "runtime-guarded" + }, + format!("class {class_name}"), + ), + ElementShapeIdentity::Shape => ( + "runtime-guarded", + "runtime ordinary shape (class-less records)".to_string(), + ), + }; crate::opt_report::select( crate::opt_report::Position::Local, &name, @@ -981,17 +1361,9 @@ pub(super) fn lower_element_shape_versioned_for( "Ptr", 1, Some(format!( - "element-shape loop clone ({}): class {}, {} tracked field(s); \ + "element-shape loop clone ({mode}): {described}, {} tracked field(s); \ element reads in this loop lower to offset loads behind the preheader guard", - if matched.statically_layout_proven { - "statically layout-proven" - } else if matched.statically_class_proven { - "statically proven" - } else { - "runtime-guarded" - }, - matched.class_name, - matched.fields.len() + matched.props.len() )), ); } 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..1ca8524a06 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -369,9 +369,15 @@ fn assert_fast_clone_is_entered(ir: &str) { } /// The emitted text the fast clone owns: exactly the blocks named -/// `for.element_shape_fast.*` and any `element_shape.load` blocks its -/// runtime-guarded field reads branch into. A statically layout-proven clone -/// keeps the field load directly in its body and owns no such side-exit block. +/// `for.element_shape_fast.*`, any `element_shape.load` blocks its +/// runtime-guarded field reads branch into, and (#10123) any +/// `element_shape.number` blocks a shape-keyed read's tag test branches into. +/// A statically layout-proven clone keeps the field load directly in its body +/// and owns no such side-exit block. +/// +/// Every block the clone can execute must be listed here, not just the ones a +/// given assertion is about: the negatives below (call-free, no element-read +/// tier) are only true of the clone if the slice really is the whole clone. /// /// #7480 step 3 — ANTI-VACUITY. This used to slice from the first *substring* /// occurrence of `for.element_shape_fast.cond`, which is the @@ -402,7 +408,8 @@ fn fast_clone_slice(ir: &str) -> String { // belongs to whichever block was last opened. if !line.starts_with(char::is_whitespace) && trimmed.ends_with(':') { in_fast_block = trimmed.starts_with("for.element_shape_fast.") - || trimmed.starts_with("element_shape.load"); + || trimmed.starts_with("element_shape.load") + || trimmed.starts_with("element_shape.number"); } if in_fast_block { owned.push_str(line); @@ -1609,3 +1616,436 @@ fn element_binding_form_through_a_parameter_gets_the_clone() { let ir = emit(&m); assert_clone_fires_call_free(&ir, "parameter binding form"); } + +// --------------------------------------------------------------------------- +// #10123 — SHAPE-KEYED clones over an untyped (`any`) record array. +// +// This is the `JSON.parse` shape: `function run(rows: any, count: number)`, +// with the array's element identity known only at run time. Four things had to +// change together for it to fire, and every one of them is asserted below, +// because each alone still compiles and still prints the right answer: +// +// 1. the preheader asks `js_array_ensure_element_shape_ordinary` (the +// class-keyed query returns 0 for a class-0 record array, so a clone +// built on it would never be entered); +// 2. it resolves each tracked property to an inline slot with +// `js_shape_ordinary_inline_slot_for_key` (there is no class to bake a +// packed index from); +// 3. the residual per-element mask DROPS the typed-layout bit — a parsed +// record never has it, so the class-keyed mask would side-exit on the +// FIRST element of every loop while every label assertion still passed; +// 4. the loaded word is tag-tested as a Number, because a shape says which +// slot holds `id` and nothing about what is in it. +// --------------------------------------------------------------------------- + +const ROWS_ID: u32 = 31; +const COUNT_ID: u32 = 32; +const MOD_ID: u32 = 33; +const U_SUM_ID: u32 = 34; +const U_COUNTER_ID: u32 = 35; +const U_INDEX_ID: u32 = 36; + +/// `rows[].` +fn untyped_elem_field(index: Expr, prop: &str) -> Expr { + Expr::PropertyGet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ROWS_ID)), + index: Box::new(index), + }), + property: prop.to_string(), + byte_offset: 0, + } +} + +/// `sum = sum + ` +fn untyped_accumulate(value: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet( + U_SUM_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(U_SUM_ID)), + right: Box::new(value), + }), + )) +} + +/// `const index = i % n;` +fn derived_index_stmt(modulus: Expr) -> Stmt { + Stmt::Let { + id: U_INDEX_ID, + name: "index".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::Binary { + op: BinaryOp::Mod, + left: Box::new(Expr::LocalGet(U_COUNTER_ID)), + right: Box::new(modulus), + }), + } +} + +/// The benchmark's own function, parametrized: +/// +/// ```text +/// function run(rows: , count: number, n: number): number { +/// let sum: = 0; +/// for (let i = 0; i < count; i++) +/// return sum; +/// } +/// ``` +fn untyped_param_module(rows_ty: Type, sum_ty: Type, body: Vec) -> Module { + let mut m = Module::new("element_shape_loop.ts"); + m.functions = vec![perry_hir::Function { + id: 901, + name: "run".to_string(), + type_params: Vec::new(), + params: vec![ + perry_hir::Param { + id: ROWS_ID, + name: "rows".to_string(), + ty: rows_ty, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + perry_hir::Param { + id: COUNT_ID, + name: "count".to_string(), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + perry_hir::Param { + id: MOD_ID, + name: "n".to_string(), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + ], + return_type: Type::Number, + body: vec![ + Stmt::Let { + id: U_SUM_ID, + name: "sum".to_string(), + ty: sum_ty, + mutable: true, + init: Some(Expr::Integer(0)), + }, + Stmt::For { + init: Some(Box::new(Stmt::Let { + id: U_COUNTER_ID, + name: "i".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(0)), + })), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(U_COUNTER_ID)), + right: Box::new(Expr::LocalGet(COUNT_ID)), + }), + update: Some(Expr::Update { + id: U_COUNTER_ID, + op: UpdateOp::Increment, + prefix: false, + }), + body, + }, + Stmt::Return(Some(Expr::LocalGet(U_SUM_ID))), + ], + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }]; + m.init_kind = ModuleInitKind::Eager; + m +} + +/// The four shape-keyed obligations, asserted together. +fn assert_shape_keyed_clone(ir: &str, what: &str) { + assert_clone_fires_call_free(ir, what); + assert!( + ir.contains("call i32 @js_array_ensure_element_shape_ordinary"), + "{what}: the preheader must ask the SHAPE query — the class-keyed one \ + answers 0 for a class-0 record array, so a clone guarded on it could \ + never be entered" + ); + assert!( + !ir.contains("call i32 @js_array_ensure_element_shape("), + "{what}: an untyped array has no class to compare against" + ); + assert!( + ir.contains("call i32 @js_shape_ordinary_inline_slot_for_key"), + "{what}: each tracked property's inline slot must be resolved once in \ + the preheader; there is no class to bake a packed index from" + ); + let fast = fast_clone_slice(ir); + assert!( + fast.contains("134250751"), + "{what}: the fast clone must use the shape-keyed residual mask \ + (0x0800_80FF); emitted:\n{fast}" + ); + assert!( + !fast.contains("402686207"), + "{what}: the class-keyed mask requires GC_OBJ_TYPED_LAYOUT_INTACT, \ + which a JSON record NEVER has — using it here would side-exit on the \ + first element of every loop while every label assertion still passed; \ + emitted:\n{fast}" + ); + assert!( + fast.contains("element_shape.number"), + "{what}: a shape says which slot holds the field, not what is in it — \ + the loaded word must be tag-tested as a Number before it is consumed \ + as a raw double; emitted:\n{fast}" + ); + assert!( + fast.contains("getelementptr double"), + "{what}: the read must still be a bare offset load" + ); +} + +#[test] +fn an_untyped_record_array_gets_a_shape_keyed_clone() { + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "v", + ))], + )); + assert_shape_keyed_clone(&ir, "counter-indexed untyped array"); +} + +#[test] +fn a_constant_index_gets_a_shape_keyed_clone_with_a_hoisted_bounds_check() { + // `for (let i = 0; i < count; i++) sum += rows[7].id` — the benchmark's + // `repeat` mode. The trip count says NOTHING about index 7, so the + // preheader owes its own `length > 7`, and the clone owes no per-read + // bounds test in exchange. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::Integer(7), + "v", + ))], + )); + assert_shape_keyed_clone(&ir, "constant index"); + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert!( + deref.contains("icmp ugt i32") && deref.contains(", 7"), + "the preheader must prove `length > 7` before the clone is reachable; \ + emitted:\n{deref}" + ); + let fast = fast_clone_slice(&ir); + assert!( + !fast.contains("icmp ult i32") && !fast.contains("icmp ugt i32"), + "the bounds obligation is discharged ONCE in the preheader; a per-read \ + test in the clone would mean it was not; emitted:\n{fast}" + ); +} + +#[test] +fn a_derived_modulo_index_gets_a_shape_keyed_clone_with_an_srem_in_the_body() { + // `const index = i % n; sum += rows[index].id` — the benchmark's + // `sequential` mode, and the shape every wrap-around pass over a record + // array is written in. The generic `%` lowering is a runtime call, which + // inside this clone would DELETE it (#7690), so the `Let` must become one + // `srem`. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![ + derived_index_stmt(Expr::LocalGet(MOD_ID)), + untyped_accumulate(untyped_elem_field(Expr::LocalGet(U_INDEX_ID), "v")), + ], + )); + assert_shape_keyed_clone(&ir, "derived modulo index"); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("srem i32"), + "the derived index must be one `srem` inside the clone; emitted:\n{fast}" + ); + assert!( + ir.contains("element_shape.loop.modulus.range"), + "the modulus must be materialized as a validated i32 in the preheader \ + — `srem` by zero is UB and `x % 0` is NaN in JS" + ); + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert!( + deref.contains("icmp uge i32"), + "the preheader must prove `modulus <= length`, which is what makes \ + every derived index in bounds with no per-read test; emitted:\n{deref}" + ); +} + +#[test] +fn a_shape_keyed_clone_admits_an_untyped_accumulator() { + // `let sum = 0; sum += rows[i].id` widens `sum` to `Any` in HIR exactly + // when the element read is untyped — which is every program this clone + // exists for. The preheader's tag check on the accumulator is what makes + // the numeric fact sound, and it is emitted either way. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Any, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "v", + ))], + )); + assert_shape_keyed_clone(&ir, "untyped accumulator"); +} + +#[test] +fn a_class_typed_array_still_takes_the_class_arm() { + // #10123 must not steal the loops #7480 already owns: a declared element + // class still compares a class id and bakes a packed slot index. + let ir = emit(&element_shape_module( + vec![accumulate_stmt( + SUM_ID, + ARRAY_ID, + Expr::LocalGet(COUNTER_ID), + )], + None, + )); + assert_fast_clone_is_entered(&ir); + assert!( + ir.contains("call i32 @js_array_ensure_element_shape("), + "a declared element class must keep the class-keyed query" + ); + // CALLS, not declarations: both shape-keyed helpers are declared in every + // module by `runtime_decls`, so a bare substring search would be vacuous. + assert!( + !ir.contains("call i32 @js_array_ensure_element_shape_ordinary") + && !ir.contains("call i32 @js_shape_ordinary_inline_slot_for_key"), + "a declared element class must not pay the shape-keyed runtime queries" + ); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("402686207") && !fast.contains("134250751"), + "the class arm keeps the typed-layout conjunct in its residual mask" + ); +} + +// --------------------------------------------------------------------------- +// #10123 SABOTAGE — shapes the shape-keyed arm must decline. +// --------------------------------------------------------------------------- + +#[test] +fn a_body_mixing_index_forms_declines() { + // Each index form carries its OWN preheader bounds obligation, and the + // fact records exactly one. A body reading both `rows[i]` and `rows[7]` + // would have one of them discharged and the other not. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(untyped_elem_field(Expr::LocalGet(U_COUNTER_ID), "v")), + right: Box::new(untyped_elem_field(Expr::Integer(7), "w")), + })], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a body mixing index forms must decline the clone" + ); +} + +#[test] +fn a_modulo_index_with_the_operands_swapped_declines() { + // `const index = n % i` is not a bounded index at all — it is bounded by + // the COUNTER, which grows. Only `counter % modulus` is admitted. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![ + Stmt::Let { + id: U_INDEX_ID, + name: "index".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::Binary { + op: BinaryOp::Mod, + left: Box::new(Expr::LocalGet(MOD_ID)), + right: Box::new(Expr::LocalGet(U_COUNTER_ID)), + }), + }, + untyped_accumulate(untyped_elem_field(Expr::LocalGet(U_INDEX_ID), "v")), + ], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "`modulus % counter` must decline the clone" + ); +} + +#[test] +fn an_untracked_local_index_declines() { + // A local that is neither the counter nor the body's own derived binding + // has no range the preheader proved anything about. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(MOD_ID), + "v", + ))], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "an arbitrary local index must decline the clone" + ); +} + +#[test] +fn a_denylisted_property_declines_the_shape_keyed_arm() { + // `length`, `name`, `constructor`, … are answered by the runtime or by the + // prototype, not out of an inline slot, so a shape's key position for one + // would be the wrong answer even when it exists. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "length", + ))], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a denylisted property must decline the clone" + ); +} + +#[test] +fn a_declared_but_unresolvable_element_type_still_declines() { + // The shape-keyed arm is entered only by a receiver that declares NO + // element type. `Widget[]` where `Widget` names no module class is a + // declined CLASS resolution, not an untyped receiver: the annotation is a + // layout claim this arm would be second-guessing. + let mut m = untyped_param_module( + Type::Array(Box::new(Type::Named("Widget".to_string()))), + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "v", + ))], + ); + m.classes = Vec::new(); + let ir = emit(&m); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "an unresolvable declared element type must decline both arms" + ); +} diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index f9bdad3bc5..b9fe5ba0e1 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -48,6 +48,46 @@ pub(crate) fn lower_let( { return Ok(()); } + // #10123: the derived-index twin. Inside a shape-keyed element-shape fast + // clone, `const d = j % m` is not lowered generically — `%` on two + // possibly-untyped operands is a runtime call, and a call inside the clone + // DELETES it (#7690) rather than slowing it. The preheader already proved + // `m` is an integral `1..=i32::MAX` and materialized it as an i32, and the + // counter is a non-negative i32, so the whole statement is one `srem`. + // + // Sound for the same four reasons the element binding is: nothing reads + // `d` bare inside the clone (matcher), the clone is call-free so no GC + // observes the slot mid-loop, `const` scoping means nothing after the loop + // can read it, and a residual-check side exit re-runs the current + // iteration in the slow clone, whose OWN `Let` binds the real slot before + // any use. The fact is popped before the slow clone lowers, so this arm + // cannot fire there. + if let Some((counter_slot, modulus_i32, derived_slot)) = + ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { + let crate::expr::ElementShapeIndex::DerivedMod { + local_id, + modulus_i32, + slot, + } = &fact.index + else { + return None; + }; + if *local_id != id { + return None; + } + Some(( + ctx.i32_counter_slots.get(&fact.index_local_id)?.clone(), + modulus_i32.clone(), + slot.clone(), + )) + }) + { + let blk = ctx.block(); + let counter = blk.load(I32, &counter_slot); + let derived = blk.srem(I32, &counter, &modulus_i32); + blk.store(I32, &derived, &derived_slot); + return Ok(()); + } // `let C = SomeClass` aliases the local `C` to the class // `SomeClass` for `new C()` site rerouting. The HIR lowers // class identifiers referenced as values to `Expr::ClassRef`, diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index 03dd401291..e3677034e2 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -375,12 +375,17 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { return true; } // repsel #7480 step 3: inside an element-shape fast clone a tracked - // `arr[i].field` read is a GUARD-PROVEN raw double — the preheader - // pinned the element class and the per-element residual check - // requires `GC_OBJ_TYPED_LAYOUT_INTACT`, so the slot cannot hold a - // NaN-boxed value. This is a stronger proof than the declared-type - // answer below, and it is the ONLY one available for an - // object-literal element type, whose owner class + // `arr[i].field` read is a GUARD-PROVEN raw double. The class-keyed + // arm gets that from the residual check's + // `GC_OBJ_TYPED_LAYOUT_INTACT` conjunct, which says the slot holds + // a raw `double` rather than a NaN-boxed value; #10123's + // shape-keyed arm gets it from a Number-tag test on the loaded word + // that side-exits to the slow clone when it fails + // (`expr::element_shape_guard::emit_element_shape_field_load`). + // Either way the value this predicate licenses a consumer to treat + // as an f64 has been proven to be one. This is a stronger proof + // than the declared-type answer below, and it is the ONLY one + // available for an object-literal element type, whose owner class // `receiver_class_name` deliberately does not resolve. // // It is also load-bearing rather than a bonus: without it diff --git a/test-files/test_gap_json_record_loop_clone.ts b/test-files/test_gap_json_record_loop_clone.ts new file mode 100644 index 0000000000..0a60a97359 --- /dev/null +++ b/test-files/test_gap_json_record_loop_clone.ts @@ -0,0 +1,272 @@ +// #10123: the SHAPE-keyed element-shape versioned loop clone. +// +// #7480's clone keyed every proof on a compile-time class, so it could never +// fire for the one array shape record-processing code is actually written +// against: `JSON.parse`'d objects, which are class 0 with an ordinary birth +// ShapeId. The runtime now proves the exact ShapeId instead, and the clone's +// preheader resolves each tracked property's inline slot against it. +// +// Three things are new and every one of them is a MISCOMPILE if it is wrong, +// not a slow path, so each gets cases here rather than only a codegen unit +// test: +// +// * the per-element residual no longer requires `GC_OBJ_TYPED_LAYOUT_INTACT` +// (a parsed record never has it), so the loaded word is a NaN-boxed +// JSValue and is tag-tested as a Number per read; +// * `rows[7]` and `const d = i % n; rows[d]` are admitted as indices, each +// with its own preheader bounds obligation; +// * the array head is repaired BEFORE the `GC_TYPE_ARRAY` brand, so a lazy +// JSON array is materialized instead of rejected. + +function buildRecords(count: number, pad: string): string { + const parts: string[] = []; + for (let i = 0; i < count; i++) { + parts.push( + '{"id":' + i + ',"name":"user_' + i + '","active":' + + (i % 2 === 0 ? "false" : "true") + ',"score":' + (i * 1.5) + + ',"note":"' + pad + '"}', + ); + } + return "[" + parts.join(",") + "]"; +} + +// `repeat`: a CONSTANT index. The trip count says nothing about index 7, so +// the preheader owes its own `length > 7`. +function repeatSum(rows: any, count: number): number { + let sum = 0; + for (let i = 0; i < count; i++) sum += rows[7].id; + return sum; +} + +// `sequential`: the derived `i % n` index, lowered to one `srem` in the clone. +function sequentialSum(rows: any, count: number, n: number): number { + let sum = 0; + for (let i = 0; i < count; i++) { + const index = i % n; + sum += rows[index].id; + } + return sum; +} + +// The original counter-indexed shape, now over an untyped receiver. +function scanSum(rows: any, count: number): number { + let sum = 0; + for (let i = 0; i < count; i++) sum += rows[i].id; + return sum; +} + +// `+` on a possibly-non-numeric field: JavaScript switches to string +// concatenation the moment one `id` is a string, which is exactly what the +// clone's per-read Number tag test has to preserve. +function concatIds(rows: any, count: number): string { + let out = ""; + for (let i = 0; i < count; i++) out += String(rows[i].id) + ","; + return out; +} + +// --------------------------------------------------------------------------- +// 1. A LAZY parsed array (a top-level array over 1 KB is returned as a lazy +// header, which the preheader's brand test rejected before the repair was +// moved ahead of it). +// --------------------------------------------------------------------------- +const lazyText = buildRecords(64, "0123456789abcdef0123456789abcdef"); +console.log("lazy-bytes-over-1k:", lazyText.length > 1024); +const lazy: any = JSON.parse(lazyText); +const lazyLength: number = lazy.length; + +console.log("lazy-repeat:", repeatSum(lazy, 50)); +console.log("lazy-repeat-again:", repeatSum(lazy, 50)); +console.log("lazy-sequential:", sequentialSum(lazy, 200, lazyLength)); +console.log("lazy-scan:", scanSum(lazy, lazyLength)); +console.log("lazy-scan-again:", scanSum(lazy, lazyLength)); + +// --------------------------------------------------------------------------- +// 2. An EAGER parsed array (under 1 KB), the same loops. +// --------------------------------------------------------------------------- +const eagerText = buildRecords(6, "x"); +console.log("eager-bytes-under-1k:", eagerText.length < 1024); +const eager: any = JSON.parse(eagerText); +console.log("eager-repeat-index-in-range:", eager.length > 7); +console.log("eager-sequential:", sequentialSum(eager, 20, eager.length)); +console.log("eager-scan:", scanSum(eager, eager.length)); + +// --------------------------------------------------------------------------- +// 3. HETEROGENEOUS key sets. One record with an extra key has a different +// ShapeId, so the array-level proof must decline for the WHOLE array — +// a shape says which slot holds `id`, and the second shape's may differ. +// --------------------------------------------------------------------------- +const heteroText = + '[{"id":1,"name":"a"},{"id":2,"name":"b"},{"extra":9,"id":3,"name":"c"},' + + '{"id":4,"name":"d"}]'; +const hetero: any = JSON.parse(heteroText); +console.log("hetero-scan:", scanSum(hetero, hetero.length)); +console.log("hetero-sequential:", sequentialSum(hetero, 12, hetero.length)); + +// A key set that is the same NAMES in a different ORDER is still a different +// shape, and its `id` sits at a different slot. +const reorderedText = + '[{"id":1,"name":"a"},{"name":"b","id":2},{"id":3,"name":"c"}]'; +const reordered: any = JSON.parse(reorderedText); +console.log("reordered-scan:", scanSum(reordered, reordered.length)); + +// --------------------------------------------------------------------------- +// 4. NON-NUMERIC field values. The array is perfectly homogeneous in SHAPE — +// every record has exactly `{id, name}` — so the array-level proof holds +// and only the per-read Number tag test stands between the clone and +// reading a string pointer's bits as a double. +// --------------------------------------------------------------------------- +const stringIdText = + '[{"id":1,"name":"a"},{"id":2,"name":"b"},{"id":"three","name":"c"},' + + '{"id":4,"name":"d"}]'; +const stringId: any = JSON.parse(stringIdText); +console.log("string-id-sum:", scanSum(stringId, stringId.length)); +console.log("string-id-concat:", concatIds(stringId, stringId.length)); + +const nullIdText = '[{"id":1},{"id":null},{"id":3}]'; +const nullId: any = JSON.parse(nullIdText); +console.log("null-id-sum:", scanSum(nullId, nullId.length)); + +const boolIdText = '[{"id":1},{"id":true},{"id":3}]'; +const boolId: any = JSON.parse(boolIdText); +console.log("bool-id-sum:", scanSum(boolId, boolId.length)); + +const objIdText = '[{"id":1},{"id":{"n":2}},{"id":3}]'; +const objId: any = JSON.parse(objIdText); +console.log("obj-id-concat:", concatIds(objId, objId.length)); + +// Fractional and negative values still read as plain doubles. +const floatText = '[{"id":-1.5},{"id":0.25},{"id":1e21}]'; +const floats: any = JSON.parse(floatText); +console.log("float-ids:", scanSum(floats, floats.length)); +console.log("float-concat:", concatIds(floats, floats.length)); + +// --------------------------------------------------------------------------- +// 5. REVOCATION after the proof was established. Each of these leaves the +// array's length alone, so only the element-store funnel or the per-element +// residual can catch it. +// --------------------------------------------------------------------------- +const mutated: any = JSON.parse(buildRecords(40, "pad")); +console.log("mutated-before:", scanSum(mutated, mutated.length)); +mutated[5] = 123; +console.log("mutated-after-primitive:", scanSum(mutated, mutated.length)); + +const reshaped: any = JSON.parse(buildRecords(40, "pad")); +console.log("reshaped-before:", scanSum(reshaped, reshaped.length)); +delete reshaped[9].name; +console.log("reshaped-after-delete:", scanSum(reshaped, reshaped.length)); + +const downgraded: any = JSON.parse(buildRecords(40, "pad")); +console.log("downgraded-before:", scanSum(downgraded, downgraded.length)); +downgraded[11].id = "eleven"; +console.log("downgraded-after:", concatIds(downgraded, 14)); + +const accessorised: any = JSON.parse(buildRecords(40, "pad")); +Object.defineProperty(accessorised[3], "id", { + get() { + return 99; + }, + configurable: true, +}); +console.log("own-accessor:", scanSum(accessorised, accessorised.length)); + +// Length changes retire the proof through the pinned `verified_len`. +const grown: any = JSON.parse(buildRecords(40, "pad")); +console.log("grown-before:", scanSum(grown, grown.length)); +grown.push({ id: 1000, name: "extra", active: true, score: 0, note: "pad" }); +console.log("grown-after:", scanSum(grown, grown.length)); +grown.pop(); +grown.length = 5; +console.log("grown-truncated:", scanSum(grown, grown.length)); + +// --------------------------------------------------------------------------- +// 6. BOUNDS. Each index form's obligation is discharged once in the preheader; +// a form whose obligation cannot be met must take the slow clone and +// observe ordinary JavaScript semantics. +// --------------------------------------------------------------------------- +const shortArr: any = JSON.parse('[{"id":1},{"id":2},{"id":3}]'); +// `rows[7]` on a 3-element array: `undefined.id` throws, exactly as JS says. +try { + console.log("short-repeat:", repeatSum(shortArr, 4)); +} catch (err) { + console.log("short-repeat-threw:", String(err).slice(0, 9)); +} +// A modulus LARGER than the array: the derived index runs past the end. +try { + console.log("modulus-past-length:", sequentialSum(shortArr, 8, 10)); +} catch (err) { + console.log("modulus-past-length-threw:", String(err).slice(0, 9)); +} +// `i % 0` is NaN in JavaScript, and `rows[NaN]` is `undefined` — the clone +// must never turn this into an `srem` by zero. +try { + console.log("modulus-zero:", sequentialSum(shortArr, 4, 0)); +} catch (err) { + console.log("modulus-zero-threw:", String(err).slice(0, 9)); +} +// A modulus SMALLER than the array is in range and stays specialized. +console.log("modulus-under-length:", sequentialSum(shortArr, 9, 2)); +// A trip count past the array's length with the counter index. +try { + console.log("scan-past-length:", scanSum(shortArr, 5)); +} catch (err) { + console.log("scan-past-length-threw:", String(err).slice(0, 9)); +} +// Empty array: the invariant declines a vacuous proof. +const emptyArr: any = JSON.parse("[]"); +console.log("empty-scan:", scanSum(emptyArr, emptyArr.length)); + +// --------------------------------------------------------------------------- +// 7. RECEIVERS THAT ARE NOT PLAIN PARSED ARRAYS. Each must decline the clone +// and still produce the right answer. +// --------------------------------------------------------------------------- +const literalRows: any = [ + { id: 1, name: "a" }, + { id: 2, name: "b" }, + { id: 3, name: "c" }, +]; +console.log("object-literal-scan:", scanSum(literalRows, literalRows.length)); + +class RowList extends Array {} +const subclass: any = new RowList(); +subclass.push({ id: 4, name: "d" }); +subclass.push({ id: 5, name: "e" }); +console.log("subclass-scan:", scanSum(subclass, subclass.length)); + +const nested: any = JSON.parse('{"rows":[{"id":1},{"id":2},{"id":3}]}'); +console.log("nested-scan:", scanSum(nested.rows, nested.rows.length)); + +const stringsArr: any = JSON.parse('["a","b","c"]'); +console.log("primitive-elements-concat:", concatIds(stringsArr, 3)); + +const nullElems: any = JSON.parse('[{"id":1},null,{"id":3}]'); +try { + console.log("null-element:", scanSum(nullElems, nullElems.length)); +} catch (err) { + console.log("null-element-threw:", String(err).slice(0, 9)); +} + +// --------------------------------------------------------------------------- +// 8. A MODULE-LEVEL parsed array read from inside a function — the +// module-global arm of the preheader's repaired-head write-back. +// --------------------------------------------------------------------------- +const moduleRows: any = JSON.parse(buildRecords(48, "0123456789abcdef")); +const moduleLength: number = moduleRows.length; + +function sumModuleRows(count: number): number { + let sum = 0; + for (let i = 0; i < count; i++) { + const index = i % moduleLength; + sum += moduleRows[index].score; + } + return sum; +} +console.log("module-global:", sumModuleRows(120)); +console.log("module-global-again:", sumModuleRows(120)); + +// A second field on the same records, so the preheader resolves two slots. +function sumTwoFields(rows: any, count: number): number { + let sum = 0; + for (let i = 0; i < count; i++) sum += rows[i].id + rows[i].score; + return sum; +} +console.log("two-fields:", sumTwoFields(moduleRows, moduleLength)); From f603e0dba2313ee013b4202891b32b168ee29a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 09:10:37 +0200 Subject: [PATCH 3/7] fix(codegen): drop the trip-count obligation for a non-counter index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shape-keyed clone (#10123) was entered by a `cond_br` the IR census could see and never once at run time on its own benchmark. The preheader still emitted the counter arm's `length >= bound`, so `for (i = 0; i < 1000000; i++) sum += rows[7].id` over a 7,600-element array asked it to prove `length >= 1000000` — false — and a million iterations ran the slow clone. Measured: 5.73 ns/iter before, 5.43 after, with the whole optimization inert. The counter is not an index in the `Constant` and `DerivedMod` forms, so the verified prefix has nothing to say about the trip count; each form already carries its own obligation (`length > k`, `m <= length`). The `arr.length` trip-count arm keeps its i32-fits check for every index form, because the emitted trip test is signed whatever the index is. Both shape-keyed index tests now assert the ABSENCE of the counter arm's comparison, which is the assertion that would have caught this: the derived case pins the count at exactly one `icmp uge`, and the counter case asserts the obligation is still there, so neither can drift into the other. `element_shape_loop_tests.rs` crosses the 2000-line cap with those cases, so the shape-keyed half moves to `element_shape_shape_keyed_tests.rs` — a CHILD module, because every helper it uses is private to the parent and duplicating an IR census is how two of them drift apart. `let_stmt.rs` crossed the cap too; its two virtual-binding arms (#7771's element binding, #10123's derived index) are now one call into `element_shape_loop::lower_virtual_clone_binding`, which is where their soundness arguments belong anyway. --- .../src/expr/element_shape_guard.rs | 61 ++- .../src/stmt/element_shape_loop.rs | 66 +++ .../src/stmt/element_shape_loop_tests.rs | 440 +--------------- .../stmt/element_shape_shape_keyed_tests.rs | 491 ++++++++++++++++++ crates/perry-codegen/src/stmt/let_stmt.rs | 64 +-- 5 files changed, 610 insertions(+), 512 deletions(-) create mode 100644 crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs diff --git a/crates/perry-codegen/src/expr/element_shape_guard.rs b/crates/perry-codegen/src/expr/element_shape_guard.rs index 5d22777dbf..d2ec6ce903 100644 --- a/crates/perry-codegen/src/expr/element_shape_guard.rs +++ b/crates/perry-codegen/src/expr/element_shape_guard.rs @@ -443,34 +443,51 @@ pub(crate) fn emit_element_shape_loop_preheader_check( // `ArrayHeader { length: u32 @0, capacity: u32 @4 }`. The invariant's // query requires `verified_len == length`, and nothing has run since, so - // `length >= bound` is exactly "the verified prefix covers every index the - // loop reads". The matcher already pinned `start >= 0`. + // the comparisons below are exactly "the verified prefix covers every + // index the loop reads". The matcher already pinned `start >= 0`. let len_ptr = blk.inttoptr(I64, &handle1); let length = blk.load(I32, &len_ptr); - let (bound_i32, len_ok) = match trip_count { - ElementShapeLoopTripCount::Bound(bound) => { - (bound.to_string(), blk.icmp_uge(I32, &length, bound)) + let bound_i32 = match trip_count { + ElementShapeLoopTripCount::Bound(bound) => bound.to_string(), + ElementShapeLoopTripCount::ArrayLength => length.clone(), + }; + + // The TRIP-COUNT obligation, which exists only when the counter is also + // the index. + // + // #10123: this used to be unconditional, and that made the whole + // shape-keyed arm dead on its own benchmark. `for (i = 0; i < 1000000; + // i++) sum += rows[7].id` over a 7,600-element array asks the preheader to + // prove `length >= 1000000`, which is false — so the clone was emitted, + // was branched into by a `cond_br` the IR census could see, and was never + // once entered at run time. The counter is not an index here; the verified + // prefix has nothing to say about the trip count, and `ElementShapeIndex` + // carries its own obligation instead. + let trip_ok = match (trip_count, index_bound) { + (ElementShapeLoopTripCount::Bound(bound), ElementShapeIndexBound::FromTripCount) => { + Some(blk.icmp_uge(I32, &length, bound)) } + // A caller-materialized bound is already a non-negative i32 + // (`materialize_loop_i32`), and the counter indexes nothing. + (ElementShapeLoopTripCount::Bound(_), _) => None, // #7480 step 4: `for (j = 0; j < arr.length; j++)` — the trip count IS // the length this block just read, so "the verified prefix covers every // index" is true by construction and there is nothing to compare it - // against. What still has to be proven is that the u32 fits a - // non-negative i32: the clone's counter is an i32 and the emitted trip - // test is signed, so a length above `i32::MAX` would read as negative - // and run zero iterations while the slow clone ran billions. No such - // array is allocatable today (it would need 32 GB of element slots), - // which is exactly why the check is one `icmp` rather than a comment. - ElementShapeLoopTripCount::ArrayLength => { - let fits = blk.icmp_sgt(I32, &length, "-1"); - (length.clone(), fits) - } + // against. What still has to be proven, for EVERY index form, is that + // the u32 fits a non-negative i32: the clone's counter is an i32 and + // the emitted trip test is signed, so a length above `i32::MAX` would + // read as negative and run zero iterations while the slow clone ran + // billions. No such array is allocatable today (it would need 32 GB of + // element slots), which is exactly why the check is one `icmp` rather + // than a comment. + (ElementShapeLoopTripCount::ArrayLength, _) => Some(blk.icmp_sgt(I32, &length, "-1")), }; - // #10123: the index obligation. `FromTripCount` adds nothing — the trip - // count already covers every read — and the other two are compared - // UNSIGNED for the same reason `Bound` is: `length` is a `u32` read into - // an i32, so a hypothetical 2^31-element array must not read as negative - // and pass. + // #10123: the INDEX obligation, one per spelling. `FromTripCount` adds + // nothing — the trip-count test above already covers every read — and the + // other two are compared UNSIGNED for the same reason: `length` is a `u32` + // read into an i32, so a hypothetical 2^31-element array must not read as + // negative and pass. let index_ok = match index_bound { ElementShapeIndexBound::FromTripCount => None, ElementShapeIndexBound::Constant(k) => Some(blk.icmp_ugt(I32, &length, &k.to_string())), @@ -503,7 +520,9 @@ pub(crate) fn emit_element_shape_loop_preheader_check( let mut acc = blk.and(I1, &is_ptr1, &above1); acc = blk.and(I1, &acc, &is_array1); - acc = blk.and(I1, &acc, &len_ok); + if let Some(trip_ok) = trip_ok { + acc = blk.and(I1, &acc, &trip_ok); + } if let Some(index_ok) = index_ok { acc = blk.and(I1, &acc, &index_ok); } diff --git a/crates/perry-codegen/src/stmt/element_shape_loop.rs b/crates/perry-codegen/src/stmt/element_shape_loop.rs index 3f9f9a8d9b..1ce4f0cd8d 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop.rs @@ -590,6 +590,72 @@ fn anon_shape_field_type_is_compatible( } } +/// Lower one of the fast clone's VIRTUAL body bindings, or report that `id` is +/// not one. Called from `stmt/let_stmt.rs` before the generic `Let` lowering. +/// +/// Two shapes, both admitted by the matcher and neither lowered generically: +/// +/// * **#7771's element binding** (`const r = arr[j]`) emits NOTHING. The +/// matcher admitted the body only because every use of `r` is a tracked +/// `r.field` read, and each of those lowers through +/// `element_shape_loop_fact_for_property_get` to a bare element load. +/// Lowering the generic `IndexGet` would put a runtime-call diamond inside +/// the clone, fail its call-free admission scan, and DELETE the clone rather +/// than slow it (#7690's lesson). +/// * **#10123's derived index** (`const d = j % m`) emits one `srem i32`. `%` +/// on two possibly-untyped operands is a runtime call, with the same +/// consequence. The preheader already proved `m` is an integral +/// `1..=i32::MAX` and materialized it as an i32, and the counter is a +/// non-negative i32, so the whole statement is one instruction. +/// +/// Both are sound for the same four reasons: nothing reads the binding bare +/// inside the clone (the matcher's walk excludes both explicitly), the clone +/// is call-free so no GC observes the slot mid-loop, `const` scoping means +/// nothing after the loop can read it, and a residual-check side exit re-runs +/// the current iteration in the slow clone, whose OWN `Let` binds the real +/// slot before any use. The facts are popped before the slow clone lowers, so +/// this cannot fire there. +/// +/// Answering `false` for want of the counter's i32 slot cannot happen — the +/// matcher requires one for the derived-index form — and would only cost the +/// clone, never correctness. +pub(super) fn lower_virtual_clone_binding(ctx: &mut FnCtx<'_>, id: u32) -> bool { + if ctx + .element_shape_loop_facts + .iter() + .any(|fact| fact.element_binding == Some(id)) + { + return true; + } + let Some((counter_slot, modulus_i32, derived_slot)) = + ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { + let crate::expr::ElementShapeIndex::DerivedMod { + local_id, + modulus_i32, + slot, + } = &fact.index + else { + return None; + }; + if *local_id != id { + return None; + } + Some(( + ctx.i32_counter_slots.get(&fact.index_local_id)?.clone(), + modulus_i32.clone(), + slot.clone(), + )) + }) + else { + return false; + }; + let blk = ctx.block(); + let counter = blk.load(I32, &counter_slot); + let derived = blk.srem(I32, &counter, &modulus_i32); + blk.store(I32, &derived, &derived_slot); + true +} + /// Is `array_id` an untyped receiver — the shape-keyed arm's entry condition? /// /// The class resolver having declined is not on its own enough: a `Node[]` 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 1ca8524a06..c78cfc72e5 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -1617,435 +1617,11 @@ fn element_binding_form_through_a_parameter_gets_the_clone() { assert_clone_fires_call_free(&ir, "parameter binding form"); } -// --------------------------------------------------------------------------- -// #10123 — SHAPE-KEYED clones over an untyped (`any`) record array. -// -// This is the `JSON.parse` shape: `function run(rows: any, count: number)`, -// with the array's element identity known only at run time. Four things had to -// change together for it to fire, and every one of them is asserted below, -// because each alone still compiles and still prints the right answer: -// -// 1. the preheader asks `js_array_ensure_element_shape_ordinary` (the -// class-keyed query returns 0 for a class-0 record array, so a clone -// built on it would never be entered); -// 2. it resolves each tracked property to an inline slot with -// `js_shape_ordinary_inline_slot_for_key` (there is no class to bake a -// packed index from); -// 3. the residual per-element mask DROPS the typed-layout bit — a parsed -// record never has it, so the class-keyed mask would side-exit on the -// FIRST element of every loop while every label assertion still passed; -// 4. the loaded word is tag-tested as a Number, because a shape says which -// slot holds `id` and nothing about what is in it. -// --------------------------------------------------------------------------- - -const ROWS_ID: u32 = 31; -const COUNT_ID: u32 = 32; -const MOD_ID: u32 = 33; -const U_SUM_ID: u32 = 34; -const U_COUNTER_ID: u32 = 35; -const U_INDEX_ID: u32 = 36; - -/// `rows[].` -fn untyped_elem_field(index: Expr, prop: &str) -> Expr { - Expr::PropertyGet { - object: Box::new(Expr::IndexGet { - object: Box::new(Expr::LocalGet(ROWS_ID)), - index: Box::new(index), - }), - property: prop.to_string(), - byte_offset: 0, - } -} - -/// `sum = sum + ` -fn untyped_accumulate(value: Expr) -> Stmt { - Stmt::Expr(Expr::LocalSet( - U_SUM_ID, - Box::new(Expr::Binary { - op: BinaryOp::Add, - left: Box::new(Expr::LocalGet(U_SUM_ID)), - right: Box::new(value), - }), - )) -} - -/// `const index = i % n;` -fn derived_index_stmt(modulus: Expr) -> Stmt { - Stmt::Let { - id: U_INDEX_ID, - name: "index".to_string(), - ty: Type::Any, - mutable: false, - init: Some(Expr::Binary { - op: BinaryOp::Mod, - left: Box::new(Expr::LocalGet(U_COUNTER_ID)), - right: Box::new(modulus), - }), - } -} - -/// The benchmark's own function, parametrized: -/// -/// ```text -/// function run(rows: , count: number, n: number): number { -/// let sum: = 0; -/// for (let i = 0; i < count; i++) -/// return sum; -/// } -/// ``` -fn untyped_param_module(rows_ty: Type, sum_ty: Type, body: Vec) -> Module { - let mut m = Module::new("element_shape_loop.ts"); - m.functions = vec![perry_hir::Function { - id: 901, - name: "run".to_string(), - type_params: Vec::new(), - params: vec![ - perry_hir::Param { - id: ROWS_ID, - name: "rows".to_string(), - ty: rows_ty, - default: None, - decorators: Vec::new(), - is_rest: false, - arguments_object: None, - }, - perry_hir::Param { - id: COUNT_ID, - name: "count".to_string(), - ty: Type::Number, - default: None, - decorators: Vec::new(), - is_rest: false, - arguments_object: None, - }, - perry_hir::Param { - id: MOD_ID, - name: "n".to_string(), - ty: Type::Number, - default: None, - decorators: Vec::new(), - is_rest: false, - arguments_object: None, - }, - ], - return_type: Type::Number, - body: vec![ - Stmt::Let { - id: U_SUM_ID, - name: "sum".to_string(), - ty: sum_ty, - mutable: true, - init: Some(Expr::Integer(0)), - }, - Stmt::For { - init: Some(Box::new(Stmt::Let { - id: U_COUNTER_ID, - name: "i".to_string(), - ty: Type::Number, - mutable: true, - init: Some(Expr::Integer(0)), - })), - condition: Some(Expr::Compare { - op: CompareOp::Lt, - left: Box::new(Expr::LocalGet(U_COUNTER_ID)), - right: Box::new(Expr::LocalGet(COUNT_ID)), - }), - update: Some(Expr::Update { - id: U_COUNTER_ID, - op: UpdateOp::Increment, - prefix: false, - }), - body, - }, - Stmt::Return(Some(Expr::LocalGet(U_SUM_ID))), - ], - is_async: false, - is_generator: false, - is_strict: false, - is_exported: false, - captures: Vec::new(), - decorators: Vec::new(), - was_plain_async: false, - was_unrolled: false, - }]; - m.init_kind = ModuleInitKind::Eager; - m -} - -/// The four shape-keyed obligations, asserted together. -fn assert_shape_keyed_clone(ir: &str, what: &str) { - assert_clone_fires_call_free(ir, what); - assert!( - ir.contains("call i32 @js_array_ensure_element_shape_ordinary"), - "{what}: the preheader must ask the SHAPE query — the class-keyed one \ - answers 0 for a class-0 record array, so a clone guarded on it could \ - never be entered" - ); - assert!( - !ir.contains("call i32 @js_array_ensure_element_shape("), - "{what}: an untyped array has no class to compare against" - ); - assert!( - ir.contains("call i32 @js_shape_ordinary_inline_slot_for_key"), - "{what}: each tracked property's inline slot must be resolved once in \ - the preheader; there is no class to bake a packed index from" - ); - let fast = fast_clone_slice(ir); - assert!( - fast.contains("134250751"), - "{what}: the fast clone must use the shape-keyed residual mask \ - (0x0800_80FF); emitted:\n{fast}" - ); - assert!( - !fast.contains("402686207"), - "{what}: the class-keyed mask requires GC_OBJ_TYPED_LAYOUT_INTACT, \ - which a JSON record NEVER has — using it here would side-exit on the \ - first element of every loop while every label assertion still passed; \ - emitted:\n{fast}" - ); - assert!( - fast.contains("element_shape.number"), - "{what}: a shape says which slot holds the field, not what is in it — \ - the loaded word must be tag-tested as a Number before it is consumed \ - as a raw double; emitted:\n{fast}" - ); - assert!( - fast.contains("getelementptr double"), - "{what}: the read must still be a bare offset load" - ); -} - -#[test] -fn an_untyped_record_array_gets_a_shape_keyed_clone() { - let ir = emit(&untyped_param_module( - Type::Any, - Type::Number, - vec![untyped_accumulate(untyped_elem_field( - Expr::LocalGet(U_COUNTER_ID), - "v", - ))], - )); - assert_shape_keyed_clone(&ir, "counter-indexed untyped array"); -} - -#[test] -fn a_constant_index_gets_a_shape_keyed_clone_with_a_hoisted_bounds_check() { - // `for (let i = 0; i < count; i++) sum += rows[7].id` — the benchmark's - // `repeat` mode. The trip count says NOTHING about index 7, so the - // preheader owes its own `length > 7`, and the clone owes no per-read - // bounds test in exchange. - let ir = emit(&untyped_param_module( - Type::Any, - Type::Number, - vec![untyped_accumulate(untyped_elem_field( - Expr::Integer(7), - "v", - ))], - )); - assert_shape_keyed_clone(&ir, "constant index"); - let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); - assert!( - deref.contains("icmp ugt i32") && deref.contains(", 7"), - "the preheader must prove `length > 7` before the clone is reachable; \ - emitted:\n{deref}" - ); - let fast = fast_clone_slice(&ir); - assert!( - !fast.contains("icmp ult i32") && !fast.contains("icmp ugt i32"), - "the bounds obligation is discharged ONCE in the preheader; a per-read \ - test in the clone would mean it was not; emitted:\n{fast}" - ); -} - -#[test] -fn a_derived_modulo_index_gets_a_shape_keyed_clone_with_an_srem_in_the_body() { - // `const index = i % n; sum += rows[index].id` — the benchmark's - // `sequential` mode, and the shape every wrap-around pass over a record - // array is written in. The generic `%` lowering is a runtime call, which - // inside this clone would DELETE it (#7690), so the `Let` must become one - // `srem`. - let ir = emit(&untyped_param_module( - Type::Any, - Type::Number, - vec![ - derived_index_stmt(Expr::LocalGet(MOD_ID)), - untyped_accumulate(untyped_elem_field(Expr::LocalGet(U_INDEX_ID), "v")), - ], - )); - assert_shape_keyed_clone(&ir, "derived modulo index"); - let fast = fast_clone_slice(&ir); - assert!( - fast.contains("srem i32"), - "the derived index must be one `srem` inside the clone; emitted:\n{fast}" - ); - assert!( - ir.contains("element_shape.loop.modulus.range"), - "the modulus must be materialized as a validated i32 in the preheader \ - — `srem` by zero is UB and `x % 0` is NaN in JS" - ); - let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); - assert!( - deref.contains("icmp uge i32"), - "the preheader must prove `modulus <= length`, which is what makes \ - every derived index in bounds with no per-read test; emitted:\n{deref}" - ); -} - -#[test] -fn a_shape_keyed_clone_admits_an_untyped_accumulator() { - // `let sum = 0; sum += rows[i].id` widens `sum` to `Any` in HIR exactly - // when the element read is untyped — which is every program this clone - // exists for. The preheader's tag check on the accumulator is what makes - // the numeric fact sound, and it is emitted either way. - let ir = emit(&untyped_param_module( - Type::Any, - Type::Any, - vec![untyped_accumulate(untyped_elem_field( - Expr::LocalGet(U_COUNTER_ID), - "v", - ))], - )); - assert_shape_keyed_clone(&ir, "untyped accumulator"); -} - -#[test] -fn a_class_typed_array_still_takes_the_class_arm() { - // #10123 must not steal the loops #7480 already owns: a declared element - // class still compares a class id and bakes a packed slot index. - let ir = emit(&element_shape_module( - vec![accumulate_stmt( - SUM_ID, - ARRAY_ID, - Expr::LocalGet(COUNTER_ID), - )], - None, - )); - assert_fast_clone_is_entered(&ir); - assert!( - ir.contains("call i32 @js_array_ensure_element_shape("), - "a declared element class must keep the class-keyed query" - ); - // CALLS, not declarations: both shape-keyed helpers are declared in every - // module by `runtime_decls`, so a bare substring search would be vacuous. - assert!( - !ir.contains("call i32 @js_array_ensure_element_shape_ordinary") - && !ir.contains("call i32 @js_shape_ordinary_inline_slot_for_key"), - "a declared element class must not pay the shape-keyed runtime queries" - ); - let fast = fast_clone_slice(&ir); - assert!( - fast.contains("402686207") && !fast.contains("134250751"), - "the class arm keeps the typed-layout conjunct in its residual mask" - ); -} - -// --------------------------------------------------------------------------- -// #10123 SABOTAGE — shapes the shape-keyed arm must decline. -// --------------------------------------------------------------------------- - -#[test] -fn a_body_mixing_index_forms_declines() { - // Each index form carries its OWN preheader bounds obligation, and the - // fact records exactly one. A body reading both `rows[i]` and `rows[7]` - // would have one of them discharged and the other not. - let ir = emit(&untyped_param_module( - Type::Any, - Type::Number, - vec![untyped_accumulate(Expr::Binary { - op: BinaryOp::Add, - left: Box::new(untyped_elem_field(Expr::LocalGet(U_COUNTER_ID), "v")), - right: Box::new(untyped_elem_field(Expr::Integer(7), "w")), - })], - )); - assert!( - !ir.contains("element_shape.loop.fast.preheader"), - "a body mixing index forms must decline the clone" - ); -} - -#[test] -fn a_modulo_index_with_the_operands_swapped_declines() { - // `const index = n % i` is not a bounded index at all — it is bounded by - // the COUNTER, which grows. Only `counter % modulus` is admitted. - let ir = emit(&untyped_param_module( - Type::Any, - Type::Number, - vec![ - Stmt::Let { - id: U_INDEX_ID, - name: "index".to_string(), - ty: Type::Any, - mutable: false, - init: Some(Expr::Binary { - op: BinaryOp::Mod, - left: Box::new(Expr::LocalGet(MOD_ID)), - right: Box::new(Expr::LocalGet(U_COUNTER_ID)), - }), - }, - untyped_accumulate(untyped_elem_field(Expr::LocalGet(U_INDEX_ID), "v")), - ], - )); - assert!( - !ir.contains("element_shape.loop.fast.preheader"), - "`modulus % counter` must decline the clone" - ); -} - -#[test] -fn an_untracked_local_index_declines() { - // A local that is neither the counter nor the body's own derived binding - // has no range the preheader proved anything about. - let ir = emit(&untyped_param_module( - Type::Any, - Type::Number, - vec![untyped_accumulate(untyped_elem_field( - Expr::LocalGet(MOD_ID), - "v", - ))], - )); - assert!( - !ir.contains("element_shape.loop.fast.preheader"), - "an arbitrary local index must decline the clone" - ); -} - -#[test] -fn a_denylisted_property_declines_the_shape_keyed_arm() { - // `length`, `name`, `constructor`, … are answered by the runtime or by the - // prototype, not out of an inline slot, so a shape's key position for one - // would be the wrong answer even when it exists. - let ir = emit(&untyped_param_module( - Type::Any, - Type::Number, - vec![untyped_accumulate(untyped_elem_field( - Expr::LocalGet(U_COUNTER_ID), - "length", - ))], - )); - assert!( - !ir.contains("element_shape.loop.fast.preheader"), - "a denylisted property must decline the clone" - ); -} - -#[test] -fn a_declared_but_unresolvable_element_type_still_declines() { - // The shape-keyed arm is entered only by a receiver that declares NO - // element type. `Widget[]` where `Widget` names no module class is a - // declined CLASS resolution, not an untyped receiver: the annotation is a - // layout claim this arm would be second-guessing. - let mut m = untyped_param_module( - Type::Array(Box::new(Type::Named("Widget".to_string()))), - Type::Number, - vec![untyped_accumulate(untyped_elem_field( - Expr::LocalGet(U_COUNTER_ID), - "v", - ))], - ); - m.classes = Vec::new(); - let ir = emit(&m); - assert!( - !ir.contains("element_shape.loop.fast.preheader"), - "an unresolvable declared element type must decline both arms" - ); -} +/// #10123's shape-keyed cases, split out because this file crosses the repo's +/// 2000-line cap otherwise. A CHILD module rather than a sibling: every helper +/// above — `emit`, `block_slice`, `fast_clone_slice`, +/// `assert_clone_fires_call_free`, the class-arm module builders the +/// "still takes the class arm" case compares against — is private to this +/// module, and duplicating them is how two IR censuses drift apart. +#[path = "element_shape_shape_keyed_tests.rs"] +mod shape_keyed; diff --git a/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs b/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs new file mode 100644 index 0000000000..94ade02cc7 --- /dev/null +++ b/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs @@ -0,0 +1,491 @@ +//! #10123: the SHAPE-keyed element-shape versioned loop clone — the arm that +//! serves `JSON.parse`'d record arrays, where the element identity is a +//! runtime ShapeId rather than a compile-time class. +//! +//! A child module of `element_shape_loop_tests` (see the `mod` declaration +//! there), so `use super::*` brings that file's helpers and the class-arm +//! fixtures the negatives here compare against. + +use super::*; + +// --------------------------------------------------------------------------- +// #10123 — SHAPE-KEYED clones over an untyped (`any`) record array. +// +// This is the `JSON.parse` shape: `function run(rows: any, count: number)`, +// with the array's element identity known only at run time. Four things had to +// change together for it to fire, and every one of them is asserted below, +// because each alone still compiles and still prints the right answer: +// +// 1. the preheader asks `js_array_ensure_element_shape_ordinary` (the +// class-keyed query returns 0 for a class-0 record array, so a clone +// built on it would never be entered); +// 2. it resolves each tracked property to an inline slot with +// `js_shape_ordinary_inline_slot_for_key` (there is no class to bake a +// packed index from); +// 3. the residual per-element mask DROPS the typed-layout bit — a parsed +// record never has it, so the class-keyed mask would side-exit on the +// FIRST element of every loop while every label assertion still passed; +// 4. the loaded word is tag-tested as a Number, because a shape says which +// slot holds `id` and nothing about what is in it. +// --------------------------------------------------------------------------- + +const ROWS_ID: u32 = 31; +const COUNT_ID: u32 = 32; +const MOD_ID: u32 = 33; +const U_SUM_ID: u32 = 34; +const U_COUNTER_ID: u32 = 35; +const U_INDEX_ID: u32 = 36; + +/// `rows[].` +fn untyped_elem_field(index: Expr, prop: &str) -> Expr { + Expr::PropertyGet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ROWS_ID)), + index: Box::new(index), + }), + property: prop.to_string(), + byte_offset: 0, + } +} + +/// `sum = sum + ` +fn untyped_accumulate(value: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet( + U_SUM_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(U_SUM_ID)), + right: Box::new(value), + }), + )) +} + +/// `const index = i % n;` +fn derived_index_stmt(modulus: Expr) -> Stmt { + Stmt::Let { + id: U_INDEX_ID, + name: "index".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::Binary { + op: BinaryOp::Mod, + left: Box::new(Expr::LocalGet(U_COUNTER_ID)), + right: Box::new(modulus), + }), + } +} + +/// The benchmark's own function, parametrized: +/// +/// ```text +/// function run(rows: , count: number, n: number): number { +/// let sum: = 0; +/// for (let i = 0; i < count; i++) +/// return sum; +/// } +/// ``` +fn untyped_param_module(rows_ty: Type, sum_ty: Type, body: Vec) -> Module { + let mut m = Module::new("element_shape_loop.ts"); + m.functions = vec![perry_hir::Function { + id: 901, + name: "run".to_string(), + type_params: Vec::new(), + params: vec![ + perry_hir::Param { + id: ROWS_ID, + name: "rows".to_string(), + ty: rows_ty, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + perry_hir::Param { + id: COUNT_ID, + name: "count".to_string(), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + perry_hir::Param { + id: MOD_ID, + name: "n".to_string(), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }, + ], + return_type: Type::Number, + body: vec![ + Stmt::Let { + id: U_SUM_ID, + name: "sum".to_string(), + ty: sum_ty, + mutable: true, + init: Some(Expr::Integer(0)), + }, + Stmt::For { + init: Some(Box::new(Stmt::Let { + id: U_COUNTER_ID, + name: "i".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(0)), + })), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(U_COUNTER_ID)), + right: Box::new(Expr::LocalGet(COUNT_ID)), + }), + update: Some(Expr::Update { + id: U_COUNTER_ID, + op: UpdateOp::Increment, + prefix: false, + }), + body, + }, + Stmt::Return(Some(Expr::LocalGet(U_SUM_ID))), + ], + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }]; + m.init_kind = ModuleInitKind::Eager; + m +} + +/// The four shape-keyed obligations, asserted together. +fn assert_shape_keyed_clone(ir: &str, what: &str) { + assert_clone_fires_call_free(ir, what); + assert!( + ir.contains("call i32 @js_array_ensure_element_shape_ordinary"), + "{what}: the preheader must ask the SHAPE query — the class-keyed one \ + answers 0 for a class-0 record array, so a clone guarded on it could \ + never be entered" + ); + assert!( + !ir.contains("call i32 @js_array_ensure_element_shape("), + "{what}: an untyped array has no class to compare against" + ); + assert!( + ir.contains("call i32 @js_shape_ordinary_inline_slot_for_key"), + "{what}: each tracked property's inline slot must be resolved once in \ + the preheader; there is no class to bake a packed index from" + ); + let fast = fast_clone_slice(ir); + assert!( + fast.contains("134250751"), + "{what}: the fast clone must use the shape-keyed residual mask \ + (0x0800_80FF); emitted:\n{fast}" + ); + assert!( + !fast.contains("402686207"), + "{what}: the class-keyed mask requires GC_OBJ_TYPED_LAYOUT_INTACT, \ + which a JSON record NEVER has — using it here would side-exit on the \ + first element of every loop while every label assertion still passed; \ + emitted:\n{fast}" + ); + assert!( + fast.contains("element_shape.number"), + "{what}: a shape says which slot holds the field, not what is in it — \ + the loaded word must be tag-tested as a Number before it is consumed \ + as a raw double; emitted:\n{fast}" + ); + assert!( + fast.contains("getelementptr double"), + "{what}: the read must still be a bare offset load" + ); +} + +#[test] +fn an_untyped_record_array_gets_a_shape_keyed_clone() { + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "v", + ))], + )); + assert_shape_keyed_clone(&ir, "counter-indexed untyped array"); +} + +#[test] +fn a_constant_index_gets_a_shape_keyed_clone_with_a_hoisted_bounds_check() { + // `for (let i = 0; i < count; i++) sum += rows[7].id` — the benchmark's + // `repeat` mode. The trip count says NOTHING about index 7, so the + // preheader owes its own `length > 7`, and the clone owes no per-read + // bounds test in exchange. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::Integer(7), + "v", + ))], + )); + assert_shape_keyed_clone(&ir, "constant index"); + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert!( + deref.contains("icmp ugt i32") && deref.contains(", 7"), + "the preheader must prove `length > 7` before the clone is reachable; \ + emitted:\n{deref}" + ); + // THE LIVENESS ASSERTION. The trip-count obligation `length >= bound` is + // the counter-index arm's, and emitting it here made the whole clone + // UNENTERABLE for its own benchmark: `for (i = 0; i < 1000000; i++) sum += + // rows[7].id` over a 7,600-element array asks the preheader to prove + // `length >= 1000000`. The `cond_br` into the fast clone was emitted, every + // census assertion passed, and the loop ran the slow clone a million times. + // The counter indexes nothing here, so the verified prefix has nothing to + // say about the trip count. + assert!( + !deref.contains("icmp uge i32"), + "a constant index must NOT carry the counter arm's `length >= bound` \ + obligation — the counter indexes nothing, and requiring it makes the \ + clone unenterable whenever the loop runs more times than the array is \ + long; emitted:\n{deref}" + ); + let fast = fast_clone_slice(&ir); + assert!( + !fast.contains("icmp ult i32") && !fast.contains("icmp ugt i32"), + "the bounds obligation is discharged ONCE in the preheader; a per-read \ + test in the clone would mean it was not; emitted:\n{fast}" + ); +} + +/// The counter-index arm must KEEP the obligation the test above forbids: it +/// is the only thing that makes `arr[j]` in bounds for every `j < bound`. +#[test] +fn a_counter_index_keeps_the_trip_count_obligation() { + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "v", + ))], + )); + assert_shape_keyed_clone(&ir, "counter index"); + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert!( + deref.contains("icmp uge i32"), + "`arr[j]` is in bounds only because the verified prefix covers the \ + whole trip count; emitted:\n{deref}" + ); +} + +#[test] +fn a_derived_modulo_index_gets_a_shape_keyed_clone_with_an_srem_in_the_body() { + // `const index = i % n; sum += rows[index].id` — the benchmark's + // `sequential` mode, and the shape every wrap-around pass over a record + // array is written in. The generic `%` lowering is a runtime call, which + // inside this clone would DELETE it (#7690), so the `Let` must become one + // `srem`. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![ + derived_index_stmt(Expr::LocalGet(MOD_ID)), + untyped_accumulate(untyped_elem_field(Expr::LocalGet(U_INDEX_ID), "v")), + ], + )); + assert_shape_keyed_clone(&ir, "derived modulo index"); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("srem i32"), + "the derived index must be one `srem` inside the clone; emitted:\n{fast}" + ); + assert!( + ir.contains("element_shape.loop.modulus.range"), + "the modulus must be materialized as a validated i32 in the preheader \ + — `srem` by zero is UB and `x % 0` is NaN in JS" + ); + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert!( + deref.contains("icmp uge i32"), + "the preheader must prove `modulus <= length`, which is what makes \ + every derived index in bounds with no per-read test; emitted:\n{deref}" + ); + // Same liveness assertion as the constant-index case: the ONE `icmp uge` + // in this block must be the modulus obligation, not the counter arm's + // `length >= bound`. `for (i = 0; i < 1000000; i++) { const d = i % n; … }` + // over a short array is the benchmark's `sequential` mode, and demanding + // `length >= 1000000` made the clone unenterable there. + assert_eq!( + deref.matches("icmp uge i32").count(), + 1, + "a derived index owes exactly ONE length comparison (`modulus <= \ + length`); a second one is the counter arm's trip-count obligation, \ + which makes the clone unenterable whenever the loop runs more times \ + than the array is long; emitted:\n{deref}" + ); +} + +#[test] +fn a_shape_keyed_clone_admits_an_untyped_accumulator() { + // `let sum = 0; sum += rows[i].id` widens `sum` to `Any` in HIR exactly + // when the element read is untyped — which is every program this clone + // exists for. The preheader's tag check on the accumulator is what makes + // the numeric fact sound, and it is emitted either way. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Any, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "v", + ))], + )); + assert_shape_keyed_clone(&ir, "untyped accumulator"); +} + +#[test] +fn a_class_typed_array_still_takes_the_class_arm() { + // #10123 must not steal the loops #7480 already owns: a declared element + // class still compares a class id and bakes a packed slot index. + let ir = emit(&element_shape_module( + vec![accumulate_stmt( + SUM_ID, + ARRAY_ID, + Expr::LocalGet(COUNTER_ID), + )], + None, + )); + assert_fast_clone_is_entered(&ir); + assert!( + ir.contains("call i32 @js_array_ensure_element_shape("), + "a declared element class must keep the class-keyed query" + ); + // CALLS, not declarations: both shape-keyed helpers are declared in every + // module by `runtime_decls`, so a bare substring search would be vacuous. + assert!( + !ir.contains("call i32 @js_array_ensure_element_shape_ordinary") + && !ir.contains("call i32 @js_shape_ordinary_inline_slot_for_key"), + "a declared element class must not pay the shape-keyed runtime queries" + ); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("402686207") && !fast.contains("134250751"), + "the class arm keeps the typed-layout conjunct in its residual mask" + ); +} + +// --------------------------------------------------------------------------- +// #10123 SABOTAGE — shapes the shape-keyed arm must decline. +// --------------------------------------------------------------------------- + +#[test] +fn a_body_mixing_index_forms_declines() { + // Each index form carries its OWN preheader bounds obligation, and the + // fact records exactly one. A body reading both `rows[i]` and `rows[7]` + // would have one of them discharged and the other not. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(untyped_elem_field(Expr::LocalGet(U_COUNTER_ID), "v")), + right: Box::new(untyped_elem_field(Expr::Integer(7), "w")), + })], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a body mixing index forms must decline the clone" + ); +} + +#[test] +fn a_modulo_index_with_the_operands_swapped_declines() { + // `const index = n % i` is not a bounded index at all — it is bounded by + // the COUNTER, which grows. Only `counter % modulus` is admitted. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![ + Stmt::Let { + id: U_INDEX_ID, + name: "index".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::Binary { + op: BinaryOp::Mod, + left: Box::new(Expr::LocalGet(MOD_ID)), + right: Box::new(Expr::LocalGet(U_COUNTER_ID)), + }), + }, + untyped_accumulate(untyped_elem_field(Expr::LocalGet(U_INDEX_ID), "v")), + ], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "`modulus % counter` must decline the clone" + ); +} + +#[test] +fn an_untracked_local_index_declines() { + // A local that is neither the counter nor the body's own derived binding + // has no range the preheader proved anything about. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(MOD_ID), + "v", + ))], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "an arbitrary local index must decline the clone" + ); +} + +#[test] +fn a_denylisted_property_declines_the_shape_keyed_arm() { + // `length`, `name`, `constructor`, … are answered by the runtime or by the + // prototype, not out of an inline slot, so a shape's key position for one + // would be the wrong answer even when it exists. + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "length", + ))], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a denylisted property must decline the clone" + ); +} + +#[test] +fn a_declared_but_unresolvable_element_type_still_declines() { + // The shape-keyed arm is entered only by a receiver that declares NO + // element type. `Widget[]` where `Widget` names no module class is a + // declined CLASS resolution, not an untyped receiver: the annotation is a + // layout claim this arm would be second-guessing. + let mut m = untyped_param_module( + Type::Array(Box::new(Type::Named("Widget".to_string()))), + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + "v", + ))], + ); + m.classes = Vec::new(); + let ir = emit(&m); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "an unresolvable declared element type must decline both arms" + ); +} diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index b9fe5ba0e1..28ec377674 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -27,65 +27,11 @@ pub(crate) fn lower_let( ty: &perry_hir::types::Type, mutable: bool, ) -> Result<()> { - // #7771: inside an element-shape fast clone, the tracked - // `const r = arr[j]` binding is VIRTUAL. The matcher admitted the body - // only because every use of `r` is a tracked `r.field` read, and each of - // those lowers through `element_shape_loop_fact_for_property_get` to a - // bare element load — so the binding itself emits nothing. Lowering the - // generic `IndexGet` here would put a runtime-call diamond inside the - // clone, fail its call-free admission scan, and DELETE the clone rather - // than slow it (#7690's lesson). Skipping is sound: nothing reads `r` - // bare inside the clone (matcher), the clone is call-free so no GC - // observes the slot mid-loop, `const` scoping means nothing after the - // loop can read it, and a residual-check side exit re-runs the current - // iteration in the slow clone, whose OWN `Let` binds the slot before any - // use. The fact is popped before the slow clone lowers, so this arm - // cannot fire there. - if ctx - .element_shape_loop_facts - .iter() - .any(|fact| fact.element_binding == Some(id)) - { - return Ok(()); - } - // #10123: the derived-index twin. Inside a shape-keyed element-shape fast - // clone, `const d = j % m` is not lowered generically — `%` on two - // possibly-untyped operands is a runtime call, and a call inside the clone - // DELETES it (#7690) rather than slowing it. The preheader already proved - // `m` is an integral `1..=i32::MAX` and materialized it as an i32, and the - // counter is a non-negative i32, so the whole statement is one `srem`. - // - // Sound for the same four reasons the element binding is: nothing reads - // `d` bare inside the clone (matcher), the clone is call-free so no GC - // observes the slot mid-loop, `const` scoping means nothing after the loop - // can read it, and a residual-check side exit re-runs the current - // iteration in the slow clone, whose OWN `Let` binds the real slot before - // any use. The fact is popped before the slow clone lowers, so this arm - // cannot fire there. - if let Some((counter_slot, modulus_i32, derived_slot)) = - ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { - let crate::expr::ElementShapeIndex::DerivedMod { - local_id, - modulus_i32, - slot, - } = &fact.index - else { - return None; - }; - if *local_id != id { - return None; - } - Some(( - ctx.i32_counter_slots.get(&fact.index_local_id)?.clone(), - modulus_i32.clone(), - slot.clone(), - )) - }) - { - let blk = ctx.block(); - let counter = blk.load(I32, &counter_slot); - let derived = blk.srem(I32, &counter, &modulus_i32); - blk.store(I32, &derived, &derived_slot); + // #7771 / #10123: inside an element-shape fast clone the body's `const` + // binding is VIRTUAL — the element binding emits nothing and the derived + // index emits one `srem`. Both live with the clone, which is where their + // soundness arguments and the preheader that validated them are. + if super::element_shape_loop::lower_virtual_clone_binding(ctx, id) { return Ok(()); } // `let C = SomeClass` aliases the local `C` to the class From c865ad05dff8953fc58787c979a2306ce8c6cb92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 09:13:19 +0200 Subject: [PATCH 4/7] docs(changelog): record the measured json-record loop clone numbers --- changelog.d/10123-json-record-loop-clone.md | 27 +++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/changelog.d/10123-json-record-loop-clone.md b/changelog.d/10123-json-record-loop-clone.md index 0b04945888..399aafa09d 100644 --- a/changelog.d/10123-json-record-loop-clone.md +++ b/changelog.d/10123-json-record-loop-clone.md @@ -51,13 +51,26 @@ is still the whole admission test, enforced by the matcher and by the post-emission scan of every block the clone owns. Measured with `benchmarks/json_performance/.work/fixtures/records_array_*.json` -and a `rows: any` access worker, best of five, ns per iteration: +and a `rows: any` access worker, five interleaved rounds, best of five, on one +compiler binary per arm. Checksums agree across all four engines. | cell | before | after | node 26.5.1 | bun 1.3.14 | |---|---|---|---|---| -| 16k repeat | 5.73 | BEFORE_AFTER | 3.06 | 3.63 | -| 16k sequential | 12.27 | BEFORE_AFTER | 4.39 | 4.29 | -| 1m repeat | 5.74 | BEFORE_AFTER | 3.49 | 4.66 | -| 1m sequential | 15.80 | BEFORE_AFTER | 9.51 | 6.76 | -| 20m repeat | 5.24 | BEFORE_AFTER | 3.59 | 5.17 | -| 20m sequential | 17.65 | BEFORE_AFTER | 7.22 | 8.37 | +| 16k repeat | 5.98 | **4.29** | 3.21 | 3.55 | +| 16k sequential | 12.78 | **4.29** | 4.68 | 4.68 | +| 1m repeat | 5.96 | **4.29** | 4.03 | 4.12 | +| 1m sequential | 16.43 | **4.32** | 9.97 | 10.72 | +| 20m repeat | 5.45 | **4.29** | 3.58 | 5.01 | +| 20m sequential | 18.30 | **4.56** | 7.85 | 7.72 | + +(ns per iteration.) Instructions retired per iteration, the same loops minus a +zero-iteration run: repeat 116 -> 16, sequential 166-184 -> 24-25. The +wall-clock win is smaller than the instruction win because a 16-instruction +body is latency-bound on the element load, not instruction-bound. + +No-regression cells (`benchmarks/json_performance/worker.ts`, five interleaved +rounds): `heterogeneous_1m parse` -2.2%, `records_array_1m parse` +0.2%, +`records_array_1m sparse` +0.2%, and `records_array_16k scan` **-30.1%** — the +counter-indexed loop inside that cell is the same clone, over an array the +preheader now materializes in bulk instead of leaving lazy for per-element +reads. From 5d685a2e210012d6c174c19ead9a6ff0ff610602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 09:14:52 +0200 Subject: [PATCH 5/7] docs(codegen): note why the trip-count obligation is index-form-specific --- crates/perry-codegen/src/expr/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index aa6f68fb4e..a0df121336 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2104,6 +2104,12 @@ pub(crate) struct ClassFieldLoopFact { /// says nothing about, so each carries its OWN preheader obligation (see /// `expr::element_shape_guard::ElementShapeIndexBound`) and the fast clone /// still pays no per-read bounds test. +/// +/// The counter arm's obligation is also DROPPED for the other two, and that is +/// not an optimization: leaving it in made the whole shape-keyed arm dead on +/// its own benchmark, because `for (i = 0; i < 1000000; i++) sum += +/// rows[7].id` over a 7,600-element array asks it to prove `length >= +/// 1000000`. See the `trip_ok` comment in the guard emitter. #[derive(Debug, Clone)] pub(crate) enum ElementShapeIndex { /// `arr[j]` — the counter, read from its canonical i32 slot. From 540fc36f55209d451f8f4c9f19216daf3376a272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 09:36:54 +0200 Subject: [PATCH 6/7] docs(changelog): key the fragment on its PR number --- ...-json-record-loop-clone.md => 10171-json-record-loop-clone.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10123-json-record-loop-clone.md => 10171-json-record-loop-clone.md} (100%) diff --git a/changelog.d/10123-json-record-loop-clone.md b/changelog.d/10171-json-record-loop-clone.md similarity index 100% rename from changelog.d/10123-json-record-loop-clone.md rename to changelog.d/10171-json-record-loop-clone.md From 2b77e7d4fee0373e018fa9714f443111298d73cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 10:25:04 +0200 Subject: [PATCH 7/7] lint(codegen): classify the shape-keyed clone's untyped-receiver hint read The local binding type-proof audit requires every local_type_hint read to carry a classification. array_is_untyped reads the declared type only to decline the shape-keyed element-shape loop clone for a receiver with any layout claim; admission still goes through the preheader's runtime proof and the per-element residual check. --- scripts/local_binding_type_allowlist.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/local_binding_type_allowlist.json b/scripts/local_binding_type_allowlist.json index 2fe8a83da7..616fb82258 100644 --- a/scripts/local_binding_type_allowlist.json +++ b/scripts/local_binding_type_allowlist.json @@ -664,6 +664,14 @@ "count": 1, "classification": "metadata-only", "reason": "This indexed scope-stack iteration performs lexical name resolution during HIR construction and does not establish a codegen runtime representation." + }, + { + "path": "crates/perry-codegen/src/stmt/element_shape_loop.rs", + "function": "array_is_untyped", + "access": "local_type_hint", + "count": 1, + "classification": "runtime-validated", + "reason": "The declared type is read only to DECLINE the shape-keyed loop clone for a receiver that carries any layout claim (a named class or an anon-shape the resolver found ambiguous); it never selects a layout. A receiver admitted here (no annotation, any, unknown) still goes through the preheader's runtime proof \u2014 js_array_ensure_element_shape_ordinary must return a nonzero ShapeId and js_shape_ordinary_inline_slot_for_key a slot for every tracked key \u2014 and the per-element residual check re-validates that ShapeId on every read, so a stale or wrong hint can only cost the clone, never a wrong offset." } ] }