From c9ce44b1b2d45d39ff6417a4d68bc3f4add37768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 12:34:02 +0200 Subject: [PATCH 1/4] perf(codegen): extend the element-shape loop clone to the fields and random access shapes Three additions on top of #10171's shape-keyed arm, all needed together for the JSON access benchmark's two remaining shapes: - a LOOP-CARRIED index (`c = (a*c + b) % m; ... rows[c]`), folded to one affine pair and evaluated as `srem i64`, with the write-back placed at the END of the iteration so a mid-iteration side exit cannot double-apply the recurrence; - K accumulator statements folded into one, so the whole iteration commits once, past every side exit it can take; - `arr[i].prop.length` on a string field and `arr[i].prop ? A : B` on a boolean one, each tag-testing the loaded word and side-exiting otherwise. Plus a shared once-per-iteration element deref/residual check, hung off the body's leading virtual binding, so three reads of one element pay one check. --- crates/perry-codegen/src/expr/binary.rs | 11 + .../src/expr/element_shape_guard.rs | 199 ++++-- .../src/expr/element_shape_reads.rs | 326 +++++++++ crates/perry-codegen/src/expr/mod.rs | 97 ++- .../src/expr/property_get/helpers.rs | 31 +- crates/perry-codegen/src/expr/shadow_slot.rs | 24 +- .../src/stmt/element_shape_carried.rs | 416 +++++++++++ .../stmt/element_shape_fields_random_tests.rs | 650 ++++++++++++++++++ .../src/stmt/element_shape_loop.rs | 613 ++++++++++++++--- .../src/stmt/element_shape_loop_tests.rs | 22 +- .../stmt/element_shape_shape_keyed_tests.rs | 75 +- crates/perry-codegen/src/stmt/let_stmt.rs | 2 +- crates/perry-codegen/src/stmt/mod.rs | 10 + .../src/type_analysis/numeric.rs | 23 + 14 files changed, 2297 insertions(+), 202 deletions(-) create mode 100644 crates/perry-codegen/src/expr/element_shape_reads.rs create mode 100644 crates/perry-codegen/src/stmt/element_shape_carried.rs create mode 100644 crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index 04be6b3799..ce24554dc7 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -504,6 +504,17 @@ fn reduction_add_is_reassociable(ctx: &FnCtx<'_>, left: &Expr, right: &Expr) -> } fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String, bool)> { + // #10199: the element-shape fast clone's two NON-numeric reads — + // `arr[i].name.length` and `arr[i].active ? 1 : 0`. Both are numbers by the + // time they get here, each proven from the loaded word's own NaN-box tag + // with a side exit to the slow clone on a miss, so the `true` suppresses + // the residual `js_number_coerce` below: that call would not slow the clone + // down, it would delete it. Outside a clone the fact vector is empty and + // this is a no-op. See `expr::element_shape_reads`. + if let Some(value) = crate::expr::element_shape_reads::try_lower_cloned_read_operand(ctx, expr)? + { + return Ok((value, true)); + } // A stable-packed numeric clone has a stronger fact than the generic // untyped-local typed-array probe below: its preheader scanned the exact // range, and its emitted-IR gate proved that no call can invalidate that diff --git a/crates/perry-codegen/src/expr/element_shape_guard.rs b/crates/perry-codegen/src/expr/element_shape_guard.rs index d2ec6ce903..7b199c3951 100644 --- a/crates/perry-codegen/src/expr/element_shape_guard.rs +++ b/crates/perry-codegen/src/expr/element_shape_guard.rs @@ -538,13 +538,137 @@ pub(crate) fn emit_element_shape_loop_preheader_check( }) } -/// 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. +/// Materialize this iteration's element index as an i32, or report that the +/// clone cannot index at all. /// -/// `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( +/// One arm per [`super::ElementShapeIndex`] spelling, and each reads exactly +/// the storage whose bounds obligation the preheader discharged +/// ([`ElementShapeIndexBound`]). `None` means the counter's canonical i32 slot +/// the matcher required has gone missing — that costs the read its fast +/// lowering, never correctness, and the matcher, the fact lookup and this must +/// all agree (`ElementShapeIndex::needs_counter_i32_slot`). +pub(crate) fn emit_element_shape_index( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, +) -> Option { + match &fact.index { + super::ElementShapeIndex::Counter => { + let slot = ctx.i32_counter_slots.get(&fact.index_local_id).cloned()?; + Some(ctx.block().load(I32, &slot)) + } + super::ElementShapeIndex::Constant(k) => Some(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. + super::ElementShapeIndex::DerivedMod { slot, .. } => { + let slot = slot.clone(); + Some(ctx.block().load(I32, &slot)) + } + // #10199: the carried recurrence's private i32 slot, written by the + // body's first statement earlier in this same iteration. The REAL + // binding slot is deliberately NOT read here — it is one iteration + // behind until the trailing write-back commits, which is exactly what + // makes a mid-iteration side exit correct. + super::ElementShapeIndex::Carried(carried) => { + let slot = carried.slot.clone(); + Some(ctx.block().load(I32, &slot)) + } + } +} + +/// The masked heap handle of `arr[idx]` for THIS iteration, with the residual +/// per-element facts discharged. +/// +/// Two sources. When the matcher installed an [`super::ElementPrefetch`] the +/// body's leading virtual binding already did the deref and the residual check +/// for the whole iteration, so this is one load of an entry alloca; otherwise +/// (#10123's original shape) the deref and the check are emitted here, once per +/// read. +pub(crate) fn emit_element_shape_element_handle( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, + idx_i32: &str, +) -> String { + if let Some(prefetch) = &fact.elem_prefetch { + let slot = prefetch.handle_slot.clone(); + return ctx.block().load(I64, &slot); + } + emit_element_deref_with_residual(ctx, fact, idx_i32) +} + +/// Bare element load plus the residual per-OBJECT facts the array-level +/// invariant deliberately does not cover. Returns the masked handle. +/// +/// Emits nothing but loads, ALU and one `cond_br` — call-free by construction, +/// which is the whole revocation argument (see the module docs). +pub(crate) fn emit_element_deref_with_residual( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, + idx_i32: &str, +) -> String { + let blk = ctx.block(); + // The element-shape invariant proved every slot in the verified prefix + // 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)]); + let elem_bits = blk.load(I64, &slot_ptr); + let elem_handle = blk.and(I64, &elem_bits, crate::nanbox::POINTER_MASK_I64); + + if fact.statically_layout_proven { + return elem_handle; + } + let elem_ptr = blk.inttoptr(I64, &elem_handle); + let load_idx = ctx.new_block("element_shape.load"); + let load_label = ctx.block_label(load_idx); + let blk = ctx.block(); + + // Residual per-OBJECT facts (see the module docs for why they 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 (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")]); + let shape_id = blk.load(I32, &sid_ptr); + let shape_ok = blk.icmp_eq(I32, &shape_id, &fact.expected_shape_id); + + let ok = blk.and(I1, &hdr_ok, &shape_ok); + // The side exit resumes the CURRENT iteration in the slow clone; no effect + // of this iteration has committed yet (#10199: with a prefetch this is the + // ONLY residual exit in the iteration, and it precedes every store). + blk.cond_br(&ok, &load_label, &fact.side_exit_label); + ctx.current_block = load_idx; + elem_handle +} + +/// #10199: the once-per-iteration element prologue. +/// +/// Emitted by the body's leading virtual binding (`stmt/let_stmt.rs` → +/// `stmt/element_shape_loop::lower_virtual_clone_binding`) once the index for +/// this iteration is in its slot. Parks the masked handle in an entry alloca so +/// every read in the iteration is a bare offset load. +pub(crate) fn emit_element_shape_prefetch( + ctx: &mut FnCtx, + fact: &super::ElementShapeLoopFact, + idx_i32: &str, + handle_slot: &str, +) { + let handle = emit_element_deref_with_residual(ctx, fact, idx_i32); + ctx.block().store(I64, &handle, handle_slot); +} + +/// One tracked property's RAW slot word — a NaN-boxed `JSValue` in the +/// shape-keyed arm, a raw `double` in the class-keyed one. No tag test; the +/// caller adds the one its consumer needs. +pub(crate) fn emit_element_shape_slot_load( ctx: &mut FnCtx, fact: &super::ElementShapeLoopFact, idx_i32: &str, @@ -555,54 +679,27 @@ pub(crate) fn emit_element_shape_field_load( 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 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)]); - let elem_bits = blk.load(I64, &slot_ptr); - let elem_handle = blk.and(I64, &elem_bits, crate::nanbox::POINTER_MASK_I64); - let elem_ptr = blk.inttoptr(I64, &elem_handle); - - if !fact.statically_layout_proven { - let load_idx = ctx.new_block("element_shape.load"); - let load_label = ctx.block_label(load_idx); - let blk = ctx.block(); - - // Residual per-OBJECT facts (see the module docs for why they - // 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 (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")]); - let shape_id = blk.load(I32, &sid_ptr); - let shape_ok = blk.icmp_eq(I32, &shape_id, &fact.expected_shape_id); - - let ok = blk.and(I1, &hdr_ok, &shape_ok); - // One branch per access. The side exit resumes the CURRENT - // iteration in the slow clone; no effect has committed yet. - blk.cond_br(&ok, &load_label, &fact.side_exit_label); - ctx.current_block = load_idx; - } - elem_ptr - }; - + let elem_handle = emit_element_shape_element_handle(ctx, fact, idx_i32); let blk = ctx.block(); + let elem_ptr = blk.inttoptr(I64, &elem_handle); let fields_base = blk.gep(I8, &elem_ptr, &[(I64, &header_skip)]); let field_ptr = blk.gep(DOUBLE, &fields_base, &[(slot_ty, &slot_value)]); - let value = blk.load(DOUBLE, &field_ptr); + blk.load(DOUBLE, &field_ptr) +} + +/// 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 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_slot: &super::ElementShapeFieldSlot, +) -> String { + let value = emit_element_shape_slot_load(ctx, fact, idx_i32, field_slot); // #10123: the shape-keyed arm's representation check. // diff --git a/crates/perry-codegen/src/expr/element_shape_reads.rs b/crates/perry-codegen/src/expr/element_shape_reads.rs new file mode 100644 index 0000000000..ce207ae373 --- /dev/null +++ b/crates/perry-codegen/src/expr/element_shape_reads.rs @@ -0,0 +1,326 @@ +//! #10199: the two NON-numeric reads the element-shape fast clone admits — +//! `arr[i].prop.length` on a string field and `arr[i].prop ? A : B` on a +//! boolean one. +//! +//! ## Why they need their own lowering at all +//! +//! Both are ordinary JavaScript that the generic lowering handles perfectly +//! well — with a call. `.length` on an untyped receiver reaches the property +//! diamond and, on a miss, `js_object_get_field_by_name_f64`; a ternary on an +//! untyped condition reaches `js_is_truthy`. Inside this clone a call is not a +//! slow path, it is a DELETED clone (`contains_gc_unsafe_call`, #7690), so the +//! benchmark's `fields` shape — +//! +//! ```text +//! sum += rows[index].id; +//! sum += rows[index].name.length; +//! sum += rows[index].active ? 1 : 0; +//! ``` +//! +//! got no clone at all: one unadmitted read in the body costs the whole loop. +//! +//! ## What each one proves before it reads +//! +//! The element-shape preheader proves WHICH inline slot holds `name` and +//! `active`; it proves nothing about what is in them. So each read tag-tests +//! the loaded word and side-exits to the slow clone when the answer is not the +//! representation it is about to assume — exactly the discipline #10123 +//! established for the Number case. A `name` that is a number, a `null` or an +//! object, and an `active` that is anything but `true`/`false`, are re-run in +//! the slow clone, where full JS semantics apply. +//! +//! **`.length`.** JS `.length` is UTF-16 code units. A heap string keeps that +//! count in `StringHeader::utf16_len`, the leading `u32` — the identical load +//! the inline `.length` fast path in `property_get/generic_dispatch.rs` emits. +//! An SSO immediate (`SHORT_STRING_TAG`, up to five bytes packed into the +//! NaN-box) keeps its BYTE length in bits 40..=47, and this reads it the same +//! way the runtime's own SSO `.length` arms do +//! (`string/char_ops.rs::string_property_get_miss`, +//! `property_get/generic_dispatch.rs`). That convention is byte length, not +//! code-unit length, so a non-ASCII SSO string reports its UTF-8 size — a +//! PRE-EXISTING Perry-wide answer, reproduced here deliberately: the clone must +//! agree with the path it is a clone of, and diverging from it would be a +//! miscompile even where the shared answer is itself wrong. +//! +//! **The ternary.** JS truthiness of an arbitrary value is a runtime question +//! (`""`, `0`, `NaN`, `null`, every object). The clone does not guess it: only +//! the two boolean singletons are admitted, by exact NaN-box bit pattern, and +//! everything else side-exits. The result is one `select` between two +//! compile-time constants. + +use anyhow::Result; +use perry_hir::Expr; + +use super::FnCtx; +use crate::types::{DOUBLE, I1, I32, I64}; + +/// `STRING_TAG >> 48` — a heap `StringHeader` pointer in the low 48 bits. +const STRING_TAG_TOP16: &str = crate::nanbox::STRING_TAG_TOP16_I64; +/// `SHORT_STRING_TAG >> 48` — an SSO immediate. +const SHORT_STRING_TAG_TOP16: &str = crate::nanbox::SHORT_STRING_TAG_TOP16_I64; +/// `SHORT_STRING_LEN_SHIFT` — the length byte sits at bits 40..=47. +const SHORT_STRING_LEN_SHIFT: &str = "40"; + +/// `TAG_TRUE` (`0x7FFC_0000_0000_0004`) as a decimal i64 literal. +const TAG_TRUE_I64: &str = "9222246136947933188"; +/// `TAG_FALSE` (`0x7FFC_0000_0000_0003`) as a decimal i64 literal. +const TAG_FALSE_I64: &str = "9222246136947933187"; + +/// Is `expr` a tracked element read the fast clone has a fact for? +/// +/// Only meaningful INSIDE the clone — outside one the fact vector is empty and +/// every answer is `None`. +fn tracked_element_read(ctx: &FnCtx<'_>, expr: &Expr) -> bool { + let Expr::PropertyGet { + object, property, .. + } = expr + else { + return false; + }; + super::element_shape_loop_fact_for_property_get(ctx, object, property).is_some() +} + +/// `.length` — returns the inner read. +pub(crate) fn cloned_string_length_read<'e>(ctx: &FnCtx<'_>, expr: &'e Expr) -> Option<&'e Expr> { + let Expr::PropertyGet { + object, property, .. + } = expr + else { + return None; + }; + if property != "length" { + return None; + } + tracked_element_read(ctx, object.as_ref()).then(|| object.as_ref()) +} + +/// ` ? : ` — returns the +/// condition's inner read and the two constants. +pub(crate) fn cloned_bool_select_read<'e>( + ctx: &FnCtx<'_>, + expr: &'e Expr, +) -> Option<(&'e Expr, i64, i64)> { + let Expr::Conditional { + condition, + then_expr, + else_expr, + } = expr + else { + return None; + }; + if !tracked_element_read(ctx, condition.as_ref()) { + return None; + } + Some(( + condition.as_ref(), + integer_arm(then_expr)?, + integer_arm(else_expr)?, + )) +} + +/// A ternary arm the clone may bake in: an integer-valued literal. +/// +/// Deliberately narrow. A non-constant arm would have to be lowered on both +/// sides of a branch inside a clone whose whole admission argument is that its +/// body is one straight line of pure reads. +pub(crate) fn integer_arm(expr: &Expr) -> Option { + match expr { + Expr::Integer(k) => Some(*k), + Expr::Number(n) if n.is_finite() && n.fract() == 0.0 && *n >= -1e15 && *n <= 1e15 => { + Some(*n as i64) + } + _ => None, + } +} + +/// Resolve one tracked read to (fact, slot, index) and emit the raw slot word. +fn emit_tracked_slot_word(ctx: &mut FnCtx<'_>, read: &Expr) -> Option { + let Expr::PropertyGet { + object, property, .. + } = read + else { + return None; + }; + let (fact, slot) = super::element_shape_loop_fact_for_property_get(ctx, object, property) + .map(|(fact, slot)| (fact.clone(), slot.clone()))?; + // A class-keyed clone's slot holds a RAW double, not a NaN-boxed value — + // there is no tag to test and these two readers would be reading a + // `double`'s bit pattern as a box. Both forms are shape-keyed-only. + if !fact.shape_keyed { + return None; + } + let idx_i32 = super::element_shape_guard::emit_element_shape_index(ctx, &fact)?; + Some(super::element_shape_guard::emit_element_shape_slot_load( + ctx, &fact, &idx_i32, &slot, + )) +} + +/// The side exit the tracked read's own fact names. +fn side_exit_label(ctx: &FnCtx<'_>, read: &Expr) -> Option { + let Expr::PropertyGet { + object, property, .. + } = read + else { + return None; + }; + super::element_shape_loop_fact_for_property_get(ctx, object, property) + .map(|(fact, _)| fact.side_exit_label.clone()) +} + +/// Emit `.length` as an f64. +pub(crate) fn lower_cloned_string_length( + ctx: &mut FnCtx<'_>, + read: &Expr, +) -> Result> { + let Some(exit) = side_exit_label(ctx, read) else { + return Ok(None); + }; + let Some(value) = emit_tracked_slot_word(ctx, read) else { + return Ok(None); + }; + + let bits = ctx.block().bitcast_double_to_i64(&value); + let top16 = ctx.block().lshr(I64, &bits, "48"); + let is_heap = ctx.block().icmp_eq(I64, &top16, STRING_TAG_TOP16); + let is_sso = ctx.block().icmp_eq(I64, &top16, SHORT_STRING_TAG_TOP16); + let is_string = ctx.block().or(I1, &is_heap, &is_sso); + + let dispatch_idx = ctx.new_block("element_shape.strlen"); + let heap_idx = ctx.new_block("element_shape.strlen.heap"); + let sso_idx = ctx.new_block("element_shape.strlen.sso"); + let done_idx = ctx.new_block("element_shape.strlen.done"); + let dispatch_label = ctx.block_label(dispatch_idx); + let heap_label = ctx.block_label(heap_idx); + let sso_label = ctx.block_label(sso_idx); + let done_label = ctx.block_label(done_idx); + + // A non-string `name` re-runs the whole iteration in the slow clone rather + // than being decoded as one. + ctx.block().cond_br(&is_string, &dispatch_label, &exit); + + ctx.current_block = dispatch_idx; + ctx.block().cond_br(&is_heap, &heap_label, &sso_label); + + // `StringHeader::utf16_len` is the leading `u32`; `safe_load_i32_from_ptr` + // keeps a sub-page handle off the load, and is itself call-free (an `icmp`, + // a `select` onto a zero-valued global, and the load). + ctx.current_block = heap_idx; + let heap_handle = ctx.block().and(I64, &bits, crate::nanbox::POINTER_MASK_I64); + let heap_len_i32 = ctx.block().safe_load_i32_from_ptr(&heap_handle); + let heap_len = ctx.block().uitofp(I32, &heap_len_i32, DOUBLE); + let heap_end = ctx.block().label.clone(); + ctx.block().br(&done_label); + + ctx.current_block = sso_idx; + let sso_shifted = ctx.block().lshr(I64, &bits, SHORT_STRING_LEN_SHIFT); + let sso_len_byte = ctx.block().and(I64, &sso_shifted, "255"); + let sso_len = ctx.block().uitofp(I64, &sso_len_byte, DOUBLE); + let sso_end = ctx.block().label.clone(); + ctx.block().br(&done_label); + + ctx.current_block = done_idx; + let length = ctx + .block() + .phi(DOUBLE, &[(&heap_len, &heap_end), (&sso_len, &sso_end)]); + Ok(Some(length)) +} + +/// Emit ` ? A : B` as an f64 select over the two constants. +pub(crate) fn lower_cloned_bool_select( + ctx: &mut FnCtx<'_>, + read: &Expr, + on_true: i64, + on_false: i64, +) -> Result> { + let Some(exit) = side_exit_label(ctx, read) else { + return Ok(None); + }; + let Some(value) = emit_tracked_slot_word(ctx, read) else { + return Ok(None); + }; + + let bits = ctx.block().bitcast_double_to_i64(&value); + let is_true = ctx.block().icmp_eq(I64, &bits, TAG_TRUE_I64); + let is_false = ctx.block().icmp_eq(I64, &bits, TAG_FALSE_I64); + let is_bool = ctx.block().or(I1, &is_true, &is_false); + + let ok_idx = ctx.new_block("element_shape.bool"); + let ok_label = ctx.block_label(ok_idx); + // Everything that is not one of the two boolean singletons — `0`, `""`, + // `null`, an object — has a JS truthiness the clone is not allowed to + // guess, so it takes the side exit. + ctx.block().cond_br(&is_bool, &ok_label, &exit); + + ctx.current_block = ok_idx; + let then_literal = format!("{:.1}", on_true as f64); + let else_literal = format!("{:.1}", on_false as f64); + Ok(Some(ctx.block().select( + I1, + &is_true, + DOUBLE, + &then_literal, + &else_literal, + ))) +} + +/// The arithmetic-operand entry point (`expr::binary::lower_arithmetic_operand`). +/// +/// Returns the lowered f64, already representation-proven, so the caller must +/// NOT append a `js_number_coerce` — that call would delete the clone. +pub(crate) fn try_lower_cloned_read_operand( + ctx: &mut FnCtx<'_>, + expr: &Expr, +) -> Result> { + if let Some(read) = cloned_string_length_read(ctx, expr) { + let read = read.clone(); + return lower_cloned_string_length(ctx, &read); + } + if let Some((read, on_true, on_false)) = cloned_bool_select_read(ctx, expr) { + let read = read.clone(); + return lower_cloned_bool_select(ctx, &read, on_true, on_false); + } + Ok(None) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Anti-drift gate, the same one `element_shape_guard`'s constants carry: + /// every literal above reproduces a `perry-runtime::value::tags` constant, + /// and `perry-codegen` cannot share the definition. + #[test] + fn boolean_singleton_literals_match_the_runtime() { + assert_eq!(TAG_TRUE_I64, (0x7FFC_0000_0000_0004u64 as i64).to_string()); + assert_eq!(TAG_FALSE_I64, (0x7FFC_0000_0000_0003u64 as i64).to_string()); + // Neither may collide with any other singleton in the 0x7FFC namespace + // — a clone that read `undefined`/`null`/a hole as `false` would be a + // miscompile, not a slow path. + for other in [ + 0x7FFC_0000_0000_0001u64, // TAG_UNDEFINED + 0x7FFC_0000_0000_0002, // TAG_NULL + 0x7FFC_0000_0000_0010, // TAG_HOLE + 0x7FFC_0000_0000_0011, // TAG_TDZ + ] { + assert_ne!(TAG_TRUE_I64, (other as i64).to_string()); + assert_ne!(TAG_FALSE_I64, (other as i64).to_string()); + } + } + + #[test] + fn string_tag_literals_match_the_runtime() { + assert_eq!(STRING_TAG_TOP16, (0x7FFFu64).to_string()); + assert_eq!(SHORT_STRING_TAG_TOP16, (0x7FF9u64).to_string()); + assert_eq!(SHORT_STRING_LEN_SHIFT, "40"); + } + + #[test] + fn only_integral_ternary_arms_are_admitted() { + assert_eq!(integer_arm(&Expr::Integer(1)), Some(1)); + assert_eq!(integer_arm(&Expr::Number(0.0)), Some(0)); + assert_eq!(integer_arm(&Expr::Number(1.5)), None); + assert_eq!(integer_arm(&Expr::Number(f64::NAN)), None); + assert_eq!(integer_arm(&Expr::Number(f64::INFINITY)), None); + assert_eq!(integer_arm(&Expr::LocalGet(3)), None); + } +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index a0df121336..6f45f0795e 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2134,6 +2134,66 @@ pub(crate) enum ElementShapeIndex { /// Entry-block i32 alloca the clone writes the derived index to. slot: String, }, + /// #10199: a LOOP-CARRIED index — `let c = …;` outside the loop, + /// `c = (a*c + b) % m;` as the body's first statement, then `arr[c]` (or + /// `const index = c; arr[index]`). The benchmark's `random` mode, and the + /// shape every pseudo-random walk over a record array is written in. + /// + /// Unlike [`Self::DerivedMod`]'s `d`, `c` is a real mutable binding that is + /// LIVE AFTER THE LOOP, so the clone owes a write-back. It pays it once per + /// iteration, at the END of the iteration (`Carried::commit_slot`), which is + /// also what makes the residual side exit correct: every exit from the + /// middle of an iteration leaves the real slot holding the value it had at + /// that iteration's ENTRY, so the slow clone re-runs the update exactly + /// once. A write-back at the update site would double-apply the recurrence. + Carried(Box), +} + +/// #10199: everything [`ElementShapeIndex::Carried`] needs, boxed to keep the +/// enum small. +#[derive(Debug, Clone)] +pub(crate) struct CarriedIndex { + /// The `let` binding the recurrence advances. + pub local_id: u32, + /// `const index = c` — an optional alias the body may spell the subscript + /// through. Also virtual: its `Let` emits nothing. + pub alias_id: Option, + /// `c' = (a*c + b) % m`, folded to one affine pair by the matcher. Both + /// non-negative, and `|a| * i32::MAX + |b|` proven below 2^53 so the i64 + /// evaluation agrees with the f64 one JavaScript performs. + pub coeff_a: i64, + pub coeff_b: i64, + /// i32 SSA value of the modulus, materialized in the preheader. + pub modulus_i32: String, + /// Entry-block i32 alloca holding the live carried value inside the clone. + pub slot: String, + /// The binding's REAL (NaN-boxed double) slot, written once per iteration + /// after every side exit for that iteration has been passed. + pub commit_slot: String, +} + +/// #10199: the per-iteration element prefetch. +/// +/// `rows[index].id + rows[index].name.length + (rows[index].active ? 1 : 0)` +/// reads ONE element three times. Without this each read repeats the element +/// load AND the residual header/ShapeId check, which is three branches and +/// three redundant loads per iteration. The prologue attached to the body's +/// leading virtual binding does both once and parks the masked element handle +/// in an entry alloca; every read is then a bare offset load plus the tag test +/// its own consumer needs. +/// +/// It is also what makes a MULTI-READ body's side exit correct: every residual +/// exit now happens in the prologue, before any accumulator store has +/// committed, so resuming the iteration in the slow clone cannot double-apply +/// an earlier read. +#[derive(Debug, Clone)] +pub(crate) struct ElementPrefetch { + /// The virtual binding whose lowering emits the prologue. Exactly one + /// statement owns it, so a body with both a carried update and an alias + /// emits it once. + pub site_local_id: u32, + /// Entry-block i64 alloca holding the masked element handle. + pub handle_slot: String, } impl ElementShapeIndex { @@ -2152,7 +2212,22 @@ impl ElementShapeIndex { /// 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(_)) + !matches!( + self, + ElementShapeIndex::Constant(_) | ElementShapeIndex::Carried(_) + ) + } + + /// #10199: the locals whose `Let` / `LocalSet` the fast clone lowers + /// VIRTUALLY for this index form. Nothing may read one of them bare inside + /// the clone, and their shadow slots must not be cleared there. + pub(crate) fn virtual_locals(&self) -> impl Iterator + '_ { + let (a, b) = match self { + ElementShapeIndex::DerivedMod { local_id, .. } => (Some(*local_id), None), + ElementShapeIndex::Carried(carried) => (Some(carried.local_id), carried.alias_id), + _ => (None, None), + }; + a.into_iter().chain(b) } } @@ -2200,6 +2275,10 @@ pub(crate) struct ElementShapeLoopFact { pub index_local_id: u32, /// #10123: how the clone computes the element index. pub index: ElementShapeIndex, + /// #10199: the shared per-iteration element deref, when the body has a + /// leading virtual binding to hang it off. `None` keeps #10123's per-read + /// deref + residual check. + pub elem_prefetch: Option, /// #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 @@ -2229,6 +2308,15 @@ pub(crate) struct ElementShapeLoopFact { /// raw-f64 candidates; shape-keyed ones are the preheader's live query /// results (#10123). pub fields: std::collections::BTreeMap, + /// #10199: true when the fast clone lowers a body the matcher SYNTHESIZED + /// (the K-statement accumulator fold, or the carried index's trailing + /// write-back) rather than the source body. The function-wide + /// shadow-slot-clear map is keyed by statement INDEX, so a rewritten body + /// would attribute a clear to the wrong statement; the clone is call-free, + /// so no collection can observe a slot inside it and the clears are simply + /// suppressed there (`expr::emit_shadow_slot_clear`). The slow clone lowers + /// the original body, with its original indices and its original clears. + pub synthesized_body: bool, /// #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 @@ -2304,6 +2392,12 @@ pub(crate) fn element_shape_loop_fact_for_property_get<'f>( (ElementShapeIndex::DerivedMod { local_id, .. }, Expr::LocalGet(id)) => { *id == *local_id } + // #10199: `arr[c]` and `const index = c; arr[index]` are + // the same subscript — the alias is virtual, so both + // spellings read the carried slot the preheader bounded. + (ElementShapeIndex::Carried(carried), Expr::LocalGet(id)) => { + *id == carried.local_id || carried.alias_id == Some(*id) + } _ => false, }; if !spelled { @@ -2923,6 +3017,7 @@ mod typed_array_rmw; pub(crate) use instance_misc1::builtin_parent_reserved_class_id; pub(crate) mod class_field_inline_guard; pub(crate) mod element_shape_guard; +pub(crate) mod element_shape_reads; mod js_runtime; mod literals_vars; mod logical_collections; diff --git a/crates/perry-codegen/src/expr/property_get/helpers.rs b/crates/perry-codegen/src/expr/property_get/helpers.rs index 51a8e8dbe6..302fedd4b8 100644 --- a/crates/perry-codegen/src/expr/property_get/helpers.rs +++ b/crates/perry-codegen/src/expr/property_get/helpers.rs @@ -368,29 +368,14 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context( // 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 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) - } - }; + // #10123: the index the preheader discharged a bounds obligation for. + // Every form is an i32 in `[0, length)` by the time it reaches the GEP, + // which is why the fast clone pays no per-read bounds test in any of + // them. `None` means the counter's canonical i32 slot the matcher + // required is missing, so there is nothing to index with. + if let Some(idx_i32) = + crate::expr::element_shape_guard::emit_element_shape_index(ctx, &fact) + { let value = crate::expr::element_shape_guard::emit_element_shape_field_load( ctx, &fact, diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index 4f1ff8e459..02c60d1bbe 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -176,18 +176,32 @@ pub(crate) fn emit_shadow_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) { // #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. + // + // #10199's carried index (`c = (a*c + b) % m`) and its optional + // `const index = c` alias are two more of the same. 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 .into_iter() - .chain(virtual_binding) + .chain(fact.index.virtual_locals()) .any(|id| ctx.shadow_slot_map.get(&id) == Some(&slot_idx)) }) { return; } + // #10199: a SYNTHESIZED fast-clone body (the accumulator fold, the carried + // write-back) no longer has the statement indices the function-wide clear + // map is keyed by, so every clear reached from inside it would be + // attributed to some other statement's local. Suppress them for the fast + // clone only: it is call-free, so no collection can observe a shadow slot + // while it runs, an over-rooted stale value is rewritten like any root, and + // the slow clone — lowered from the ORIGINAL body after the facts are + // popped — keeps every clear it always had. + if ctx + .element_shape_loop_facts + .iter() + .any(|fact| fact.synthesized_body) + { + return; + } // Never-bound slot: it provably still holds its initial 0 (slots are only // written through bind/set, and every value-set site binds first), so the // clear would be a redundant `js_shadow_slot_set(idx, 0)` TLS hit. diff --git a/crates/perry-codegen/src/stmt/element_shape_carried.rs b/crates/perry-codegen/src/stmt/element_shape_carried.rs new file mode 100644 index 0000000000..d6e8700436 --- /dev/null +++ b/crates/perry-codegen/src/stmt/element_shape_carried.rs @@ -0,0 +1,416 @@ +//! #10199: the element-shape clone's LOOP-CARRIED index — the `random` access +//! shape. +//! +//! ```text +//! let cursor = 0; +//! for (let i = 0; i < count; i++) { +//! cursor = (cursor * 17 + 7) % length; // the recurrence +//! const index = cursor; // optional alias +//! sum += rows[index].id; +//! } +//! ``` +//! +//! ## What makes this different from `const d = i % m` +//! +//! `d` is dead the moment the iteration ends, so #10123 could keep it entirely +//! virtual — its `Let` writes a private i32 alloca and nothing ever reads the +//! real slot. `cursor` is a real `let` that OUTLIVES the loop, so the clone +//! owes a write-back. +//! +//! **Where that write-back goes is a correctness question, not a scheduling +//! one.** The clone's residual check side-exits by resuming the CURRENT +//! iteration in the slow clone, which re-runs the whole body — the recurrence +//! included. A write-back at the update site would therefore apply +//! `cursor = (cursor * 17 + 7) % length` twice for that iteration and every +//! subsequent index would be wrong (silently: still in bounds, still a valid +//! record, just not the one JavaScript names). So the commit is the LAST thing +//! the iteration does, after every side exit it can take: +//! +//! * mid-iteration exit ⇒ the real slot still holds the previous commit, which +//! is exactly this iteration's entry value, and the slow clone advances it +//! once; +//! * normal exit ⇒ the last iteration committed, so the real slot holds the +//! final value the code after the loop reads. +//! +//! ## Why the recurrence is evaluated in i64 +//! +//! JavaScript evaluates `(cursor * 17 + 7) % length` in doubles. `frem` on +//! aarch64 is a libm `fmod` call — inside this clone a call is not a slow path, +//! it DELETES the clone (#7690) — so the recurrence is folded to one affine +//! pair `(a, b)` and evaluated as `srem i64`. +//! +//! That is only equal to what JavaScript computes while every intermediate is +//! exactly representable as a double, so the matcher tracks the maximum +//! magnitude any sub-expression can reach over `carried ∈ [0, i32::MAX]` and +//! declines above 2^53. (Fitting i64, which the original design note asked for, +//! is necessary but NOT sufficient: `c * 1e10 + 1` fits i64 comfortably and +//! loses its low bits in the f64 JavaScript actually runs.) `a` and `b` are +//! also required non-negative, so the dividend is non-negative and `srem` +//! agrees with JS `%` — which returns a NEGATIVE remainder for a negative +//! dividend, i.e. an out-of-bounds subscript, not a slow path. + +use anyhow::Result; +use perry_hir::{BinaryOp, Expr, Stmt}; + +use super::loops::{local_bound_is_loop_invariant, local_has_readable_slot}; +use crate::expr::FnCtx; +use crate::types::{DOUBLE, I32, I64}; + +/// The largest integer magnitude an `f64` represents exactly. Above it the i64 +/// recurrence and the f64 one JavaScript runs can disagree. +const EXACT_F64_INTEGER_LIMIT: i64 = 9_007_199_254_740_992; // 2^53 + +/// The preheader materializes the entry value in `0..=i32::MAX`, so this bounds +/// every `carried` the recurrence can ever see. +const MAX_CARRIED: i64 = i32::MAX as i64; + +/// A matched `carried = % m` update. +#[derive(Clone, Copy, Debug)] +pub(super) struct MatchedCarried { + pub carried_id: u32, + pub modulus_id: u32, + pub coeff_a: i64, + pub coeff_b: i64, +} + +/// Fold `expr` to `a * carried + b`, tracking the largest magnitude ANY +/// sub-expression can reach for `carried ∈ [0, i32::MAX]`. +/// +/// Returns `(a, b, max_abs)`. `None` declines — an unknown node, a second +/// variable, a non-integral literal, or any overflow of the checked i64 +/// arithmetic used to compute the bounds. +/// +/// The magnitude is tracked per NODE rather than taken from the folded result +/// because folding can cancel: `c * 1000 - c * 999` is `1 * c`, but JavaScript +/// still evaluates both thousand-fold products. +fn affine_over_carried(expr: &Expr, carried_id: u32) -> Option<(i64, i64, i64)> { + let magnitude = |a: i64, b: i64| -> Option { + a.checked_abs()? + .checked_mul(MAX_CARRIED)? + .checked_add(b.checked_abs()?) + }; + match expr { + Expr::LocalGet(id) if *id == carried_id => Some((1, 0, MAX_CARRIED)), + Expr::Integer(k) => Some((0, *k, k.checked_abs()?)), + Expr::Number(n) if n.is_finite() && n.fract() == 0.0 => { + // `as i64` saturates rather than wrapping, so an out-of-range + // literal would fold to i64::MAX and read as a huge-but-valid + // coefficient. Range-check before converting. + if *n < -(EXACT_F64_INTEGER_LIMIT as f64) || *n > EXACT_F64_INTEGER_LIMIT as f64 { + return None; + } + let k = *n as i64; + Some((0, k, k.checked_abs()?)) + } + Expr::Binary { op, left, right } => { + let (la, lb, lmax) = affine_over_carried(left, carried_id)?; + let (ra, rb, rmax) = affine_over_carried(right, carried_id)?; + let (a, b) = match op { + BinaryOp::Add => (la.checked_add(ra)?, lb.checked_add(rb)?), + BinaryOp::Sub => (la.checked_sub(ra)?, lb.checked_sub(rb)?), + // A product is affine only when one side is a constant; `c * c` + // is not, and neither is `c * m` for a runtime `m`. + BinaryOp::Mul if la == 0 => (ra.checked_mul(lb)?, rb.checked_mul(lb)?), + BinaryOp::Mul if ra == 0 => (la.checked_mul(rb)?, lb.checked_mul(rb)?), + _ => return None, + }; + let here = magnitude(a, b)?; + Some((a, b, lmax.max(rmax).max(here))) + } + _ => None, + } +} + +/// Does `expr` read `id`? +/// +/// Conservative by construction: an expression node this does not enumerate +/// answers "yes". The alternative shape — a catch-all that returns `false` — +/// is the #6377 / `collect_local_refs_expr` failure mode CLAUDE.md names, and +/// here it would let an accumulator read be folded into a statement that no +/// longer sees the value it was written against. +pub(super) fn expr_reads_local(expr: &Expr, id: u32) -> bool { + match expr { + Expr::LocalGet(read) => *read == id, + Expr::Number(_) | Expr::Integer(_) => false, + Expr::Binary { left, right, .. } + | Expr::MathImul(left, right) + | Expr::MathPow(left, right) => expr_reads_local(left, id) || expr_reads_local(right, id), + Expr::NumberCoerce(operand) => expr_reads_local(operand, id), + Expr::MathMin(values) | Expr::MathMax(values) => { + values.iter().any(|e| expr_reads_local(e, id)) + } + Expr::MathAbs(value) + | Expr::MathSqrt(value) + | Expr::MathFloor(value) + | Expr::MathCeil(value) + | Expr::MathRound(value) + | Expr::MathTrunc(value) + | Expr::MathSign(value) + | Expr::MathF16round(value) => expr_reads_local(value, id), + Expr::PropertyGet { object, .. } => expr_reads_local(object, id), + Expr::IndexGet { object, index } => { + expr_reads_local(object, id) || expr_reads_local(index, id) + } + Expr::Conditional { + condition, + then_expr, + else_expr, + } => { + expr_reads_local(condition, id) + || expr_reads_local(then_expr, id) + || expr_reads_local(else_expr, id) + } + _ => true, + } +} + +/// Match the body's first statement as `carried = % m`. +#[allow(clippy::too_many_arguments)] +pub(super) fn match_carried_update( + ctx: &FnCtx<'_>, + stmt: &Stmt, + counter_id: u32, + condition: &Expr, + update: Option<&Expr>, + body: &[Stmt], +) -> Option { + let Stmt::Expr(Expr::LocalSet(carried_id, value)) = stmt else { + return None; + }; + let Expr::Binary { + op: BinaryOp::Mod, + left, + right, + } = value.as_ref() + else { + return None; + }; + let Expr::LocalGet(modulus_id) = right.as_ref() else { + return None; + }; + if *carried_id == counter_id || *modulus_id == counter_id || *modulus_id == *carried_id { + return None; + } + + let (coeff_a, coeff_b, max_abs) = affine_over_carried(left, *carried_id)?; + // A negative dividend makes JS `%` return a negative remainder — an + // out-of-bounds subscript the preheader's `m <= length` says nothing about. + // With `carried >= 0` (the preheader materializes it in `0..=i32::MAX`) and + // both coefficients non-negative, the dividend cannot be negative. + if coeff_a < 0 || coeff_b < 0 { + return None; + } + if max_abs > EXACT_F64_INTEGER_LIMIT { + return None; + } + + // The binding must be a plain, directly-addressable local the clone can + // write back to with one store. A module global would need the rooted-store + // path, a boxed or captured binding lives in a cell a plain store would + // leave stale for an observer outside the clone. + if !ctx.locals.contains_key(carried_id) + || ctx.module_globals.contains_key(carried_id) + || ctx.boxed_vars.contains(carried_id) + || ctx.closure_captures.contains_key(carried_id) + || !local_has_readable_slot(ctx, *carried_id) + || ctx.pod_records.contains_key(carried_id) + || ctx.scalar_replaced.contains_key(carried_id) + { + return None; + } + // `carried` is written exactly once per iteration — by the statement above. + // Every other statement the body matcher admits writes the accumulator, and + // it rejects a body whose accumulator IS the carried local, so the only way + // a second write could appear is a `for` update expression naming it. + if update.is_some_and(|u| expr_writes_local(u, *carried_id)) { + return None; + } + // The modulus is materialized ONCE in the preheader and every iteration's + // `srem` uses that value, so a body (or update, or condition) that could + // rewrite it would derive indices against a stale bound. + if 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) + { + return None; + } + + Some(MatchedCarried { + carried_id: *carried_id, + modulus_id: *modulus_id, + coeff_a, + coeff_b, + }) +} + +/// Does `expr` assign to `id`? Conservative in the same direction as +/// [`expr_reads_local`] — an unenumerated node answers "yes". +fn expr_writes_local(expr: &Expr, id: u32) -> bool { + match expr { + Expr::LocalSet(target, _) => *target == id, + Expr::Update { id: target, .. } => *target == id, + Expr::LocalGet(_) | Expr::Number(_) | Expr::Integer(_) => false, + Expr::Binary { left, right, .. } => { + expr_writes_local(left, id) || expr_writes_local(right, id) + } + _ => true, + } +} + +/// Lower the fast clone's carried-index statements, or report that this +/// expression is not one. +/// +/// Two shapes, both synthesized or admitted by the matcher, and neither lowered +/// generically: +/// +/// * the UPDATE `carried = % m` — one `mul`/`add`/`srem` chain in i64 +/// writing the clone's private i32 slot, never the real binding; +/// * the COMMIT `carried = carried`, the trailing statement the matcher appends +/// to the synthesized fast body — one `sitofp` + `store` that publishes the +/// iteration's value to the real slot. +/// +/// The commit is spelled as a self-assignment because it has to be a statement +/// the generic lowering would also accept (the slow clone never sees it — the +/// facts are popped first — but nothing may depend on that). A user-written +/// `c = c` inside a body cannot reach here: the body matcher admits only the +/// carried update, one virtual binding and accumulator writes, and `c` is not +/// the accumulator. +pub(super) fn lower_virtual_carried_stmt(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { + let Expr::LocalSet(id, value) = expr else { + return Ok(false); + }; + let Some((carried, scope_id)) = ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { + let crate::expr::ElementShapeIndex::Carried(carried) = &fact.index else { + return None; + }; + (carried.local_id == *id).then(|| (carried.clone(), fact.scope_id)) + }) else { + return Ok(false); + }; + + if matches!(value.as_ref(), Expr::LocalGet(read) if *read == *id) { + // COMMIT. The value published here is the one every read in this + // iteration already used, so the slot and the binding agree from the + // end of the iteration onward. + let slot = carried.slot.clone(); + let commit_slot = carried.commit_slot.clone(); + let blk = ctx.block(); + let current = blk.load(I32, &slot); + let boxed = blk.sitofp(I32, ¤t, DOUBLE); + blk.store(DOUBLE, &boxed, &commit_slot); + return Ok(true); + } + + // UPDATE. `0 <= carried <= i32::MAX` and `1 <= m <= i32::MAX` hold by the + // preheader's two materializations, and the matcher proved + // `a * i32::MAX + b < 2^53`, so the i64 chain cannot overflow and agrees + // with the f64 arithmetic JavaScript performs. + let slot = carried.slot.clone(); + let modulus_i32 = carried.modulus_i32.clone(); + let coeff_a = carried.coeff_a.to_string(); + let coeff_b = carried.coeff_b.to_string(); + { + let blk = ctx.block(); + let current = blk.load(I32, &slot); + let current64 = blk.sext(I32, ¤t, I64); + let scaled = blk.mul(I64, ¤t64, &coeff_a); + let shifted = blk.add(I64, &scaled, &coeff_b); + let modulus64 = blk.sext(I32, &modulus_i32, I64); + let wrapped = blk.srem(I64, &shifted, &modulus64); + let next = blk.trunc(I64, &wrapped, I32); + blk.store(I32, &next, &slot); + } + // The index for this iteration is now in its slot, so a body with no alias + // binding hangs the shared element prologue off the update itself. + super::element_shape_loop::emit_element_prefetch_for(ctx, scope_id, *id); + Ok(true) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn local(id: u32) -> Expr { + Expr::LocalGet(id) + } + fn mul(l: Expr, r: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(l), + right: Box::new(r), + } + } + fn add(l: Expr, r: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(l), + right: Box::new(r), + } + } + fn sub(l: Expr, r: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Sub, + left: Box::new(l), + right: Box::new(r), + } + } + + #[test] + fn the_benchmark_recurrence_folds() { + // `(cursor * 17 + 7)` + let e = add(mul(local(9), Expr::Integer(17)), Expr::Integer(7)); + assert_eq!( + affine_over_carried(&e, 9), + Some((17, 7, 17 * MAX_CARRIED + 7)) + ); + } + + #[test] + fn a_constant_only_expression_folds_to_b() { + assert_eq!(affine_over_carried(&Expr::Integer(5), 9), Some((0, 5, 5))); + } + + #[test] + fn a_second_variable_declines() { + assert_eq!(affine_over_carried(&add(local(9), local(10)), 9), None); + assert_eq!(affine_over_carried(&mul(local(9), local(9)), 9), None); + } + + /// The magnitude bound is per NODE, not of the folded pair: cancellation + /// must not hide an intermediate JavaScript really computes in f64. + #[test] + fn cancelling_products_still_report_their_intermediates() { + // `c * 1000000000 - c * 999999999` folds to `1 * c`, but both products + // reach ~2e18 for a large `c` — far past 2^53, where the f64 JS runs + // and this i64 chain stop agreeing. + let e = sub( + mul(local(9), Expr::Integer(1_000_000_000)), + mul(local(9), Expr::Integer(999_999_999)), + ); + let (a, b, max_abs) = affine_over_carried(&e, 9).expect("affine"); + assert_eq!((a, b), (1, 0)); + assert!( + max_abs > EXACT_F64_INTEGER_LIMIT, + "the intermediate magnitude must be reported, not the folded one" + ); + } + + #[test] + fn a_non_integral_literal_declines() { + assert_eq!(affine_over_carried(&Expr::Number(1.5), 9), None); + assert_eq!(affine_over_carried(&Expr::Number(f64::NAN), 9), None); + assert_eq!(affine_over_carried(&Expr::Number(f64::INFINITY), 9), None); + // Saturating `as i64` would turn this into a plausible coefficient. + assert_eq!(affine_over_carried(&Expr::Number(1e300), 9), None); + } + + #[test] + fn reads_are_detected_conservatively() { + assert!(expr_reads_local(&add(local(9), Expr::Integer(1)), 9)); + assert!(!expr_reads_local(&add(local(8), Expr::Integer(1)), 9)); + // An unenumerated node answers "reads it". + assert!(expr_reads_local(&Expr::This, 9)); + } +} diff --git a/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs b/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs new file mode 100644 index 0000000000..c86fbd7e88 --- /dev/null +++ b/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs @@ -0,0 +1,650 @@ +//! #10199: the `fields` and `random` access shapes — IR census for the +//! element-shape clone's loop-carried index, its multi-statement accumulator +//! fold, and its two non-numeric reads. +//! +//! A child module of `element_shape_shape_keyed_tests` (see the `mod` +//! declaration there), so `use super::*` brings both that file's shape-keyed +//! assertions and `element_shape_loop_tests`'s slicing helpers. +//! +//! Every positive here is paired with a sabotage case, and the two liveness +//! assertions #10171 was short — the preheader's EXACT comparison set, and that +//! the residual check really is shared rather than repeated — are the ones that +//! would have caught a clone that is emitted, branched into, and never entered +//! or never actually cheaper. + +use super::*; + +const ROWS2_ID: u32 = 40; +const COUNT2_ID: u32 = 41; +const MOD2_ID: u32 = 42; +const SUM2_ID: u32 = 43; +const COUNTER2_ID: u32 = 44; +const CURSOR_ID: u32 = 45; +const ALIAS_ID: u32 = 46; +const OTHER_ID: u32 = 47; + +/// `rows[].` +fn rows_field(index: Expr, prop: &str) -> Expr { + Expr::PropertyGet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ROWS2_ID)), + index: Box::new(index), + }), + property: prop.to_string(), + byte_offset: 0, + } +} + +/// `sum = sum + ` +fn add_to_sum(value: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet( + SUM2_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(SUM2_ID)), + right: Box::new(value), + }), + )) +} + +fn binary(op: BinaryOp, left: Expr, right: Expr) -> Expr { + Expr::Binary { + op, + left: Box::new(left), + right: Box::new(right), + } +} + +/// `cursor = % n` +fn cursor_update(affine: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet( + CURSOR_ID, + Box::new(binary(BinaryOp::Mod, affine, Expr::LocalGet(MOD2_ID))), + )) +} + +/// The benchmark's own recurrence, `(cursor * 17 + 7)`. +fn benchmark_affine() -> Expr { + binary( + BinaryOp::Add, + binary(BinaryOp::Mul, Expr::LocalGet(CURSOR_ID), Expr::Integer(17)), + Expr::Integer(7), + ) +} + +/// `const index = cursor;` +fn cursor_alias() -> Stmt { + Stmt::Let { + id: ALIAS_ID, + name: "index".to_string(), + ty: Type::Number, + mutable: false, + init: Some(Expr::LocalGet(CURSOR_ID)), + } +} + +/// `const index = i % n;` +fn modulo_index() -> Stmt { + Stmt::Let { + id: ALIAS_ID, + name: "index".to_string(), + ty: Type::Any, + mutable: false, + init: Some(binary( + BinaryOp::Mod, + Expr::LocalGet(COUNTER2_ID), + Expr::LocalGet(MOD2_ID), + )), + } +} + +/// ```text +/// function run(rows: any, count: number, n: number): number { +/// let sum = 0; +/// let cursor = 0; +/// for (let i = 0; i < count; i++) +/// return sum; +/// } +/// ``` +fn access_module(body: Vec) -> Module { + let mut m = Module::new("element_shape_loop.ts"); + let param = |id: u32, name: &str, ty: Type| perry_hir::Param { + id, + name: name.to_string(), + ty, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }; + m.functions = vec![perry_hir::Function { + id: 902, + name: "run".to_string(), + type_params: Vec::new(), + params: vec![ + param(ROWS2_ID, "rows", Type::Any), + param(COUNT2_ID, "count", Type::Number), + param(MOD2_ID, "n", Type::Number), + ], + return_type: Type::Number, + body: vec![ + Stmt::Let { + id: SUM2_ID, + name: "sum".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(0)), + }, + Stmt::Let { + id: CURSOR_ID, + name: "cursor".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(0)), + }, + Stmt::For { + init: Some(Box::new(Stmt::Let { + id: COUNTER2_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(COUNTER2_ID)), + right: Box::new(Expr::LocalGet(COUNT2_ID)), + }), + update: Some(Expr::Update { + id: COUNTER2_ID, + op: UpdateOp::Increment, + prefix: false, + }), + body, + }, + Stmt::Return(Some(Expr::LocalGet(SUM2_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 benchmark's `random` body, with and without the alias spelling. +fn random_body(with_alias: bool) -> Vec { + let index = if with_alias { ALIAS_ID } else { CURSOR_ID }; + let mut body = vec![cursor_update(benchmark_affine())]; + if with_alias { + body.push(cursor_alias()); + } + body.push(add_to_sum(rows_field(Expr::LocalGet(index), "id"))); + body +} + +/// The benchmark's `fields` body: three accumulator statements over ONE +/// element, one through a string and one through a boolean. +fn fields_body() -> Vec { + vec![ + modulo_index(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + add_to_sum(Expr::PropertyGet { + object: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "name")), + property: "length".to_string(), + byte_offset: 0, + }), + add_to_sum(Expr::Conditional { + condition: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "active")), + then_expr: Box::new(Expr::Integer(1)), + else_expr: Box::new(Expr::Integer(0)), + }), + ] +} + +/// How many side exits the clone can take per iteration — one branch per +/// residual / tag test that leaves for the slow preheader. +fn side_exit_count(fast: &str) -> usize { + fast.matches("label %element_shape.loop.slow.preheader") + .count() +} + +/// The LAST block in `fast` whose label starts with `prefix`, body only. +/// +/// Used to name the block a commit must live in. Reaching a block named after a +/// passed tag test IS the proof that every side exit of the iteration is behind +/// it — which is stronger than any textual before/after comparison, because the +/// emitted block ORDER is not the execution order (the loop's `update` block is +/// printed before the body blocks that branch into it). +fn last_block_with_prefix<'a>(fast: &'a str, prefix: &str) -> &'a str { + let start = fast + .rmatch_indices('\n') + .map(|(idx, _)| idx + 1) + .find(|&idx| { + fast[idx..].starts_with(prefix) + && fast[idx..] + .lines() + .next() + .unwrap_or("") + .trim_end() + .ends_with(':') + }) + .unwrap_or_else(|| panic!("no block named `{prefix}*` in:\n{fast}")); + let body = &fast[start..]; + let end = body.find("\n\n").unwrap_or(body.len()); + &body[..end] +} + +/// Count the `store` instructions one iteration of the clone executes. +fn store_count(fast: &str) -> usize { + fast.lines() + .filter(|l| l.trim_start().starts_with("store ")) + .count() +} + +// --------------------------------------------------------------------------- +// #10199 — the LOOP-CARRIED index (`random`). +// --------------------------------------------------------------------------- + +#[test] +fn a_carried_recurrence_gets_a_shape_keyed_clone() { + let ir = emit(&access_module(random_body(true))); + assert_shape_keyed_clone(&ir, "carried index with an alias"); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("srem i64") && fast.contains("mul i64"), + "the recurrence must be the i64 chain, not an `frem` (a libm call on \ + aarch64, which would delete the clone); emitted:\n{fast}" + ); + assert!( + !fast.contains("frem"), + "an `frem` in the clone is the whole reason this form needed its own \ + lowering; emitted:\n{fast}" + ); + assert!( + ir.contains("element_shape.loop.carried.range"), + "the carried ENTRY value must be materialized as a validated \ + non-negative i32 — the i64 bound and the `srem` sign argument are \ + both stated over `0..=i32::MAX`" + ); + assert!( + ir.contains("element_shape.loop.modulus.range"), + "the modulus must be materialized: `srem` by zero is UB and `x % 0` \ + is NaN in JS" + ); +} + +#[test] +fn a_carried_recurrence_matches_without_the_alias() { + // `rows[cursor]` directly — the alias is sugar, not part of the form. + let ir = emit(&access_module(random_body(false))); + assert_shape_keyed_clone(&ir, "carried index, no alias"); + let fast = fast_clone_slice(&ir); + assert!(fast.contains("srem i64"), "emitted:\n{fast}"); +} + +/// THE correctness assertion for this form. +/// +/// The residual side exit resumes the CURRENT iteration in the slow clone, +/// which re-runs the whole body — the recurrence included. So the write-back to +/// the real binding must come after EVERY exit the iteration can take, or the +/// recurrence is applied twice and every later index is silently wrong (still +/// in bounds, still a valid record, just not the one JavaScript names). +#[test] +fn the_carried_write_back_follows_every_side_exit() { + let ir = emit(&access_module(random_body(true))); + let fast = fast_clone_slice(&ir); + // Every side exit this iteration can take is a tag test on a loaded word, + // and the LAST of them is the `id` read's Number test. Its admitted arm — + // `element_shape.number.*` — is therefore reachable only once the whole + // iteration is past every exit, so a commit inside it is provably last. + let committed = last_block_with_prefix(&fast, "element_shape.number"); + assert!( + committed.contains("sitofp i32") && committed.contains("store double"), + "the carried write-back must live past every side exit; a write-back \ + before one makes the slow clone apply the recurrence twice for that \ + iteration, and every later index is silently wrong. \ + final block:\n{committed}\nfull clone:\n{fast}" + ); + assert_eq!( + committed.matches("store double").count(), + 1, + "the carried value must be published exactly once per iteration; \ + final block:\n{committed}" + ); + assert_eq!( + store_count(&fast), + // the carried i32 slot, the prefetched element handle, the + // accumulator, the carried write-back, and the counter's increment + 5, + "a second write to the carried binding would double-apply the \ + recurrence on a side exit; emitted:\n{fast}" + ); +} + +/// The same liveness assertion #10123's derived index carries: the ONE length +/// comparison the preheader owes is `modulus <= length`, NOT the counter arm's +/// `length >= bound`. Demanding the latter makes the clone unenterable whenever +/// the loop runs more times than the array is long — which is every access +/// benchmark. +#[test] +fn a_carried_index_owes_exactly_the_modulus_obligation() { + let ir = emit(&access_module(random_body(true))); + let deref = block_slice(&ir, "element_shape.loop.preheader.deref"); + assert_eq!( + deref.matches("icmp uge i32").count(), + 1, + "a carried index owes exactly ONE length comparison (`modulus <= \ + length`); 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_negative_coefficient_declines() { + // `(cursor * 17 - 7) % n` can produce a negative dividend, and JS `%` + // returns a NEGATIVE remainder for one — an out-of-bounds subscript the + // preheader's `m <= length` says nothing about. + let body = vec![ + cursor_update(binary( + BinaryOp::Sub, + binary(BinaryOp::Mul, Expr::LocalGet(CURSOR_ID), Expr::Integer(17)), + Expr::Integer(7), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a recurrence whose dividend can go negative must decline" + ); +} + +#[test] +fn a_non_affine_recurrence_declines() { + // `(cursor * cursor) % n` is not affine, so no (a, b) pair describes it and + // no magnitude bound can be proven. + let body = vec![ + cursor_update(binary( + BinaryOp::Mul, + Expr::LocalGet(CURSOR_ID), + Expr::LocalGet(CURSOR_ID), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a non-affine recurrence must decline" + ); +} + +#[test] +fn a_recurrence_that_leaves_exact_double_range_declines() { + // `cursor * 1e9 + 1` fits i64 comfortably and does NOT fit the f64 that + // JavaScript actually evaluates it in, so the i64 chain and the program + // would disagree. "Fits i64" was the original design note's obligation; it + // is necessary and not sufficient. + let body = vec![ + cursor_update(binary( + BinaryOp::Add, + binary( + BinaryOp::Mul, + Expr::LocalGet(CURSOR_ID), + Expr::Integer(1_000_000_000), + ), + Expr::Integer(1), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a recurrence whose intermediates pass 2^53 must decline: JS evaluates \ + it in doubles and the clone's i64 chain would not agree" + ); +} + +#[test] +fn a_bare_read_of_the_carried_local_declines() { + // Inside the clone the real slot is one iteration behind until the trailing + // commit, so a bare `cursor` in the accumulator would read a stale value. + let body = vec![ + cursor_update(benchmark_affine()), + cursor_alias(), + add_to_sum(binary( + BinaryOp::Add, + rows_field(Expr::LocalGet(ALIAS_ID), "id"), + Expr::LocalGet(CURSOR_ID), + )), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a bare read of the carried local must decline the clone" + ); +} + +#[test] +fn a_recurrence_over_a_second_variable_declines() { + // `(cursor + other) % n` is not a function of `cursor` alone; `other` is + // not materialized, bounded, or proven loop-invariant. + let body = vec![ + cursor_update(binary( + BinaryOp::Add, + Expr::LocalGet(CURSOR_ID), + Expr::LocalGet(OTHER_ID), + )), + cursor_alias(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "a recurrence reading a second variable must decline" + ); +} + +// --------------------------------------------------------------------------- +// #10199 — the MULTI-STATEMENT body, the string `.length` and the ternary +// (`fields`). +// --------------------------------------------------------------------------- + +#[test] +fn the_fields_body_gets_a_shape_keyed_clone() { + let ir = emit(&access_module(fields_body())); + assert_shape_keyed_clone(&ir, "three reads of one element"); + let fast = fast_clone_slice(&ir); + assert!( + fast.contains("element_shape.strlen.heap") && fast.contains("element_shape.strlen.sso"), + "a string `.length` must decode BOTH representations inline — a call \ + to the runtime would delete the clone; emitted:\n{fast}" + ); + assert!( + fast.contains("select i1"), + "the ternary must become a select between the two constants; \ + emitted:\n{fast}" + ); + assert!( + fast.contains("9222246136947933188") && fast.contains("9222246136947933187"), + "the ternary must test the two boolean singletons by exact NaN-box bit \ + pattern; guessing JS truthiness in the clone would be a miscompile; \ + emitted:\n{fast}" + ); +} + +/// THE cost assertion. Three reads of `rows[index]` are three loads of the same +/// element and three repeats of the residual header/ShapeId check unless the +/// prologue shares them — and a clone that is entered but pays 3x the guard is +/// exactly the "emitted, branched into, and not actually cheaper" failure the +/// IR census exists to catch. +#[test] +fn three_reads_of_one_element_share_one_residual_check() { + let ir = emit(&access_module(fields_body())); + let fast = fast_clone_slice(&ir); + assert_eq!( + fast.matches("134250751").count(), + 1, + "the shape-keyed residual mask must appear ONCE per iteration, not \ + once per read; emitted:\n{fast}" + ); + assert_eq!( + fast.matches("element_shape.load").count(), + // one block definition plus the `br` that targets it + 2, + "there must be exactly one residual-check block in the iteration; \ + emitted:\n{fast}" + ); + // Four tag/residual tests, not twelve: the residual once, then one + // representation test per read. + assert_eq!( + side_exit_count(&fast), + 4, + "one residual check plus one tag test per read; emitted:\n{fast}" + ); +} + +/// THE correctness assertion for the multi-statement body. `sum += a; sum += b` +/// commits `sum` twice; if the second read's tag test side-exits, the slow +/// clone re-runs the iteration and applies `a` a second time. The fold makes +/// the whole iteration commit once, after every exit. +#[test] +fn the_accumulator_commits_once_after_every_side_exit() { + let ir = emit(&access_module(fields_body())); + let fast = fast_clone_slice(&ir); + assert_eq!( + fast.matches("fadd double").count(), + 3, + "the fold keeps all three additions in source order — it changes when \ + the result is STORED, never what is computed; emitted:\n{fast}" + ); + assert_eq!( + store_count(&fast), + // the derived index, the prefetched element handle, the accumulator, + // and the counter's own increment + 4, + "three accumulator statements must fold to ONE accumulator store; a \ + second one means a side exit from a later read can leave an earlier \ + read already applied when the slow clone re-runs the iteration; \ + emitted:\n{fast}" + ); + // The one store lives in the block the LAST tag test branches into, so it + // is behind every side exit of the iteration. + let committed = last_block_with_prefix(&fast, "element_shape.bool"); + assert!( + committed.contains("store "), + "the accumulator store must live past every side exit; final \ + block:\n{committed}\nfull clone:\n{fast}" + ); +} + +#[test] +fn an_addend_that_reads_the_accumulator_declines() { + // `sum += rows[i].id; sum += sum` — folding would substitute the PRE-first + // -statement value of `sum` into the second addend. + let body = vec![ + modulo_index(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + add_to_sum(Expr::LocalGet(SUM2_ID)), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "an addend reading the accumulator must decline the fold, and with it \ + the clone" + ); +} + +#[test] +fn two_accumulators_decline() { + let body = vec![ + modulo_index(), + add_to_sum(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + Stmt::Expr(Expr::LocalSet( + CURSOR_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(CURSOR_ID)), + right: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + }), + )), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "the body must write exactly one accumulator" + ); +} + +#[test] +fn a_ternary_with_a_non_constant_arm_declines() { + let body = vec![ + modulo_index(), + add_to_sum(Expr::Conditional { + condition: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "active")), + then_expr: Box::new(rows_field(Expr::LocalGet(ALIAS_ID), "id")), + else_expr: Box::new(Expr::Integer(0)), + }), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "only integer-constant ternary arms are admitted" + ); +} + +#[test] +fn a_ternary_on_something_other_than_an_element_read_declines() { + let body = vec![ + modulo_index(), + add_to_sum(Expr::Conditional { + condition: Box::new(Expr::LocalGet(COUNTER2_ID)), + then_expr: Box::new(Expr::Integer(1)), + else_expr: Box::new(Expr::Integer(0)), + }), + ]; + let ir = emit(&access_module(body)); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "the clone lowers a boolean-singleton test, not general JS truthiness" + ); +} + +/// The two non-numeric reads are SHAPE-keyed only: a class-keyed clone's slot +/// holds a raw `double`, so there is no NaN-box tag to test and both readers +/// would be decoding a double's bit pattern. +#[test] +fn a_class_typed_array_declines_the_string_length_read() { + let ir = emit(&element_shape_module( + vec![Stmt::Expr(Expr::LocalSet( + SUM_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(SUM_ID)), + right: Box::new(Expr::PropertyGet { + object: Box::new(elem_field(ARRAY_ID, Expr::LocalGet(COUNTER_ID))), + property: "length".to_string(), + byte_offset: 0, + }), + }), + ))], + None, + )); + assert!( + !ir.contains("element_shape.strlen"), + "the class-keyed arm reads raw doubles; there is no tag to test" + ); +} diff --git a/crates/perry-codegen/src/stmt/element_shape_loop.rs b/crates/perry-codegen/src/stmt/element_shape_loop.rs index 1ce4f0cd8d..9bc43a8d17 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop.rs @@ -152,7 +152,7 @@ use super::loops::{ CLASS_FIELD_LOOP_PROP_DENYLIST, }; use crate::expr::{lower_expr, FnCtx}; -use crate::types::{DOUBLE, I1, I32}; +use crate::types::{DOUBLE, I1, I32, I64}; /// Loop bound: a literal, a loop-invariant local / module global that is /// materialized to i32 once in the preheader, or the tracked array's own @@ -210,14 +210,28 @@ enum ElementShapeIdentity { enum MatchedIndex { Counter, Constant(i64), - DerivedMod { local_id: u32, modulus_id: u32 }, + DerivedMod { + local_id: u32, + modulus_id: u32, + }, + /// #10199: `c = (a*c + b) % m; … arr[c]`, optionally spelled through a + /// `const index = c` alias. The recurrence itself lives on + /// [`ElementShapeVersionedLoop::carried`] — this arm records only the + /// spellings the subscript may take. See `stmt/element_shape_carried.rs`. + Carried { + carried_id: u32, + alias_id: Option, + }, } 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(_)) + !matches!( + self, + MatchedIndex::Constant(_) | MatchedIndex::Carried { .. } + ) } } @@ -235,6 +249,21 @@ struct ElementShapeVersionedLoop { /// form; `None` for the original single-statement accumulator body. element_binding: Option, accumulator_id: u32, + /// #10199: the matched recurrence, for the [`MatchedIndex::Carried`] form. + carried: Option, + /// #10199: the body the FAST clone lowers, when it is not the source body. + /// + /// Two rewrites, both of which exist to put every side exit an iteration + /// can take BEFORE any of its stores: + /// + /// * K accumulator statements fold into one, so a tag test that fails on + /// the third read cannot leave the first two already applied to `acc` + /// when the slow clone re-runs the iteration; + /// * a carried recurrence gains a trailing commit statement, so the real + /// binding is published once the iteration is past every exit. + /// + /// The SLOW clone always lowers the original `body`. + fast_body: Option>, } /// The locals the pure-expression walk reasons about. @@ -246,6 +275,11 @@ struct PureExprScope { /// #10123: `(d, m)` for a body whose first statement is /// `const d = counter % m`. derived: Option<(u32, u32)>, + /// #10199: `(carried, alias)` for a body whose first statement advances a + /// loop-carried index. Both are excluded from bare reads for the same + /// reason `derived` is — inside the clone the real slot is one iteration + /// behind until the trailing commit. + carried: Option<(u32, Option)>, } /// What the walk collects. @@ -311,8 +345,33 @@ fn element_shape_loop_pure_expr_collect( out.props.insert(property.clone()); true } + // #10199: `arr[].prop.length` — a STRING field's JS length. + // Admitted as its own form rather than as "a read of `prop` + // followed by a generic `.length`", because the generic one is a + // property diamond ending in a runtime call, and a call inside this + // clone deletes it. The emitted read tag-tests the loaded word for + // both string representations and side-exits otherwise + // (`expr::element_shape_reads`). + Expr::PropertyGet { .. } if property == "length" => { + element_shape_loop_pure_expr_collect(ctx, object, scope, out) + } _ => false, }, + // #10199: `arr[].prop ? A : B` with constant arms. JS truthiness + // of an arbitrary value is a runtime question the clone is not allowed + // to guess, so the emitted read admits ONLY the two boolean singletons + // and side-exits on everything else. Anything but a tracked element + // read in the condition, or a non-constant arm, declines. + Expr::Conditional { + condition, + then_expr, + else_expr, + } => { + matches!(condition.as_ref(), Expr::PropertyGet { .. }) + && crate::expr::element_shape_reads::integer_arm(then_expr).is_some() + && crate::expr::element_shape_reads::integer_arm(else_expr).is_some() + && element_shape_loop_pure_expr_collect(ctx, condition, scope, out) + } // 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 @@ -324,6 +383,10 @@ fn element_shape_loop_pure_expr_collect( Expr::LocalGet(id) => { scope.element_binding != Some(*id) && scope.derived.map(|(d, _)| d) != Some(*id) + // #10199: the carried index and its alias are virtual too — + // their real slots are one iteration behind inside the clone, + // so a bare read would hand out a stale value. + && scope.carried.is_none_or(|(c, alias)| *id != c && alias != Some(*id)) && out.array.is_none_or(|a| a != *id) && (*id == scope.accumulator_id || crate::type_analysis::is_numeric_expr(ctx, expr)) } @@ -381,7 +444,18 @@ fn match_index_form(index: &perry_hir::Expr, scope: &PureExprScope) -> Option None, + // #10199: `arr[c]` and `const index = c; arr[index]` are the same + // subscript; the alias is virtual and carries no obligation of its + // own. + _ => match scope.carried { + Some((carried_id, alias_id)) if carried_id == *id || alias_id == Some(*id) => { + Some(MatchedIndex::Carried { + carried_id, + alias_id, + }) + } + _ => None, + }, }, // `0..=i32::MAX`, so the preheader's `length > k` compare is an i32 // one and the emitted index needs no conversion. @@ -619,41 +693,100 @@ fn anon_shape_field_type_is_compatible( /// 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; +pub(super) fn lower_virtual_clone_binding(ctx: &mut FnCtx<'_>, id: u32) -> Result { + enum VirtualBinding { + /// #7771 `const r = arr[j]` and #10199 `const index = c`: pure aliases, + /// no instruction of their own. + Alias, + /// #10123 `const d = j % m`: one `srem i32`. + DerivedMod { + counter_id: u32, + modulus_i32: String, + slot: String, + }, } - let Some((counter_slot, modulus_i32, derived_slot)) = - ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { - let crate::expr::ElementShapeIndex::DerivedMod { + + let Some((scope_id, binding)) = ctx.element_shape_loop_facts.iter().rev().find_map(|fact| { + if fact.element_binding == Some(id) { + return Some((fact.scope_id, VirtualBinding::Alias)); + } + match &fact.index { + crate::expr::ElementShapeIndex::DerivedMod { local_id, modulus_i32, slot, - } = &fact.index - else { - return None; - }; - if *local_id != id { - return None; + } if *local_id == id => Some(( + fact.scope_id, + VirtualBinding::DerivedMod { + counter_id: fact.index_local_id, + modulus_i32: modulus_i32.clone(), + slot: slot.clone(), + }, + )), + crate::expr::ElementShapeIndex::Carried(carried) if carried.alias_id == Some(id) => { + Some((fact.scope_id, VirtualBinding::Alias)) } - Some(( - ctx.i32_counter_slots.get(&fact.index_local_id)?.clone(), - modulus_i32.clone(), - slot.clone(), - )) - }) + _ => None, + } + }) else { + return Ok(false); + }; + + match binding { + VirtualBinding::Alias => {} + VirtualBinding::DerivedMod { + counter_id, + modulus_i32, + slot, + } => { + let Some(counter_slot) = ctx.i32_counter_slots.get(&counter_id).cloned() else { + return Ok(false); + }; + let blk = ctx.block(); + let counter = blk.load(I32, &counter_slot); + let derived = blk.srem(I32, &counter, &modulus_i32); + blk.store(I32, &derived, &slot); + } + } + // #10199: this iteration's index is now in its slot, so this is where the + // shared element deref belongs — for the ONE binding that owns it. + emit_element_prefetch_for(ctx, scope_id, id); + Ok(true) +} + +/// #10199: emit the once-per-iteration element prologue, if `site_local` is the +/// statement the fact designated to own it. +/// +/// Every read in the clone loads the handle this parks, so the designated +/// statement MUST run before any of them — which is why the matcher only ever +/// designates the body's leading virtual statement, and why a fact carrying a +/// prefetch whose index cannot be materialized is a compiler bug rather than a +/// missed optimization (the reads would load an uninitialized alloca). The +/// matcher checks `needs_counter_i32_slot` before the fact is built, so the +/// index is always available here. +pub(super) fn emit_element_prefetch_for(ctx: &mut FnCtx<'_>, scope_id: u32, site_local: u32) { + let Some(fact) = ctx + .element_shape_loop_facts + .iter() + .find(|fact| fact.scope_id == scope_id) + .cloned() else { - return false; + return; + }; + let Some(prefetch) = fact.elem_prefetch.clone() else { + return; }; - 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 + if prefetch.site_local_id != site_local { + return; + } + let idx_i32 = crate::expr::element_shape_guard::emit_element_shape_index(ctx, &fact) + .expect("a prefetching fact always has a materializable index"); + crate::expr::element_shape_guard::emit_element_shape_prefetch( + ctx, + &fact, + &idx_i32, + &prefetch.handle_slot, + ); } /// Is `array_id` an untyped receiver — the shape-keyed arm's entry condition? @@ -799,7 +932,7 @@ fn match_element_shape_versioned_loop( return None; } - // Store-free body, in one of three admitted shapes (see the module docs): + // Store-free body, in one of four admitted shapes (see the module docs): // // 1. `acc = ].field>` — the original // single statement; @@ -807,89 +940,156 @@ fn match_element_shape_versioned_loop( // 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. + // parsed record array is written in; + // 4. #10199's loop-carried form, `c = (a*c + b) % m;` optionally followed + // by `const index = c;`. // - // 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. + // In 2, 3 and 4 the leading statement is VIRTUAL inside the fast clone: it + // emits nothing (form 2), one `srem i32` (form 3) or the i64 recurrence + // (form 4) rather than the generic lowering (`stmt/let_stmt.rs`, + // `stmt/element_shape_carried.rs`), so the revocation argument (no store, + // no call in the clone) is unchanged. The bindings are `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. Form 4's `c` is the one mutable exception, and it pays for it with + // a write-back (see `element_shape_carried`). // + // #10199 also admits K >= 1 accumulator statements instead of exactly one, + // which is what `sum += rows[i].id; sum += rows[i].name.length; …` needs. // NOTHING else is admitted. 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(binding_init), - .. - }, Stmt::Expr(Expr::LocalSet(acc_id, value))] => { - // 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; + let mut carried: Option = None; + let mut carried_alias: Option = None; + let mut rest: &[Stmt] = body; + + // (4) The recurrence, which must be the body's FIRST statement: the index + // it produces is read by every statement after it, and a read BEFORE it + // would see a value the preheader bounded but the commit protocol does not + // describe. + if let Some(first) = rest.first() { + if let Some(matched) = super::element_shape_carried::match_carried_update( + ctx, first, counter_id, condition?, update, body, + ) { + carried = Some(matched); + rest = &rest[1..]; + } + } + + // (2)/(3)/(4-alias) The leading virtual binding. + if let Some(Stmt::Let { + id, + mutable: false, + init: Some(binding_init), + .. + }) = rest.first() + { + // The binding must be a plain, loop-owned const local in every 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; + } + match binding_init { + Expr::IndexGet { object, index } if carried.is_none() => { + 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)); } - 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, + } if carried.is_none() => { + 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; } - // #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)); + if !modulus_local_is_admissible(ctx, *modulus_id, condition?, update, body) { + return None; } - _ => return None, + derived = Some((*id, *modulus_id)); } - (acc_id, value) + // #10199: `const index = c` after the recurrence. A pure alias, so + // it emits nothing at all — the subscript resolves to the carried + // slot either way. + Expr::LocalGet(aliased) => { + let matched = carried.as_ref()?; + if *aliased != matched.carried_id || *id == matched.modulus_id { + return None; + } + carried_alias = Some(*id); + } + _ => return None, } - _ => return None, - }; + rest = &rest[1..]; + } + + // (1) One or more accumulator statements over the SAME local. + let mut acc_id: Option<&u32> = None; + let mut values: Vec<&perry_hir::Expr> = Vec::new(); + for stmt in rest { + let Stmt::Expr(Expr::LocalSet(id, value)) = stmt else { + return None; + }; + match acc_id { + None => acc_id = Some(id), + Some(seen) if seen == id => {} + Some(_) => return None, + } + values.push(value.as_ref()); + } + let acc_id = acc_id?; if *acc_id == counter_id || !ctx.locals.contains_key(acc_id) || ctx.boxed_vars.contains(acc_id) || ctx.module_globals.contains_key(acc_id) + || carried.is_some_and(|c| c.carried_id == *acc_id || c.modulus_id == *acc_id) { return None; } + // #10199: statements 2..K are folded into statement 1 for the fast clone + // (`acc = a; acc = acc + b` ≡ `acc = (a) + b`), so that the WHOLE iteration + // commits once, after every side exit it can take. Each of them must + // therefore be `acc ` with `acc` as the left operand — the shape + // every compound assignment lowers to — and the right operand must not read + // `acc`, or substituting it would make it see the pre-statement value. + for value in &values[1..] { + let Expr::Binary { left, right, .. } = value else { + return None; + }; + if !matches!(left.as_ref(), Expr::LocalGet(id) if id == acc_id) { + return None; + } + if super::element_shape_carried::expr_reads_local(right, *acc_id) { + 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 scope = PureExprScope { @@ -897,15 +1097,22 @@ fn match_element_shape_versioned_loop( accumulator_id: *acc_id, element_binding: element_binding.map(|(id, _)| id), derived, + carried: carried.map(|c| (c.carried_id, carried_alias)), }; - if scope.element_binding == Some(*acc_id) || derived.map(|(d, _)| d) == Some(*acc_id) { + if scope.element_binding == Some(*acc_id) + || derived.map(|(d, _)| d) == Some(*acc_id) + || carried_alias == Some(*acc_id) + { return None; } 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) { + if !values + .iter() + .all(|value| element_shape_loop_pure_expr_collect(ctx, value, &scope, &mut facts)) + { return None; } let array_id = facts.array?; @@ -927,12 +1134,35 @@ fn match_element_shape_versioned_loop( return None; } } + // #10199: a matched recurrence whose value nothing subscripts with would + // still have its `LocalSet` lowered generically — a `frem` libcall inside + // the clone, which deletes it. Decline instead, so the loop keeps whatever + // lowering it has today. + match (carried.as_ref(), index) { + (Some(matched), MatchedIndex::Carried { .. }) => { + if matched.carried_id == array_id + || matched.modulus_id == array_id + || carried_alias == Some(array_id) + { + return None; + } + } + // A recurrence the subscripts do not use, or a carried subscript with + // no recurrence (which the walk's scope makes unreachable). + (Some(_), _) | (None, MatchedIndex::Carried { .. }) => return None, + (None, _) => {} + } match bound { ElementShapeLoopBound::Local(bound_id) => { if bound_id == array_id || bound_id == *acc_id || Some(bound_id) == scope.element_binding || derived.map(|(d, _)| d) == Some(bound_id) + // The bound is materialized ONCE in the preheader, so a bound + // the recurrence rewrites every iteration would be stale. The + // carried local is also not readable bare inside the clone. + || carried.is_some_and(|c| c.carried_id == bound_id) + || carried_alias == Some(bound_id) { return None; } @@ -975,12 +1205,6 @@ fn match_element_shape_versioned_loop( return None; } - 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 @@ -999,6 +1223,42 @@ fn match_element_shape_versioned_loop( None => return None, }; + // The property denylist, which is arm-specific (#10199). + // + // The CLASS arm's list exists because its read bakes in a compile-time + // packed slot index while the surrounding lowering may route the name + // somewhere else entirely (`length` header loads, `errors` runtime calls, + // accessor-ish names) — a name collision there costs the call-free + // guarantee. + // + // The SHAPE arm bakes in nothing: the preheader asks the runtime for the + // inline slot of that exact key in that exact ordinary ShapeId and declines + // on `-1`, and the per-element residual pins `obj_type == GC_TYPE_OBJECT` + // with no per-object descriptors. Every receiver kind whose builtin branch + // could answer a name differently — a function's `name`, an array's + // `length`, a Map's `size`, an AggregateError's `errors` — fails that + // residual, and for a plain record an own data property SHADOWS the + // prototype name it collides with, which is exactly what the inline slot + // holds. The whole list would otherwise have cost the benchmark its + // `fields` shape outright, for a field literally called `name`. + // + // `__proto__` stays denied, and not because of JavaScript: node gives + // `JSON.parse('{"__proto__":1}')` an OWN `__proto__` data property and + // reads `1` back from it, so the inline slot would be right. It is denied + // because Perry's generic property path may special-case the name ahead of + // own-property lookup, and the clone must agree with the clone-of, not with + // the spec, wherever those two differ. + const SHAPE_PROP_DENYLIST: &[&str] = &["__proto__"]; + let denylist = match identity { + ElementShapeIdentity::Class { .. } => CLASS_FIELD_LOOP_PROP_DENYLIST, + ElementShapeIdentity::Shape => SHAPE_PROP_DENYLIST, + }; + for prop in &facts.props { + if denylist.contains(&prop.as_str()) { + 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 @@ -1018,6 +1278,40 @@ fn match_element_shape_versioned_loop( return None; } + // #10199: the fast clone's body, when the two rewrites apply. `lead` is + // whatever statements the walk consumed ahead of the accumulator run (the + // recurrence and/or the virtual binding), kept verbatim. + let lead = &body[..body.len() - rest.len()]; + let fast_body = (values.len() > 1 || carried.is_some()).then(|| { + let mut stmts: Vec = lead.to_vec(); + // `acc = v1; acc = acc p2; acc = acc p3` + // ≡ `acc = ((v1) p2) p3` + // — the same operations on the same operands in the same order, which + // is what keeps the folded float arithmetic bit-identical. + let mut folded = values[0].clone(); + for value in &values[1..] { + let Expr::Binary { op, right, .. } = value else { + unreachable!("the accumulator fold validated every statement's shape"); + }; + folded = Expr::Binary { + op: *op, + left: Box::new(folded), + right: right.clone(), + }; + } + stmts.push(Stmt::Expr(Expr::LocalSet(*acc_id, Box::new(folded)))); + if let Some(matched) = carried.as_ref() { + // The commit marker (`stmt/element_shape_carried.rs`). It is the + // LAST statement, so it runs only once the iteration is past every + // exit it could take. + stmts.push(Stmt::Expr(Expr::LocalSet( + matched.carried_id, + Box::new(Expr::LocalGet(matched.carried_id)), + ))); + } + stmts + }); + Some(ElementShapeVersionedLoop { counter_id, bound, @@ -1027,6 +1321,8 @@ fn match_element_shape_versioned_loop( index, element_binding: scope.element_binding, accumulator_id: *acc_id, + carried, + fast_body, }) } @@ -1200,10 +1496,25 @@ pub(super) fn lower_element_shape_versioned_for( // 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( + // + // #10199's carried recurrence takes the same obligation for the same + // reason, and one more of its own: its ENTRY value must be a non-negative + // integral i32, because the whole i64 recurrence bound + // (`a * i32::MAX + b < 2^53`, and a non-negative dividend for `srem` to + // agree with JS `%`) is stated over that range. A fractional, negative or + // non-number `cursor` at loop entry is a slow-clone case. + let modulus_i32: Option = match (&matched.index, &matched.carried) { + (MatchedIndex::DerivedMod { modulus_id, .. }, _) => Some(materialize_loop_i32( + ctx, + *modulus_id, + 1, + i32::MAX, + &slow_pre_label, + "element_shape.loop.modulus", + )?), + (MatchedIndex::Carried { .. }, Some(carried)) => Some(materialize_loop_i32( ctx, - modulus_id, + carried.modulus_id, 1, i32::MAX, &slow_pre_label, @@ -1211,6 +1522,25 @@ pub(super) fn lower_element_shape_versioned_for( )?), _ => None, }; + let carried_slot: Option = match &matched.carried { + Some(carried) => { + let entry = materialize_loop_i32( + ctx, + carried.carried_id, + 0, + i32::MAX, + &slow_pre_label, + "element_shape.loop.carried", + )?; + // Entry-block alloca: LLVM lowers a non-entry `alloca` as a real + // stack bump with no restore, and this one is written every + // iteration. + let slot = ctx.func.alloca_entry(I32); + ctx.block().store(I32, &entry, &slot); + Some(slot) + } + None => None, + }; let trip_count = match &materialized_bound { Some(bound) => { @@ -1225,11 +1555,14 @@ pub(super) fn lower_element_shape_versioned_for( (MatchedIndex::Constant(k), _) => { crate::expr::element_shape_guard::ElementShapeIndexBound::Constant(*k) } - (MatchedIndex::DerivedMod { .. }, Some(modulus)) => { + // #10199: the recurrence's result is `srem` of a non-negative dividend + // by `m`, so it lands in `[0, m)` exactly like the derived index and + // takes exactly the same preheader obligation, `m <= length`. + (MatchedIndex::DerivedMod { .. } | MatchedIndex::Carried { .. }, Some(modulus)) => { crate::expr::element_shape_guard::ElementShapeIndexBound::Modulus(modulus.as_str()) } - (MatchedIndex::DerivedMod { .. }, None) => { - unreachable!("a derived index always materializes its modulus") + (MatchedIndex::DerivedMod { .. } | MatchedIndex::Carried { .. }, None) => { + unreachable!("a modulo-derived index always materializes its modulus") } }; let expected_class_id_str = match &matched.identity { @@ -1323,8 +1656,58 @@ pub(super) fn lower_element_shape_versioned_for( // iteration. slot: ctx.func.alloca_entry(I32), }, + MatchedIndex::Carried { + carried_id, + alias_id, + } => { + let carried = matched + .carried + .as_ref() + .expect("a carried index always matches a recurrence"); + crate::expr::ElementShapeIndex::Carried(Box::new(crate::expr::CarriedIndex { + local_id: carried_id, + alias_id, + coeff_a: carried.coeff_a, + coeff_b: carried.coeff_b, + modulus_i32: modulus_i32 + .clone() + .expect("a carried index always materializes its modulus"), + slot: carried_slot + .clone() + .expect("a carried index always materializes its entry value"), + commit_slot: ctx + .locals + .get(&carried_id) + .cloned() + .expect("the matcher required a directly-addressable local"), + })) + } }; + // #10199: the shared once-per-iteration element deref, hung off the body's + // leading virtual statement. Shape-keyed only: the class-keyed arm's reads + // are raw doubles behind a `GC_OBJ_TYPED_LAYOUT_INTACT` residual whose + // emitted IR #7480's census pins, and nothing in #10199 measures it. + let elem_prefetch = shape_keyed + .then(|| { + let site_local_id = match (&fact_index, matched.element_binding) { + (_, Some(binding)) => Some(binding), + (crate::expr::ElementShapeIndex::DerivedMod { local_id, .. }, _) => Some(*local_id), + // With an alias the prologue belongs to the alias `Let`, which + // runs after the recurrence; without one it belongs to the + // recurrence itself. Exactly one statement owns it either way. + (crate::expr::ElementShapeIndex::Carried(carried), _) => { + Some(carried.alias_id.unwrap_or(carried.local_id)) + } + _ => None, + }?; + Some(crate::expr::ElementPrefetch { + site_local_id, + handle_slot: ctx.func.alloca_entry(I64), + }) + }) + .flatten(); + let scope_id = ctx.next_loop_proof_scope_id(); let fast_scan_start = ctx.func.num_blocks(); ctx.current_block = fast_pre_idx; @@ -1333,6 +1716,7 @@ pub(super) fn lower_element_shape_versioned_for( array_local_id: matched.array_id, index_local_id: matched.counter_id, index: fact_index, + elem_prefetch, shape_keyed, scope_id, class_name: report_class, @@ -1341,6 +1725,7 @@ pub(super) fn lower_element_shape_versioned_for( side_exit_label: slow_pre_label.clone(), statically_layout_proven, fields, + synthesized_body: matched.fast_body.is_some(), element_binding: matched.element_binding, numeric_accumulator: matched.accumulator_id, }); @@ -1349,7 +1734,7 @@ pub(super) fn lower_element_shape_versioned_for( init, condition, update, - body, + matched.fast_body.as_deref().unwrap_or(body), "for.element_shape_fast", Some((matched.counter_id, guard.bound_i32)), ); 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 c78cfc72e5..488d64aa4c 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -402,14 +402,34 @@ fn assert_fast_clone_is_entered(ir: &str) { fn fast_clone_slice(ir: &str) -> String { let mut owned = String::new(); let mut in_fast_block = false; + // #10199: the emitted text carries the function TWICE (same block labels), + // so every block would be collected twice and any `matches().count()` + // assertion against the slice would read double. Stop at the first repeated + // label, which is where the second copy begins — `contains` assertions are + // unaffected, and counting one iteration of the clone is what makes "the + // residual check happens ONCE" expressible at all. + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); for line in ir.split_inclusive('\n') { let trimmed = line.trim_end(); // A block DEFINITION starts at column 0 and ends in `:`; anything else // belongs to whichever block was last opened. if !line.starts_with(char::is_whitespace) && trimmed.ends_with(':') { + // #10199 added three more: the string-`.length` decode + // (`element_shape.strlen*`), the boolean ternary's admitted arm + // (`element_shape.bool`), and nothing for the shared prefetch, + // which reuses `element_shape.load`. Every block the clone can + // execute must be listed, or the negatives below go vacuous for it. in_fast_block = trimmed.starts_with("for.element_shape_fast.") || trimmed.starts_with("element_shape.load") - || trimmed.starts_with("element_shape.number"); + || trimmed.starts_with("element_shape.number") + || trimmed.starts_with("element_shape.strlen") + || trimmed.starts_with("element_shape.bool"); + // Only a repeated CLONE label means the second copy: unrelated + // functions share ordinary labels (`entry:`), and breaking on one + // of those would slice away the clone entirely. + if in_fast_block && !seen.insert(trimmed.to_string()) { + break; + } } if in_fast_block { owned.push_str(line); 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 index 94ade02cc7..fe47ad0e89 100644 --- a/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs @@ -450,21 +450,78 @@ fn an_untracked_local_index_declines() { } #[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. +fn the_shape_keyed_arm_denies_only_proto() { + // #10199 narrowed this list. The CLASS arm still denies every name with a + // dedicated branch in the property dispatch, because its read bakes in a + // compile-time packed slot. The SHAPE arm bakes in nothing: the preheader + // asks the runtime for that exact key's inline slot in that exact ordinary + // ShapeId and declines on `-1`, and the residual pins + // `obj_type == GC_TYPE_OBJECT` with no per-object descriptors — so every + // receiver whose builtin branch could answer a name differently (a + // function's `name`, an array's `length`, a Map's `size`) is already + // excluded, and for a plain record an own data property shadows the + // prototype name it collides with. Denying the whole list cost the access + // benchmark its `fields` shape outright, for a field called `name`. + for property in ["length", "name", "constructor", "size", "message"] { + let ir = emit(&untyped_param_module( + Type::Any, + Type::Number, + vec![untyped_accumulate(untyped_elem_field( + Expr::LocalGet(U_COUNTER_ID), + property, + ))], + )); + assert!( + ir.contains("element_shape.loop.fast.preheader"), + "`{property}` is an ordinary own inline slot on a parsed record; \ + the shape-keyed arm must serve it" + ); + } + // `__proto__` stays denied — not because of JavaScript (node gives + // `JSON.parse('{\"__proto__\":1}')` an own data property and reads `1` + // back), but because Perry's generic property path may special-case the + // name ahead of own-property lookup, and the clone must agree with the + // path it is a clone of. let ir = emit(&untyped_param_module( Type::Any, Type::Number, vec![untyped_accumulate(untyped_elem_field( Expr::LocalGet(U_COUNTER_ID), - "length", + "__proto__", + ))], + )); + assert!( + !ir.contains("element_shape.loop.fast.preheader"), + "`__proto__` must decline the shape-keyed clone" + ); +} + +#[test] +fn the_class_keyed_arm_keeps_the_full_property_denylist() { + // The narrowing above is shape-arm-only: a class-keyed read bakes in a + // packed slot index while the surrounding lowering may route the name + // somewhere else entirely, which is what the list has always protected. + let ir = emit(&element_shape_module( + vec![Stmt::Expr(Expr::LocalSet( + SUM_ID, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(SUM_ID)), + right: Box::new(Expr::PropertyGet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ARRAY_ID)), + index: Box::new(Expr::LocalGet(COUNTER_ID)), + }), + property: "length".to_string(), + byte_offset: 0, + }), + }), ))], + None, )); assert!( !ir.contains("element_shape.loop.fast.preheader"), - "a denylisted property must decline the clone" + "a denylisted property must still decline the CLASS-keyed clone" ); } @@ -489,3 +546,9 @@ fn a_declared_but_unresolvable_element_type_still_declines() { "an unresolvable declared element type must decline both arms" ); } + +/// #10199's `fields` and `random` shapes — a child module so it inherits both +/// this file's shape-keyed assertions and `element_shape_loop_tests`'s slicing +/// helpers. +#[path = "element_shape_fields_random_tests.rs"] +mod fields_random; diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index 28ec377674..288e799086 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -31,7 +31,7 @@ pub(crate) fn lower_let( // 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) { + if super::element_shape_loop::lower_virtual_clone_binding(ctx, id)? { return Ok(()); } // `let C = SomeClass` aliases the local `C` to the class diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index cc407d30c0..5b9ac4f33e 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -20,6 +20,7 @@ mod cached_field_index_return; #[cfg(test)] mod class_field_loop_tests; mod counter_range; +mod element_shape_carried; mod element_shape_loop; #[cfg(test)] mod element_shape_loop_tests; @@ -281,6 +282,15 @@ fn lower_return_expr(ctx: &mut FnCtx<'_>, expr: &perry_hir::Expr) -> Result, stmt: &Stmt) -> Result<()> { match stmt { Stmt::Expr(e) => { + // #10199: the element-shape fast clone's carried-index statements + // (the recurrence and its trailing write-back) are lowered + // VIRTUALLY, exactly like the `Let` bindings in `let_stmt.rs` — + // the generic lowering of `c = (c * 17 + 7) % length` is an + // `frem` libcall, and a call inside the clone DELETES it (#7690). + // Outside a clone the fact vector is empty and this is a no-op. + if element_shape_carried::lower_virtual_carried_stmt(ctx, e)? { + return Ok(()); + } let prev_discard = ctx.discard_expr_value; ctx.discard_expr_value = true; // #7590: the non-leaking companion. `lower_expr` takes this at the diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index e3677034e2..e3c20605f2 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -356,6 +356,18 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { Expr::Logical { left, right, .. } => { is_numeric_expr(ctx, left) && is_numeric_expr(ctx, right) } + // #10199: `arr[i]. ? 1 : 0` inside an element-shape fast + // clone. Deliberately NOT the general "both arms are numeric" rule — a + // general ternary's CONDITION is an arbitrary JS truthiness test, which + // is a runtime call; this arm is true only for the exact shape the + // clone lowers itself, where the condition is a tracked element read + // admitted as one of the two boolean singletons and everything else + // side-exits. + Expr::Conditional { .. } + if crate::expr::element_shape_reads::cloned_bool_select_read(ctx, e).is_some() => + { + true + } // `obj.field` where the field is declared as `number` on the // owning class. Without this, `this.value + 1` in a hot loop // wraps the field load in `js_number_coerce` which prevents @@ -374,6 +386,17 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { if property == "length" && expression_has_numeric_length(ctx, object) { return true; } + // #10199: `arr[i]..length` inside an element-shape + // fast clone. The receiver is an untyped element read, so the + // declared-type answer above cannot see a string there; the clone's + // own lowering tag-tests the loaded word for both string + // representations and side-exits when it is not one, which is a + // stronger proof than an annotation. Without this the `+` consuming + // it bails to `js_dynamic_string_or_number_add` and the call + // deletes the clone. + if crate::expr::element_shape_reads::cloned_string_length_read(ctx, e).is_some() { + 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 class-keyed // arm gets that from the residual check's From bf0904a9385a3e6f0b6110f926e783df2916f6f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 12:50:11 +0200 Subject: [PATCH 2/4] test(gap): cover the carried index, the accumulator fold and the two new reads 40 cases in test_gap_json_record_loop_clone.ts, all byte-identical to node 26.5.1: the recurrence with and without its alias, the carried value read after the loop, negative and fractional entry values, a zero and an over-long modulus, a multiplier past the exact-double range, a mid-loop side exit whose sum AND final cursor observe the write-back protocol, SSO and heap names, a non-string and a null name, five non-boolean `active` values, a two-statement fold whose second read side-exits, and records carrying own `name`/`length`/ `size` properties. Plus the changelog fragment with the measured 12-cell table and the instruction counts. --- .../10185-element-shape-fields-random.md | 157 +++++++++ crates/perry-codegen/src/expr/binary.rs | 2 +- .../src/expr/element_shape_guard.rs | 6 +- .../src/expr/element_shape_reads.rs | 2 +- crates/perry-codegen/src/expr/mod.rs | 14 +- crates/perry-codegen/src/expr/shadow_slot.rs | 4 +- .../src/stmt/element_shape_carried.rs | 2 +- .../stmt/element_shape_fields_random_tests.rs | 6 +- .../src/stmt/element_shape_loop.rs | 44 +-- .../src/stmt/element_shape_loop_tests.rs | 4 +- .../stmt/element_shape_shape_keyed_tests.rs | 4 +- crates/perry-codegen/src/stmt/mod.rs | 2 +- .../src/type_analysis/numeric.rs | 4 +- test-files/test_gap_json_record_loop_clone.ts | 298 ++++++++++++++++++ 14 files changed, 502 insertions(+), 47 deletions(-) create mode 100644 changelog.d/10185-element-shape-fields-random.md diff --git a/changelog.d/10185-element-shape-fields-random.md b/changelog.d/10185-element-shape-fields-random.md new file mode 100644 index 0000000000..788c133fdf --- /dev/null +++ b/changelog.d/10185-element-shape-fields-random.md @@ -0,0 +1,157 @@ +Serve the JSON access benchmark's `fields` and `random` shapes from the +element-shape versioned loop clone (#7480 / #5093 / #10171). Stacked on +#10171, which made the clone fire for `JSON.parse`'d record arrays at all. + +#10171 took the benchmark's `repeat` and `sequential` modes to parity. The two +remaining modes got nothing, for three separate reasons — and because a loop is +admitted as a whole, any one of them cost the entire clone: + +```ts +// random: a loop-carried index +cursor = (cursor * 17 + 7) % length; +const index = cursor; +sum += rows[index].id; + +// fields: three accumulator statements, one string read, one boolean ternary +const index = i % length; +sum += rows[index].id; +sum += rows[index].name.length; +sum += rows[index].active ? 1 : 0; +``` + +* **A loop-carried index** (`stmt/element_shape_carried.rs`). The recurrence is + folded to one affine pair `(a, b)` and evaluated as `srem i64` — the generic + `%` is an `frem`, which on aarch64 is a libm call, and a call inside this + clone does not slow it down, it DELETES it (#7690). Three obligations, all + discharged once in the preheader: the modulus materializes as an i32 in + `1..=i32::MAX` (so `m <= length` puts every index in bounds with no per-read + test), the carried binding's ENTRY value materializes as a non-negative + integral i32, and `a`/`b` are required non-negative so the dividend cannot go + negative — JS `%` returns a *negative* remainder for a negative dividend, + which is an out-of-bounds subscript rather than a slow path. + + The matcher also tracks the largest magnitude any sub-expression of the + recurrence can reach and declines above 2^53. "Fits i64" is necessary and NOT + sufficient: JavaScript evaluates the recurrence in doubles, and + `cursor * 1e9 + 1` fits i64 comfortably while losing its low bits in the f64 + the program actually runs. The bound is tracked per NODE rather than taken + from the folded pair, because folding cancels: `c * 1000 - c * 999` is + `1 * c`, but JavaScript still evaluates both thousand-fold products. + +* **Where the carried write-back goes is a correctness question, not a + scheduling one.** The residual side exit resumes the CURRENT iteration in the + slow clone, which re-runs the whole body — the recurrence included. So the + commit to the real binding is the LAST statement of the iteration: a + mid-iteration exit then leaves the binding holding that iteration's entry + value and the slow clone advances it exactly once. A write-back at the update + site double-applies it, and every later index is silently a different — still + in-bounds, still a valid record — one. + +* **K accumulator statements**, folded into one for the fast clone + (`acc = a; acc = acc + b` ≡ `acc = (a) + b`: the same operations on the same + operands in the same order, so the float result is bit-identical). Same + reason as above — the whole iteration must commit once, past every exit it + can take, or a tag test that fails on the third read leaves the first two + already applied when the slow clone re-runs it. Statements 2..K must read the + accumulator as the LEFT operand and must not read it in the addend, or the + substitution would change which value the addend sees. + +* **`arr[i].prop.length` and `arr[i].prop ? A : B`** + (`expr/element_shape_reads.rs`). The preheader proves which inline slot holds + a property; it proves nothing about what is in it. So each read tag-tests the + loaded word and side-exits when the answer is not the representation it is + about to assume: both string forms for `.length` (heap `utf16_len`, and the + SSO length byte — the same decode the runtime's own `.length` arms use), and + the two boolean singletons by exact NaN-box bit pattern for the ternary. + JS truthiness of `0` / `""` / `null` / an object is a runtime question the + clone does not guess, and a non-constant ternary arm is not admitted at all. + +* **One residual check per iteration, not one per read.** Three reads of + `rows[index]` were three element loads and three header/ShapeId checks. The + body's leading virtual binding now emits the deref and the residual once and + parks the masked handle in an entry alloca. That is also what makes the + multi-read side exit correct: every residual exit now precedes every store. + +* **The property denylist is arm-specific now.** The class-keyed arm keeps the + full list — its read bakes in a compile-time packed slot index while the + surrounding lowering may route the name elsewhere. The shape-keyed arm bakes + in nothing: it asks the runtime for that exact key's inline slot in that exact + ordinary ShapeId and declines on `-1`, and the residual pins + `obj_type == GC_TYPE_OBJECT` with no per-object descriptors — so every + receiver whose builtin branch could answer a name differently (a function's + `name`, an array's `length`, a Map's `size`) is already excluded, and for a + plain record an own data property SHADOWS the prototype name it collides + with. Only `__proto__` stays denied, and not because of JavaScript (node + gives `JSON.parse('{"__proto__":1}')` an own data property and reads `1` back + from it): Perry's generic property path may special-case the name ahead of + own-property lookup, and the clone must agree with the path it is a clone of + wherever the two differ. The full list would otherwise have cost the + benchmark its `fields` mode outright, for a field literally called `name`. + +**Measured** (ns/iter, 1M iterations, 5 interleaved rounds, best of 5, +`PERRY_NO_AUTO_OPTIMIZE=1`, on a loaded shared host; `new` and `base` are both +built from this worktree, `base` at #10171's head `2b77e7d4fe`): + +| cell | base (#10171) | new | node 26.5.1 | bun 1.3.14 | new / best | +|---|---:|---:|---:|---:|---:| +| 16k `repeat` | 4.15 | 4.19 | 3.24 | 5.03 | 1.293 | +| 16k `sequential` | 4.21 | 4.17 | 4.35 | 6.14 | 0.959 | +| 16k `random` | 15.38 | **4.99** | 7.10 | 9.61 | **0.703** | +| 16k `fields` | 25.76 | **6.19** | 5.75 | 8.35 | **1.077** | +| 1m `repeat` | 4.12 | 4.19 | 3.57 | 5.25 | 1.174 | +| 1m `sequential` | 4.24 | 4.18 | 9.35 | 8.06 | 0.519 | +| 1m `random` | 15.90 | **5.25** | 12.66 | 11.66 | **0.450** | +| 1m `fields` | 29.02 | **6.31** | 12.68 | 15.41 | **0.498** | +| 20m `repeat` | 4.19 | 4.17 | 3.52 | 5.13 | 1.185 | +| 20m `sequential` | 4.29 | 4.37 | 7.48 | 8.83 | 0.584 | +| 20m `random` | 27.56 | **5.60** | 9.45 | 10.64 | **0.593** | +| 20m `fields` | 27.26 | **6.22** | 11.53 | 14.93 | **0.539** | + +The two targeted shapes move 3.1x-4.9x and land below the better of node and +bun on five of their six cells. The sixth, 16k `fields`, is the one that does +not reach parity: 6.08 against node's 5.83 on a 12-round re-measure (1.043x). +That is the smallest fixture, where the whole array is in L1 and the guard +overhead is the only thing left — node's own `fields` is just 1.34x its +`sequential` there, because its inline caches have type feedback proving `name` +is a string and `active` a boolean, while this clone tag-tests both on every +read. Paying that test is what lets it side-exit instead of deoptimize, so it +is a floor of the design rather than a missing peephole. Reported rather than +rounded off. + +`repeat` and `sequential`, which #10171 already served, are unchanged within +noise in both wall clock and instruction count — `sequential`'s single read has +nothing to share, and `repeat`'s constant index has no leading binding to hang +the prologue off. + +Instructions retired per iteration on the 16k fixture (2M iterations minus a +0-iteration run, `/usr/bin/time -l`): + +| shape | base | new | +|---|---:|---:| +| random | 207.9 | 29.1 | +| fields | 466.6 | 47.0 | +| sequential | 25.1 | 25.1 | +| repeat | 16.1 | 15.8 | + +`random` lands one recurrence above `sequential`'s 25, and `fields` two extra +guarded reads plus a string decode and a select above it — which is what the +shared residual buys, and the shape the numbers have to have if the clone is +really being entered rather than merely emitted (#10171's own lesson: a clone +can be `cond_br`-entered and never executed while every IR-census assertion +passes, and only the wall clock notices). + +Validation: `test-files/test_gap_json_record_loop_clone.ts` gains 40 cases — +the recurrence with and without its alias, the carried value read after the +loop, negative and fractional entry values, a zero and an over-long modulus, a +multiplier past the exact-double range, a mid-loop side exit whose sum AND +final cursor both observe the write-back protocol, SSO and heap `name`s, a +non-string and a null `name`, five non-boolean `active` values, a two-statement +fold whose second read side-exits, and records with own `name`/`length`/`size` +properties — all byte-identical to `node --experimental-strip-types` 26.5.1. +70 codegen IR-census tests (`element_shape_fields_random_tests.rs` and its +siblings), every positive paired with a sabotage case asserting the clone is +ABSENT. Seeded GC stress on the gap binary +(`PERRY_GC_SCHEDULE_SEED=1..4 PERRY_GC_SCHEDULE_RATE=0.2 +PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=32`): output +identical across all four seeds and to node, with 3–11 copying minors and +~14.4k moved objects per run, so the instrument's subject was live. diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index ce24554dc7..ebeac06c0f 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -504,7 +504,7 @@ fn reduction_add_is_reassociable(ctx: &FnCtx<'_>, left: &Expr, right: &Expr) -> } fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String, bool)> { - // #10199: the element-shape fast clone's two NON-numeric reads — + // #10185: the element-shape fast clone's two NON-numeric reads — // `arr[i].name.length` and `arr[i].active ? 1 : 0`. Both are numbers by the // time they get here, each proven from the loaded word's own NaN-box tag // with a side exit to the slow clone on a miss, so the `true` suppresses diff --git a/crates/perry-codegen/src/expr/element_shape_guard.rs b/crates/perry-codegen/src/expr/element_shape_guard.rs index 7b199c3951..a8565b21b3 100644 --- a/crates/perry-codegen/src/expr/element_shape_guard.rs +++ b/crates/perry-codegen/src/expr/element_shape_guard.rs @@ -563,7 +563,7 @@ pub(crate) fn emit_element_shape_index( let slot = slot.clone(); Some(ctx.block().load(I32, &slot)) } - // #10199: the carried recurrence's private i32 slot, written by the + // #10185: the carried recurrence's private i32 slot, written by the // body's first statement earlier in this same iteration. The REAL // binding slot is deliberately NOT read here — it is one iteration // behind until the trailing write-back commits, which is exactly what @@ -642,14 +642,14 @@ pub(crate) fn emit_element_deref_with_residual( let ok = blk.and(I1, &hdr_ok, &shape_ok); // The side exit resumes the CURRENT iteration in the slow clone; no effect - // of this iteration has committed yet (#10199: with a prefetch this is the + // of this iteration has committed yet (#10185: with a prefetch this is the // ONLY residual exit in the iteration, and it precedes every store). blk.cond_br(&ok, &load_label, &fact.side_exit_label); ctx.current_block = load_idx; elem_handle } -/// #10199: the once-per-iteration element prologue. +/// #10185: the once-per-iteration element prologue. /// /// Emitted by the body's leading virtual binding (`stmt/let_stmt.rs` → /// `stmt/element_shape_loop::lower_virtual_clone_binding`) once the index for diff --git a/crates/perry-codegen/src/expr/element_shape_reads.rs b/crates/perry-codegen/src/expr/element_shape_reads.rs index ce207ae373..9d78d32174 100644 --- a/crates/perry-codegen/src/expr/element_shape_reads.rs +++ b/crates/perry-codegen/src/expr/element_shape_reads.rs @@ -1,4 +1,4 @@ -//! #10199: the two NON-numeric reads the element-shape fast clone admits — +//! #10185: the two NON-numeric reads the element-shape fast clone admits — //! `arr[i].prop.length` on a string field and `arr[i].prop ? A : B` on a //! boolean one. //! diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 6f45f0795e..5af15820f4 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2134,7 +2134,7 @@ pub(crate) enum ElementShapeIndex { /// Entry-block i32 alloca the clone writes the derived index to. slot: String, }, - /// #10199: a LOOP-CARRIED index — `let c = …;` outside the loop, + /// #10185: a LOOP-CARRIED index — `let c = …;` outside the loop, /// `c = (a*c + b) % m;` as the body's first statement, then `arr[c]` (or /// `const index = c; arr[index]`). The benchmark's `random` mode, and the /// shape every pseudo-random walk over a record array is written in. @@ -2149,7 +2149,7 @@ pub(crate) enum ElementShapeIndex { Carried(Box), } -/// #10199: everything [`ElementShapeIndex::Carried`] needs, boxed to keep the +/// #10185: everything [`ElementShapeIndex::Carried`] needs, boxed to keep the /// enum small. #[derive(Debug, Clone)] pub(crate) struct CarriedIndex { @@ -2172,7 +2172,7 @@ pub(crate) struct CarriedIndex { pub commit_slot: String, } -/// #10199: the per-iteration element prefetch. +/// #10185: the per-iteration element prefetch. /// /// `rows[index].id + rows[index].name.length + (rows[index].active ? 1 : 0)` /// reads ONE element three times. Without this each read repeats the element @@ -2218,7 +2218,7 @@ impl ElementShapeIndex { ) } - /// #10199: the locals whose `Let` / `LocalSet` the fast clone lowers + /// #10185: the locals whose `Let` / `LocalSet` the fast clone lowers /// VIRTUALLY for this index form. Nothing may read one of them bare inside /// the clone, and their shadow slots must not be cleared there. pub(crate) fn virtual_locals(&self) -> impl Iterator + '_ { @@ -2275,7 +2275,7 @@ pub(crate) struct ElementShapeLoopFact { pub index_local_id: u32, /// #10123: how the clone computes the element index. pub index: ElementShapeIndex, - /// #10199: the shared per-iteration element deref, when the body has a + /// #10185: the shared per-iteration element deref, when the body has a /// leading virtual binding to hang it off. `None` keeps #10123's per-read /// deref + residual check. pub elem_prefetch: Option, @@ -2308,7 +2308,7 @@ pub(crate) struct ElementShapeLoopFact { /// raw-f64 candidates; shape-keyed ones are the preheader's live query /// results (#10123). pub fields: std::collections::BTreeMap, - /// #10199: true when the fast clone lowers a body the matcher SYNTHESIZED + /// #10185: true when the fast clone lowers a body the matcher SYNTHESIZED /// (the K-statement accumulator fold, or the carried index's trailing /// write-back) rather than the source body. The function-wide /// shadow-slot-clear map is keyed by statement INDEX, so a rewritten body @@ -2392,7 +2392,7 @@ pub(crate) fn element_shape_loop_fact_for_property_get<'f>( (ElementShapeIndex::DerivedMod { local_id, .. }, Expr::LocalGet(id)) => { *id == *local_id } - // #10199: `arr[c]` and `const index = c; arr[index]` are + // #10185: `arr[c]` and `const index = c; arr[index]` are // the same subscript — the alias is virtual, so both // spellings read the carried slot the preheader bounded. (ElementShapeIndex::Carried(carried), Expr::LocalGet(id)) => { diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index 02c60d1bbe..2383e3890d 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -177,7 +177,7 @@ pub(crate) fn emit_shadow_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) { // 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. // - // #10199's carried index (`c = (a*c + b) % m`) and its optional + // #10185's carried index (`c = (a*c + b) % m`) and its optional // `const index = c` alias are two more of the same. if ctx.element_shape_loop_facts.iter().any(|fact| { fact.element_binding @@ -187,7 +187,7 @@ pub(crate) fn emit_shadow_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) { }) { return; } - // #10199: a SYNTHESIZED fast-clone body (the accumulator fold, the carried + // #10185: a SYNTHESIZED fast-clone body (the accumulator fold, the carried // write-back) no longer has the statement indices the function-wide clear // map is keyed by, so every clear reached from inside it would be // attributed to some other statement's local. Suppress them for the fast diff --git a/crates/perry-codegen/src/stmt/element_shape_carried.rs b/crates/perry-codegen/src/stmt/element_shape_carried.rs index d6e8700436..3924d8e733 100644 --- a/crates/perry-codegen/src/stmt/element_shape_carried.rs +++ b/crates/perry-codegen/src/stmt/element_shape_carried.rs @@ -1,4 +1,4 @@ -//! #10199: the element-shape clone's LOOP-CARRIED index — the `random` access +//! #10185: the element-shape clone's LOOP-CARRIED index — the `random` access //! shape. //! //! ```text diff --git a/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs b/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs index c86fbd7e88..429523b2b5 100644 --- a/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_fields_random_tests.rs @@ -1,4 +1,4 @@ -//! #10199: the `fields` and `random` access shapes — IR census for the +//! #10185: the `fields` and `random` access shapes — IR census for the //! element-shape clone's loop-carried index, its multi-statement accumulator //! fold, and its two non-numeric reads. //! @@ -248,7 +248,7 @@ fn store_count(fast: &str) -> usize { } // --------------------------------------------------------------------------- -// #10199 — the LOOP-CARRIED index (`random`). +// #10185 — the LOOP-CARRIED index (`random`). // --------------------------------------------------------------------------- #[test] @@ -459,7 +459,7 @@ fn a_recurrence_over_a_second_variable_declines() { } // --------------------------------------------------------------------------- -// #10199 — the MULTI-STATEMENT body, the string `.length` and the ternary +// #10185 — the MULTI-STATEMENT body, the string `.length` and the ternary // (`fields`). // --------------------------------------------------------------------------- diff --git a/crates/perry-codegen/src/stmt/element_shape_loop.rs b/crates/perry-codegen/src/stmt/element_shape_loop.rs index 9bc43a8d17..47e54c7c46 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop.rs @@ -214,7 +214,7 @@ enum MatchedIndex { local_id: u32, modulus_id: u32, }, - /// #10199: `c = (a*c + b) % m; … arr[c]`, optionally spelled through a + /// #10185: `c = (a*c + b) % m; … arr[c]`, optionally spelled through a /// `const index = c` alias. The recurrence itself lives on /// [`ElementShapeVersionedLoop::carried`] — this arm records only the /// spellings the subscript may take. See `stmt/element_shape_carried.rs`. @@ -249,9 +249,9 @@ struct ElementShapeVersionedLoop { /// form; `None` for the original single-statement accumulator body. element_binding: Option, accumulator_id: u32, - /// #10199: the matched recurrence, for the [`MatchedIndex::Carried`] form. + /// #10185: the matched recurrence, for the [`MatchedIndex::Carried`] form. carried: Option, - /// #10199: the body the FAST clone lowers, when it is not the source body. + /// #10185: the body the FAST clone lowers, when it is not the source body. /// /// Two rewrites, both of which exist to put every side exit an iteration /// can take BEFORE any of its stores: @@ -275,7 +275,7 @@ struct PureExprScope { /// #10123: `(d, m)` for a body whose first statement is /// `const d = counter % m`. derived: Option<(u32, u32)>, - /// #10199: `(carried, alias)` for a body whose first statement advances a + /// #10185: `(carried, alias)` for a body whose first statement advances a /// loop-carried index. Both are excluded from bare reads for the same /// reason `derived` is — inside the clone the real slot is one iteration /// behind until the trailing commit. @@ -345,7 +345,7 @@ fn element_shape_loop_pure_expr_collect( out.props.insert(property.clone()); true } - // #10199: `arr[].prop.length` — a STRING field's JS length. + // #10185: `arr[].prop.length` — a STRING field's JS length. // Admitted as its own form rather than as "a read of `prop` // followed by a generic `.length`", because the generic one is a // property diamond ending in a runtime call, and a call inside this @@ -357,7 +357,7 @@ fn element_shape_loop_pure_expr_collect( } _ => false, }, - // #10199: `arr[].prop ? A : B` with constant arms. JS truthiness + // #10185: `arr[].prop ? A : B` with constant arms. JS truthiness // of an arbitrary value is a runtime question the clone is not allowed // to guess, so the emitted read admits ONLY the two boolean singletons // and side-exits on everything else. Anything but a tracked element @@ -383,7 +383,7 @@ fn element_shape_loop_pure_expr_collect( Expr::LocalGet(id) => { scope.element_binding != Some(*id) && scope.derived.map(|(d, _)| d) != Some(*id) - // #10199: the carried index and its alias are virtual too — + // #10185: the carried index and its alias are virtual too — // their real slots are one iteration behind inside the clone, // so a bare read would hand out a stale value. && scope.carried.is_none_or(|(c, alias)| *id != c && alias != Some(*id)) @@ -444,7 +444,7 @@ fn match_index_form(index: &perry_hir::Expr, scope: &PureExprScope) -> Option match scope.carried { @@ -695,7 +695,7 @@ fn anon_shape_field_type_is_compatible( /// clone, never correctness. pub(super) fn lower_virtual_clone_binding(ctx: &mut FnCtx<'_>, id: u32) -> Result { enum VirtualBinding { - /// #7771 `const r = arr[j]` and #10199 `const index = c`: pure aliases, + /// #7771 `const r = arr[j]` and #10185 `const index = c`: pure aliases, /// no instruction of their own. Alias, /// #10123 `const d = j % m`: one `srem i32`. @@ -748,13 +748,13 @@ pub(super) fn lower_virtual_clone_binding(ctx: &mut FnCtx<'_>, id: u32) -> Resul blk.store(I32, &derived, &slot); } } - // #10199: this iteration's index is now in its slot, so this is where the + // #10185: this iteration's index is now in its slot, so this is where the // shared element deref belongs — for the ONE binding that owns it. emit_element_prefetch_for(ctx, scope_id, id); Ok(true) } -/// #10199: emit the once-per-iteration element prologue, if `site_local` is the +/// #10185: emit the once-per-iteration element prologue, if `site_local` is the /// statement the fact designated to own it. /// /// Every read in the clone loads the handle this parks, so the designated @@ -941,7 +941,7 @@ fn match_element_shape_versioned_loop( // 3. `const d = j % m; acc = ` — // #10123's derived-index form, the shape every sequential pass over a // parsed record array is written in; - // 4. #10199's loop-carried form, `c = (a*c + b) % m;` optionally followed + // 4. #10185's loop-carried form, `c = (a*c + b) % m;` optionally followed // by `const index = c;`. // // In 2, 3 and 4 the leading statement is VIRTUAL inside the fast clone: it @@ -954,7 +954,7 @@ fn match_element_shape_versioned_loop( // value. Form 4's `c` is the one mutable exception, and it pays for it with // a write-back (see `element_shape_carried`). // - // #10199 also admits K >= 1 accumulator statements instead of exactly one, + // #10185 also admits K >= 1 accumulator statements instead of exactly one, // which is what `sum += rows[i].id; sum += rows[i].name.length; …` needs. // NOTHING else is admitted. let mut element_binding: Option<(u32, u32)> = None; @@ -1034,7 +1034,7 @@ fn match_element_shape_versioned_loop( } derived = Some((*id, *modulus_id)); } - // #10199: `const index = c` after the recurrence. A pure alias, so + // #10185: `const index = c` after the recurrence. A pure alias, so // it emits nothing at all — the subscript resolves to the carried // slot either way. Expr::LocalGet(aliased) => { @@ -1072,7 +1072,7 @@ fn match_element_shape_versioned_loop( { return None; } - // #10199: statements 2..K are folded into statement 1 for the fast clone + // #10185: statements 2..K are folded into statement 1 for the fast clone // (`acc = a; acc = acc + b` ≡ `acc = (a) + b`), so that the WHOLE iteration // commits once, after every side exit it can take. Each of them must // therefore be `acc ` with `acc` as the left operand — the shape @@ -1134,7 +1134,7 @@ fn match_element_shape_versioned_loop( return None; } } - // #10199: a matched recurrence whose value nothing subscripts with would + // #10185: a matched recurrence whose value nothing subscripts with would // still have its `LocalSet` lowered generically — a `frem` libcall inside // the clone, which deletes it. Decline instead, so the loop keeps whatever // lowering it has today. @@ -1223,7 +1223,7 @@ fn match_element_shape_versioned_loop( None => return None, }; - // The property denylist, which is arm-specific (#10199). + // The property denylist, which is arm-specific (#10185). // // The CLASS arm's list exists because its read bakes in a compile-time // packed slot index while the surrounding lowering may route the name @@ -1278,7 +1278,7 @@ fn match_element_shape_versioned_loop( return None; } - // #10199: the fast clone's body, when the two rewrites apply. `lead` is + // #10185: the fast clone's body, when the two rewrites apply. `lead` is // whatever statements the walk consumed ahead of the accumulator run (the // recurrence and/or the virtual binding), kept verbatim. let lead = &body[..body.len() - rest.len()]; @@ -1497,7 +1497,7 @@ pub(super) fn lower_element_shape_versioned_for( // in JS, so a zero modulus is a slow-clone case rather than something the // clone may compute. // - // #10199's carried recurrence takes the same obligation for the same + // #10185's carried recurrence takes the same obligation for the same // reason, and one more of its own: its ENTRY value must be a non-negative // integral i32, because the whole i64 recurrence bound // (`a * i32::MAX + b < 2^53`, and a non-negative dividend for `srem` to @@ -1555,7 +1555,7 @@ pub(super) fn lower_element_shape_versioned_for( (MatchedIndex::Constant(k), _) => { crate::expr::element_shape_guard::ElementShapeIndexBound::Constant(*k) } - // #10199: the recurrence's result is `srem` of a non-negative dividend + // #10185: the recurrence's result is `srem` of a non-negative dividend // by `m`, so it lands in `[0, m)` exactly like the derived index and // takes exactly the same preheader obligation, `m <= length`. (MatchedIndex::DerivedMod { .. } | MatchedIndex::Carried { .. }, Some(modulus)) => { @@ -1684,10 +1684,10 @@ pub(super) fn lower_element_shape_versioned_for( } }; - // #10199: the shared once-per-iteration element deref, hung off the body's + // #10185: the shared once-per-iteration element deref, hung off the body's // leading virtual statement. Shape-keyed only: the class-keyed arm's reads // are raw doubles behind a `GC_OBJ_TYPED_LAYOUT_INTACT` residual whose - // emitted IR #7480's census pins, and nothing in #10199 measures it. + // emitted IR #7480's census pins, and nothing in #10185 measures it. let elem_prefetch = shape_keyed .then(|| { let site_local_id = match (&fact_index, matched.element_binding) { 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 488d64aa4c..1757e1e2c7 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -402,7 +402,7 @@ fn assert_fast_clone_is_entered(ir: &str) { fn fast_clone_slice(ir: &str) -> String { let mut owned = String::new(); let mut in_fast_block = false; - // #10199: the emitted text carries the function TWICE (same block labels), + // #10185: the emitted text carries the function TWICE (same block labels), // so every block would be collected twice and any `matches().count()` // assertion against the slice would read double. Stop at the first repeated // label, which is where the second copy begins — `contains` assertions are @@ -414,7 +414,7 @@ fn fast_clone_slice(ir: &str) -> String { // A block DEFINITION starts at column 0 and ends in `:`; anything else // belongs to whichever block was last opened. if !line.starts_with(char::is_whitespace) && trimmed.ends_with(':') { - // #10199 added three more: the string-`.length` decode + // #10185 added three more: the string-`.length` decode // (`element_shape.strlen*`), the boolean ternary's admitted arm // (`element_shape.bool`), and nothing for the shared prefetch, // which reuses `element_shape.load`. Every block the clone can 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 index fe47ad0e89..6d5d265332 100644 --- a/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_shape_keyed_tests.rs @@ -451,7 +451,7 @@ fn an_untracked_local_index_declines() { #[test] fn the_shape_keyed_arm_denies_only_proto() { - // #10199 narrowed this list. The CLASS arm still denies every name with a + // #10185 narrowed this list. The CLASS arm still denies every name with a // dedicated branch in the property dispatch, because its read bakes in a // compile-time packed slot. The SHAPE arm bakes in nothing: the preheader // asks the runtime for that exact key's inline slot in that exact ordinary @@ -547,7 +547,7 @@ fn a_declared_but_unresolvable_element_type_still_declines() { ); } -/// #10199's `fields` and `random` shapes — a child module so it inherits both +/// #10185's `fields` and `random` shapes — a child module so it inherits both /// this file's shape-keyed assertions and `element_shape_loop_tests`'s slicing /// helpers. #[path = "element_shape_fields_random_tests.rs"] diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index 5b9ac4f33e..cc27d26763 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -282,7 +282,7 @@ fn lower_return_expr(ctx: &mut FnCtx<'_>, expr: &perry_hir::Expr) -> Result, stmt: &Stmt) -> Result<()> { match stmt { Stmt::Expr(e) => { - // #10199: the element-shape fast clone's carried-index statements + // #10185: the element-shape fast clone's carried-index statements // (the recurrence and its trailing write-back) are lowered // VIRTUALLY, exactly like the `Let` bindings in `let_stmt.rs` — // the generic lowering of `c = (c * 17 + 7) % length` is an diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index e3c20605f2..338442e398 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -356,7 +356,7 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { Expr::Logical { left, right, .. } => { is_numeric_expr(ctx, left) && is_numeric_expr(ctx, right) } - // #10199: `arr[i]. ? 1 : 0` inside an element-shape fast + // #10185: `arr[i]. ? 1 : 0` inside an element-shape fast // clone. Deliberately NOT the general "both arms are numeric" rule — a // general ternary's CONDITION is an arbitrary JS truthiness test, which // is a runtime call; this arm is true only for the exact shape the @@ -386,7 +386,7 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { if property == "length" && expression_has_numeric_length(ctx, object) { return true; } - // #10199: `arr[i]..length` inside an element-shape + // #10185: `arr[i]..length` inside an element-shape // fast clone. The receiver is an untyped element read, so the // declared-type answer above cannot see a string there; the clone's // own lowering tag-tests the loaded word for both string diff --git a/test-files/test_gap_json_record_loop_clone.ts b/test-files/test_gap_json_record_loop_clone.ts index 0a60a97359..363fb7a04b 100644 --- a/test-files/test_gap_json_record_loop_clone.ts +++ b/test-files/test_gap_json_record_loop_clone.ts @@ -270,3 +270,301 @@ function sumTwoFields(rows: any, count: number): number { return sum; } console.log("two-fields:", sumTwoFields(moduleRows, moduleLength)); + +// --------------------------------------------------------------------------- +// 9. #10185: the LOOP-CARRIED index (`random`), the K-statement accumulator +// fold, and the two non-numeric reads. +// +// Every case below is a MISCOMPILE if it is wrong, not a slow path: +// +// * the carried write-back is placed at the END of the iteration, so a +// mid-iteration side exit leaves the real binding holding that +// iteration's ENTRY value and the slow clone advances the recurrence +// exactly once. A write-back at the update site double-applies it, and +// every later index is silently a different (still valid) record; +// * K accumulator statements fold to one store for the same reason: a +// side exit from the third read must not leave the first two applied; +// * `.length` and the ternary read a NaN-boxed word and assume a +// representation, so each tag-tests it and side-exits otherwise. +// --------------------------------------------------------------------------- + +// The benchmark's `random` mode, with the `const index = cursor` alias. +function randomSum(rows: any, count: number, n: number): number { + let sum = 0; + let cursor = 0; + for (let i = 0; i < count; i++) { + cursor = (cursor * 17 + 7) % n; + const index = cursor; + sum += rows[index].id; + } + return sum; +} + +// The same recurrence subscripted DIRECTLY, with the carried value read after +// the loop — the clone owes a write-back, and this is what observes it. +function randomSumAndCursor( + rows: any, + count: number, + n: number, + start: number, +): string { + let sum: any = 0; + let cursor = start; + for (let i = 0; i < count; i++) { + cursor = (cursor * 17 + 7) % n; + sum += rows[cursor].id; + } + return String(sum) + "|" + String(cursor); +} + +// A multiplier large enough that `a * cursor` leaves the range an `f64` +// represents exactly. JavaScript evaluates this in doubles, so an i64 +// recurrence would NOT agree — the matcher must decline and let the ordinary +// lowering run. +function randomSumWideMultiplier( + rows: any, + count: number, + n: number, +): string { + let sum = 0; + let cursor = 3; + for (let i = 0; i < count; i++) { + cursor = (cursor * 600000000 + 7) % n; + sum += rows[cursor].id; + } + return String(sum) + "|" + String(cursor); +} + +// A recurrence with an even step, so a fractional carried value still lands +// on whole-number indices after the first update. +function randomSumEvenStep( + rows: any, + count: number, + n: number, + start: number, +): string { + let sum = 0; + let cursor = start; + for (let i = 0; i < count; i++) { + cursor = (cursor * 2 + 7) % n; + sum += rows[cursor].id; + } + return String(sum) + "|" + String(cursor); +} + +// The benchmark's `fields` mode: three accumulator statements over ONE +// element, one through a string and one through a boolean. +function fieldsSum(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; + sum += rows[index].name.length; + sum += rows[index].active ? 1 : 0; + } + return sum; +} + +const randomRows: any = JSON.parse(buildRecords(40, "pad")); +console.log("random-sum:", randomSum(randomRows, 200, randomRows.length)); +console.log( + "random-sum-again:", + randomSum(randomRows, 200, randomRows.length), +); +console.log( + "random-cursor-live-after:", + randomSumAndCursor(randomRows, 200, randomRows.length, 0), +); +// An odd trip count observes a different point of the cycle than the run +// above, so the write-back is checked at two places rather than one. +console.log( + "random-cursor-live-after-odd:", + randomSumAndCursor(randomRows, 201, randomRows.length, 0), +); +// A modulus smaller than the array. 13 is chosen because `(17c + 7) % 13` +// walks six distinct indices from 0 — several plausible moduli make the +// recurrence a fixed point at 0, where every assertion below would hold of a +// loop that read one element forever. +console.log( + "random-small-modulus:", + randomSumAndCursor(randomRows, 50, 13, 0), +); +console.log( + "random-modulus-one:", + randomSumAndCursor(randomRows, 9, 1, 0), +); + +// Entry values the preheader must reject: JS `%` returns a NEGATIVE remainder +// for a negative dividend, and a fractional index is not an index at all. +// Both take the slow clone and keep ordinary JavaScript semantics. +try { + console.log( + "random-negative-start:", + randomSumAndCursor(randomRows, 4, 5, -3), + ); +} catch (err) { + console.log("random-negative-start-threw:", String(err).slice(0, 9)); +} +// A FRACTIONAL entry value. The step is even so every index the loop +// actually reads is a whole number — the divergence being checked is the +// preheader's, not the subscript's: if the clone materialized `0.5` as the +// i32 `0` it would start the recurrence one step early and every index would +// shift, which the sum and the final cursor both show. +console.log( + "random-fractional-start:", + randomSumEvenStep(randomRows, 6, 5, 0.5), +); +console.log( + "random-integral-start-same-step:", + randomSumEvenStep(randomRows, 6, 5, 0), +); +// `x % 0` is NaN, and `rows[NaN]` is `undefined`. +try { + console.log("random-modulus-zero:", randomSumAndCursor(randomRows, 3, 0, 0)); +} catch (err) { + console.log("random-modulus-zero-threw:", String(err).slice(0, 9)); +} +// A modulus past the array's length runs off the end. +try { + console.log( + "random-modulus-past-length:", + randomSumAndCursor(randomRows, 30, 500, 0), + ); +} catch (err) { + console.log("random-modulus-past-length-threw:", String(err).slice(0, 9)); +} +console.log( + "random-wide-multiplier:", + randomSumWideMultiplier(randomRows, 25, 11), +); + +// THE write-back test. The array is homogeneous in SHAPE, so the clone is +// entered — but one `id` is a string, so the per-read Number tag test side +// -exits mid-iteration and the slow clone re-runs it. If the recurrence had +// already been committed, the slow clone advances it a SECOND time and both +// the sum and the final cursor drift. +// `(17c + 7) % 5` walks 0 -> 2 -> 1 -> 4 -> 0, so index 2 — the string one — +// really is read. A modulus that skipped it would make this whole case +// vacuous while still printing a matching number. +const randomMixed: any = JSON.parse( + '[{"id":0},{"id":1},{"id":"two"},{"id":3},{"id":4}]', +); +console.log( + "random-side-exit:", + randomSumAndCursor(randomMixed, 30, randomMixed.length, 0), +); +console.log( + "random-side-exit-again:", + randomSumAndCursor(randomMixed, 30, randomMixed.length, 0), +); + +// The recurrence over a HETEROGENEOUS array declines at the array level. +console.log( + "random-hetero:", + randomSumAndCursor(hetero, 16, hetero.length, 0), +); + +// --------------------------------------------------------------------------- +// 9b. The `fields` shape. +// --------------------------------------------------------------------------- + +// SSO names (<= 5 bytes, packed into the NaN-box) and heap names in the SAME +// array — the shape is identical either way, and the `.length` decode has to +// handle both. +const mixedNames: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},' + + '{"id":2,"name":"user_222","active":false},' + + '{"id":3,"name":"","active":true},' + + '{"id":4,"name":"abcde","active":false},' + + '{"id":5,"name":"abcdef","active":true}]', +); +console.log("fields-sso-and-heap:", fieldsSum(mixedNames, 25, mixedNames.length)); +console.log( + "fields-sso-and-heap-again:", + fieldsSum(mixedNames, 25, mixedNames.length), +); + +// A NON-STRING `name`: `.length` of a number is `undefined`, so the sum goes +// NaN — and the clone must side-exit rather than decode a double as a string +// header. The fold is what keeps the earlier `id` from being applied twice. +const numberName: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},{"id":2,"name":7,"active":true},' + + '{"id":3,"name":"cd","active":false}]', +); +console.log("fields-number-name:", fieldsSum(numberName, 9, numberName.length)); + +// A NULL `name`: `null.length` throws, exactly as JS says. +const nullName: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},{"id":2,"name":null,"active":true}]', +); +try { + console.log("fields-null-name:", fieldsSum(nullName, 4, nullName.length)); +} catch (err) { + console.log("fields-null-name-threw:", String(err).slice(0, 9)); +} + +// NON-BOOLEAN `active` values. JS truthiness of `0`, `""`, `"no"` and `null` +// is a runtime question the clone is not allowed to guess: only the two +// boolean singletons are admitted and everything else side-exits. +const truthyActive: any = JSON.parse( + '[{"id":1,"name":"ab","active":true},{"id":2,"name":"cd","active":0},' + + '{"id":3,"name":"ef","active":"no"},{"id":4,"name":"gh","active":""},' + + '{"id":5,"name":"ij","active":null},{"id":6,"name":"kl","active":false}]', +); +console.log( + "fields-truthiness:", + fieldsSum(truthyActive, 24, truthyActive.length), +); +console.log( + "fields-truthiness-again:", + fieldsSum(truthyActive, 24, truthyActive.length), +); + +// Two accumulator statements where the SECOND read side-exits: the first must +// not be applied twice when the slow clone re-runs the iteration. +function twoStatementSum(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; + sum += rows[index].score; + } + return sum; +} +const secondFieldMixed: any = JSON.parse( + '[{"id":1,"score":10},{"id":2,"score":"x"},{"id":3,"score":30}]', +); +console.log( + "fold-side-exit:", + twoStatementSum(secondFieldMixed, 9, secondFieldMixed.length), +); + +// Heterogeneous shapes with the multi-read body. +console.log("fields-hetero:", fieldsSum(hetero, 12, hetero.length)); + +// --------------------------------------------------------------------------- +// 9c. Property names the CLASS-keyed arm denies. A parsed record's own data +// property SHADOWS the prototype name it collides with, and the +// shape-keyed preheader resolves it from the runtime shape table, so +// `name` and `length` are ordinary inline slots here. `name` in +// particular is what the access benchmark's own `fields` mode reads. +// --------------------------------------------------------------------------- +function ownNamedFields(rows: any, count: number, n: number): number { + let sum = 0; + for (let i = 0; i < count; i++) { + const index = i % n; + sum += rows[index].length; + sum += rows[index].size; + sum += rows[index].name.length; + } + return sum; +} +const ownNames: any = JSON.parse( + '[{"length":3,"size":4,"name":"ab"},{"length":5,"size":6,"name":"cdef"},' + + '{"length":7,"size":8,"name":"ghijklm"}]', +); +console.log("own-named-fields:", ownNamedFields(ownNames, 12, ownNames.length)); +console.log( + "own-named-fields-again:", + ownNamedFields(ownNames, 12, ownNames.length), +); From 54765a927ba8a6a8c7cf0a7c52aca885b1c8d514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 13:03:18 +0200 Subject: [PATCH 3/4] lint(census): follow the element clone's ShapeId read to its new emitter The shape-descriptor census asserts that the element-shape guard reads the authoritative ShapeId at header offset 4. That read moved out of `emit_element_shape_field_load` into the deref helper the per-read path and the shared prologue now share, so the census names the function that actually emits it. --- scripts/shape_descriptor_census.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/shape_descriptor_census.py b/scripts/shape_descriptor_census.py index 0626294170..df47d5af22 100644 --- a/scripts/shape_descriptor_census.py +++ b/scripts/shape_descriptor_census.py @@ -652,7 +652,13 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: "emit_proven_shape_recheck", "emit_class_field_inline_precheck", )), - (raw_element_guard, ("emit_element_shape_field_load",)), + # #10185: the element clone's residual moved out of + # `emit_element_shape_field_load` into the deref helper both the + # per-read path and the shared once-per-iteration prologue call, so the + # ShapeId read this asserts now lives there. Naming the function that + # actually emits the check is what keeps the assertion from going + # vacuous the next time the emitter is split. + (raw_element_guard, ("emit_element_deref_with_residual",)), ): for name in names: body = function_body(source, name) From 533b59247bc7b5da51201b365d9dfa93f1698733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 13 Sep 2026 13:33:17 +0200 Subject: [PATCH 4/4] docs(changelog): record the measured fields/random numbers 12-cell interleaved A/B against node 26.5.1 and bun 1.3.14, plus instructions retired per iteration, all from one coherent build of this worktree at the base commit and at HEAD. The one cell that does not reach parity (16k fields, 1.029x) is named rather than rounded off. --- .../10185-element-shape-fields-random.md | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/changelog.d/10185-element-shape-fields-random.md b/changelog.d/10185-element-shape-fields-random.md index 788c133fdf..07b0533c87 100644 --- a/changelog.d/10185-element-shape-fields-random.md +++ b/changelog.d/10185-element-shape-fields-random.md @@ -94,24 +94,25 @@ built from this worktree, `base` at #10171's head `2b77e7d4fe`): | cell | base (#10171) | new | node 26.5.1 | bun 1.3.14 | new / best | |---|---:|---:|---:|---:|---:| -| 16k `repeat` | 4.15 | 4.19 | 3.24 | 5.03 | 1.293 | -| 16k `sequential` | 4.21 | 4.17 | 4.35 | 6.14 | 0.959 | -| 16k `random` | 15.38 | **4.99** | 7.10 | 9.61 | **0.703** | -| 16k `fields` | 25.76 | **6.19** | 5.75 | 8.35 | **1.077** | -| 1m `repeat` | 4.12 | 4.19 | 3.57 | 5.25 | 1.174 | -| 1m `sequential` | 4.24 | 4.18 | 9.35 | 8.06 | 0.519 | -| 1m `random` | 15.90 | **5.25** | 12.66 | 11.66 | **0.450** | -| 1m `fields` | 29.02 | **6.31** | 12.68 | 15.41 | **0.498** | -| 20m `repeat` | 4.19 | 4.17 | 3.52 | 5.13 | 1.185 | -| 20m `sequential` | 4.29 | 4.37 | 7.48 | 8.83 | 0.584 | -| 20m `random` | 27.56 | **5.60** | 9.45 | 10.64 | **0.593** | -| 20m `fields` | 27.26 | **6.22** | 11.53 | 14.93 | **0.539** | - -The two targeted shapes move 3.1x-4.9x and land below the better of node and +| 16k `repeat` | 4.21 | 4.17 | 3.56 | 5.23 | 1.171 | +| 16k `sequential` | 4.17 | 4.30 | 4.57 | 6.25 | 0.941 | +| 16k `random` | 15.48 | **4.97** | 7.16 | 9.96 | **0.694** | +| 16k `fields` | 25.59 | **6.05** | 5.88 | 9.77 | **1.029** | +| 1m `repeat` | 4.15 | 4.17 | 3.19 | 5.05 | 1.307 | +| 1m `sequential` | 4.26 | 4.20 | 9.42 | 7.96 | 0.528 | +| 1m `random` | 16.00 | **5.32** | 11.71 | 11.17 | **0.476** | +| 1m `fields` | 28.14 | **6.17** | 12.70 | 14.91 | **0.486** | +| 20m `repeat` | 4.12 | 4.11 | 3.50 | 5.24 | 1.174 | +| 20m `sequential` | 4.20 | 4.19 | 6.98 | 8.51 | 0.600 | +| 20m `random` | 27.53 | **5.63** | 9.31 | 10.56 | **0.605** | +| 20m `fields` | 27.05 | **6.06** | 10.98 | 14.81 | **0.552** | + +The two targeted shapes move 3.0x-4.9x and land below the better of node and bun on five of their six cells. The sixth, 16k `fields`, is the one that does -not reach parity: 6.08 against node's 5.83 on a 12-round re-measure (1.043x). +not reach parity: 6.05 against node's 5.88 (1.029x; 6.08 vs 5.83 on a +separate 12-round re-measure of just that pair). That is the smallest fixture, where the whole array is in L1 and the guard -overhead is the only thing left — node's own `fields` is just 1.34x its +overhead is the only thing left — node's own `fields` is just 1.29x its `sequential` there, because its inline caches have type feedback proving `name` is a string and `active` a boolean, while this clone tag-tests both on every read. Paying that test is what lets it side-exit instead of deoptimize, so it @@ -128,10 +129,10 @@ Instructions retired per iteration on the 16k fixture (2M iterations minus a | shape | base | new | |---|---:|---:| -| random | 207.9 | 29.1 | -| fields | 466.6 | 47.0 | -| sequential | 25.1 | 25.1 | -| repeat | 16.1 | 15.8 | +| random | 207.6 | 29.1 | +| fields | 466.4 | 47.0 | +| sequential | 25.0 | 25.6 | +| repeat | 16.0 | 16.1 | `random` lands one recurrence above `sequential`'s 25, and `fields` two extra guarded reads plus a string decode and a select above it — which is what the