diff --git a/changelog.d/9294-affine-range-index.md b/changelog.d/9294-affine-range-index.md new file mode 100644 index 0000000000..2f057fa5b8 --- /dev/null +++ b/changelog.d/9294-affine-range-index.md @@ -0,0 +1,44 @@ +**A counted loop whose reads use an affine index now hoists its receiver guard +into the preheader** (#9253). `16_matrix_multiply` goes 100 ms → 69 ms against +node's 32 ms on an idle machine. + +The interesting part was where the time was *not*. That loop spends 97% of its +time inside generated code with no runtime calls, so the cost was never a +missing inline or an unremoved helper. Per iteration, for both receivers, it +re-derived the pointer tag check, the handle-band check, the header +dereference, the `_reserved` flag tests and the two 16,000,000 length/capacity +sanity compares — for receivers that are loop-invariant parameters whose +headers cannot change inside the loop. LLVM cannot hoist that: the guard +reloads the header through a pointer it cannot prove unaliased, and the +incremental-barrier atomic read is a motion barrier. + +The packed-f64 range tier already had everything required except a way to +describe `a[i * size + k]`, whose index has no compile-time window: it already +takes a loop-invariant local or parameter bound, emits one guard per receiver +AND-reduced into a single branch, and keeps its cached receivers GC-safe by +refreshing them on the back-edge poll. An access may now be *affine* — an +integer-producing expression over the loop counter and loop-invariant integer +locals — and such an access publishes a receiver-only fact: the entry guard +proves shape, raw-f64 packedness, integrity and the sanity bounds once in the +preheader, and each read pays one inline `icmp ult idx, len` with the fact's +existing side exit. + +The index is materialised in i64 rather than i32, because `i * size` can exceed +i32 for a large matrix even when the final index is valid and an i32 +computation would wrap — turning an out-of-bounds access into an in-bounds one. +The bounds compare is unsigned, so a negative index reads as a huge unsigned +value and side-exits; no static non-negativity proof is needed, which matters +because `size` is a parameter with no callsite range summary and no static +window for the product is obtainable. The index must also mention the counter: +a wholly loop-invariant index like `a[0]` is affine by the grammar but has a +compile-time window, and admitting it here made the classic walker succeed and +silently stole those loops from the dense tier's masked path that serves them +better. + +Reads only, in the classic mode only. Dense mode's loads carry no side exit and +so cannot take a per-read bounds check, and an affine store is rejected because +the side exit re-executes the iteration. + +This does not reach parity. The residual is the per-read bounds check and index +materialisation, plus the `c[i * size + j]` store in the enclosing loop, which +stays generic for the reason above. diff --git a/changelog.d/dense-accumulator-masked-reads.md b/changelog.d/dense-accumulator-masked-reads.md new file mode 100644 index 0000000000..e8b12f2b46 --- /dev/null +++ b/changelog.d/dense-accumulator-masked-reads.md @@ -0,0 +1,32 @@ +**A float accumulator over masked reads now earns the dense range clone** +(`17_loop_data_dependent`: 475 ms → 219 ms against node's 220 ms on an idle +machine — parity, from 2.16×). + +`sum = sum * x[i & 63] + x[(i * 7) & 63]` was rejected by the dense range tier +while `sum = sum * x[i & 63]` was admitted. The discriminator was the +accumulator's static numeric proof: `+` can be concatenation, so the +per-statement proof demands both operands numeric, and a reassigned +accumulator has no such proof — its own writes read the guarded array, whose +element proof only exists once the guard has run. A chicken-and-egg that `*` +never faces, because multiplication needs only the weaker inert fact. + +The matcher now peels the accumulator: when the proof fails on the `LocalSet` +target of a self-accumulating write, it retries with the target treated as +numeric BY CONTRACT, records it pending, and then verifies every pending +local with the same collector the lowering runs — rejecting the whole dense +match (with its own named trace reasons) if the two disagree, so the clone can +never contain a dynamic `+` under facts that forbid one. The contract is +enforced at run time twice over: the clone's entry emits a genuine-double tag +check on the accumulator, and the dense entry guard validates the whole masked +window hole-free. A string-seeded accumulator and a string element both route +to the slow copy and produce node's concatenation, verified under forced +evacuation. + +Along the way the accumulator walk's index leaf learned masked reads — and +fixed a match-arm reachability bug while doing so: `_ if offset_reads_inlined` +was a guarded catch-all, so any arm placed after it was unreachable whenever +the flag was set. Admitted masked-only single arrays now qualify for +accumulator admission (counter-bearing arrays keep priority; multiple arrays +still decline), and `MaskedWindowArrayFact` carries the admitted accumulators +so `is_numeric_expr` can see them while the clone lowers, mirroring the +string-window fact's field. diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index 7cd60fe0b6..39dfc34e4c 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -43,7 +43,10 @@ use super::{ mod foreign_counter; mod guarded_array; pub(crate) use foreign_counter::packed_f64_loop_index_parts; -use foreign_counter::{foreign_packed_loop_read, packed_f64_loop_offset_read}; +use foreign_counter::{ + affine_packed_loop_read, emit_affine_index_i64, foreign_packed_loop_read, + packed_f64_loop_offset_read, +}; mod inline_dyn_typed_array; use guarded_array::{ @@ -774,6 +777,31 @@ pub(crate) fn lower_numeric_index_get_for_number_context( // `is_packed_f64_loop_foreign_read_index` for read-only bodies. The // guard already proved this receiver's packed layout, so the element // load is the clone's raw slot load behind one inline bounds check. + + // #9253: `a[i * size + k]` against a receiver-only affine fact. The + // entry guard proved the receiver once in the preheader; the index is + // materialised in i64 and range-clamped, then the shared bounds-checked + // element load does one `icmp ult idx, len` and takes the fact's side + // exit. That leaves per iteration only the compare and the raw load, + // instead of re-deriving tag/handle/header/flags/16M-sanity per access. + if let Some(fact) = affine_packed_loop_read(ctx, *arr_id, index.as_ref()) { + let arr_box = lower_expr(ctx, object)?; + if let Some(idx64) = emit_affine_index_i64(ctx, index.as_ref(), fact.index_local_id) { + // Unsigned: a negative index reads as a huge unsigned value and + // takes the side exit, so no static non-negativity proof is + // needed. The i32 ceiling keeps the truncation below exact. + let fits = ctx.block().icmp_ult(I64, &idx64, "2147483647"); + let cont_idx = ctx.new_block("packed_f64_affine.index_fits"); + let cont_label = ctx.block_label(cont_idx); + ctx.block() + .cond_br(&fits, &cont_label, &fact.store_side_exit_label); + ctx.current_block = cont_idx; + let idx_i32 = ctx.block().trunc(I64, &idx64, I32); + return Ok(Some(lower_packed_f64_loop_index_get( + ctx, *arr_id, &arr_box, &idx_i32, &fact, true, + ))); + } + } if let Some((fact, idx_id)) = foreign_packed_loop_read(ctx, *arr_id, index.as_ref()) { if let Some(i32_slot) = ctx.i32_counter_slots.get(&idx_id).cloned() { let arr_box = lower_expr(ctx, object)?; diff --git a/crates/perry-codegen/src/expr/index_get/foreign_counter.rs b/crates/perry-codegen/src/expr/index_get/foreign_counter.rs index 61a6b7b324..679c11c740 100644 --- a/crates/perry-codegen/src/expr/index_get/foreign_counter.rs +++ b/crates/perry-codegen/src/expr/index_get/foreign_counter.rs @@ -97,3 +97,87 @@ pub(crate) fn packed_f64_loop_offset_read( let needs_bounds_check = offset != 0 && !fact.allow_holes && !fact.window_validated; Some((fact, idx_id, offset, needs_bounds_check)) } + +/// #9253: an active receiver-only (`affine_indices`) fact for `arr_id`, when +/// `index` is an affine integer expression rather than a bare local. +/// +/// The bare-local cases belong to `foreign_packed_loop_read` and the clone's +/// own counter path; this is `a[i * size + k]`. +pub(crate) fn affine_packed_loop_read( + ctx: &FnCtx<'_>, + arr_id: u32, + index: &Expr, +) -> Option { + if matches!(index, Expr::LocalGet(_)) { + return None; + } + let fact = ctx + .packed_f64_loop_facts + .iter() + .rev() + .find(|fact| fact.array_local_id == arr_id && fact.affine_indices)? + .clone(); + affine_index_leaves_materializable(ctx, index, fact.index_local_id).then_some(fact) +} + +/// Every leaf must be readable as an i32 here, matching exactly what the +/// matcher's `affine_leaf_ok` admitted. If these two disagree the matcher +/// admits a shape this lowering declines, the read falls back to a helper +/// call, and the clone's call-free scan discards the whole clone — the #9259 +/// cascade. +fn affine_index_leaves_materializable(ctx: &FnCtx<'_>, index: &Expr, counter_id: u32) -> bool { + match index { + Expr::Integer(v) => i32::try_from(*v).is_ok(), + Expr::Number(n) => { + n.fract() == 0.0 && *n >= f64::from(i32::MIN) && *n <= f64::from(i32::MAX) + } + Expr::LocalGet(id) => *id == counter_id || ctx.i32_counter_slots.contains_key(id), + Expr::Binary { + op: perry_hir::BinaryOp::Add | perry_hir::BinaryOp::Sub | perry_hir::BinaryOp::Mul, + left, + right, + } => { + affine_index_leaves_materializable(ctx, left, counter_id) + && affine_index_leaves_materializable(ctx, right, counter_id) + } + _ => false, + } +} + +/// Materialise the index in **i64** from each leaf's shared i32 shadow. +/// +/// i64 rather than i32 because the source expression evaluates in doubles and +/// `i * size` can exceed i32 for a large matrix even when the final sum is a +/// valid index; computing in i32 would wrap and could produce an in-bounds +/// index for an out-of-bounds access. Every leaf is a proven i32, so three +/// levels of add/sub/mul cannot overflow i64. +pub(crate) fn emit_affine_index_i64( + ctx: &mut FnCtx<'_>, + index: &Expr, + counter_id: u32, +) -> Option { + match index { + Expr::Integer(v) => Some(v.to_string()), + Expr::Number(n) => Some(format!("{}", *n as i64)), + Expr::LocalGet(id) => { + let slot = if *id == counter_id { + ctx.i32_counter_slots.get(id)?.clone() + } else { + ctx.i32_counter_slots.get(id)?.clone() + }; + let narrow = ctx.block().load(I32, &slot); + Some(ctx.block().sext(I32, &narrow, I64)) + } + Expr::Binary { op, left, right } => { + let l = emit_affine_index_i64(ctx, left, counter_id)?; + let r = emit_affine_index_i64(ctx, right, counter_id)?; + Some(match op { + perry_hir::BinaryOp::Add => ctx.block().add(I64, &l, &r), + perry_hir::BinaryOp::Sub => ctx.block().sub(I64, &l, &r), + perry_hir::BinaryOp::Mul => ctx.block().mul(I64, &l, &r), + _ => return None, + }) + } + _ => None, + } +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 9440e0da23..749ee678a1 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -1992,6 +1992,16 @@ pub(crate) struct PackedF64LoopFact { /// is the dense range loop: the window is additionally hole-free, so /// loads carry no hole check at all). pub window_validated: bool, + /// #9253: reads on this receiver may use an AFFINE index — an + /// integer-producing expression over the loop counter and loop-invariant + /// integer locals (`a[i * size + k]`). The entry guard proved the receiver + /// only, so every such read pays an inline `icmp ult idx, len` against the + /// live length and takes `store_side_exit_label` when it fails. + /// + /// The matcher admits the loop only when EVERY tracked access qualifies, + /// so a lowering that finds this flag set may trust that the index shape in + /// front of it was the one admitted. + pub affine_indices: bool, } /// Element storage a masked-window fact's entry guard proved (#6750 @@ -2054,6 +2064,12 @@ pub(crate) struct MaskedWindowArrayFact { /// element type is exactly i32 (Int32Array tier), so loads may /// materialize elements as native `i32`. pub values_i32: bool, + /// Accumulator locals admitted by the entry tag check for THIS clone: + /// every in-clone write is numeric-preserving (verified by the + /// accumulator walk), so `is_numeric_expr` may treat them as Numbers + /// while the fact is live. Mirrors `StringWindowArrayFact`'s + /// `numeric_accumulator` (#9160) and `PackedF64LoopFact`'s vec. + pub numeric_accumulators: Vec, /// Storage layout the guard proved — selects the inline load shape. pub elem: MaskedWindowElem, /// True only in a dense fast-loop scope whose matcher admitted masked diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 07ae4c79f2..6ac16a4cca 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -535,17 +535,17 @@ fn emit_range_loop_accumulator_admission( slow_pre_label: &str, block_prefix: &str, ) -> PackedAccumulatorScope { - let mut counter_arrays = matched - .arrays - .iter() - .filter(|access| access.counter.is_some()) - .map(|access| access.array_id); - let Some(array_id) = counter_arrays.next() else { - return PackedAccumulatorScope::empty(); + // One array may carry the accumulator proof: with several, no single + // admitted leaf spans them all. Counter-bearing arrays keep priority; + // a masked-only array (dense mode) now qualifies too, because the dense + // entry guard validates its whole window hole-free, making every + // in-window read a Number the accumulator walk may lean on. + let mut single = matched.arrays.iter().filter(|a| a.counter.is_some()); + let (array_id, masked_reads_validated) = match (single.next(), single.next()) { + (Some(access), None) => (access.array_id, access.stat.is_some()), + (None, _) if matched.arrays.len() == 1 => (matched.arrays[0].array_id, true), + _ => return PackedAccumulatorScope::empty(), }; - if counter_arrays.next().is_some() { - return PackedAccumulatorScope::empty(); - } emit_packed_numeric_accumulator_admission( ctx, body, @@ -557,6 +557,7 @@ fn emit_range_loop_accumulator_admission( // hole-checked, so an `a[i +/- c]` read is lowered inline and yields a // Number. See `accumulator_rhs_is_numeric`. true, + masked_reads_validated, ) } @@ -870,6 +871,7 @@ fn emit_packed_numeric_accumulator_admission( slow_pre_label: &str, block_prefix: &str, offset_reads_inlined: bool, + masked_reads_validated: bool, ) -> PackedAccumulatorScope { let accumulators = super::stable_packed_accumulator::collect_numeric_accumulators( ctx, @@ -877,6 +879,7 @@ fn emit_packed_numeric_accumulator_admission( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, ); // Integer (`c++`) accumulators admit independently of the float set — // a pure count loop has no float accumulator at all. @@ -1062,6 +1065,9 @@ fn lower_packed_f64_versioned_for( // read takes the generic path — which can produce `undefined`. #9259 // is the work that would make an offset read inline here. false, + // No masked windows either: this tier's guard proves the receiver and + // the length bound, not a hole-free window. + false, ); acc_scope.hoist_receivers(ctx, &[matched.array_id]); ctx.packed_f64_loop_facts.push(PackedF64LoopFact { @@ -1073,6 +1079,7 @@ fn lower_packed_f64_versioned_for( array_kind: matched.array_kind, allow_holes: false, window_validated: false, + affine_indices: false, numeric_accumulators: acc_scope.accumulators.clone(), }); // The guard just proved a live, non-forwarded plain array, and the @@ -1149,6 +1156,13 @@ pub(super) struct PackedF64RangeArrayAccess { /// (`arr[e & K]`, `arr[K1 + (e >>> k & K2)]`, … — see /// `collectors::static_index_window`). Dense mode only. pub(super) stat: Option<(i64, i64)>, + /// #9253: at least one access used an AFFINE index — an integer-producing + /// expression over the counter and loop-invariant integer locals, such as + /// `a[i * size + k]`. Such an index has no compile-time window, so the + /// entry guard proves only the receiver (shape / raw-f64 packedness / + /// integrity) and each read pays an inline `icmp ult idx, len` with a side + /// exit. Classic mode only: dense mode's loads carry no side exit. + pub(super) affine: bool, pub(super) written: bool, } @@ -1296,8 +1310,31 @@ fn match_packed_f64_range_loop( }; let mut accesses: std::collections::BTreeMap = std::collections::BTreeMap::new(); - let dense = if packed_f64_range_loop_body_collect(body, counter_id, bound_local, &mut accesses) - { + // #9253: a leaf of an affine index (`a[i * size + k]`) is admissible when + // it is an integer-valued local that the loop body never writes. The + // counter is handled by the recogniser itself. Invariance is required + // because the fast clone recomputes the index in i64 from the leaves' + // slots: a leaf written mid-body would make the recomputation and the + // source expression disagree. + let affine_leaf_ok = |id: u32| -> bool { + // The lowering materialises each leaf from its shared i32 shadow, so + // require that shadow HERE. Matcher and lowering must admit exactly + // the same shapes: admitting one the lowering declines emits a helper + // call, and the clone's call-free scan then discards the whole clone + // (the #9259 cascade). + ctx.integer_locals.contains(&id) + && ctx.i32_counter_slots.contains_key(&id) + && !stmts_mutate_local(body, id) + && !ctx.boxed_vars.contains(&id) + && !ctx.closure_captures.contains_key(&id) + }; + let dense = if packed_f64_range_loop_body_collect( + body, + counter_id, + bound_local, + &mut accesses, + Some(&affine_leaf_ok), + ) { false } else { // The classic shape (one statement, counter-offset indices, stores @@ -1305,15 +1342,35 @@ fn match_packed_f64_range_loop( // read-only DENSE mode: several scalar statements, masked // statically-windowed indices, no stores, no side exits. accesses.clear(); + let mut pending_accumulators = std::collections::BTreeSet::new(); if !packed_f64_range_loop_dense_body_collect( ctx, body, counter_id, bound_local, &mut accesses, + &mut pending_accumulators, ) { return range_loop_reject("body_not_admissible"); } + if !pending_accumulators.is_empty() { + // The peel above assumed each pending local numeric; that holds + // only if the lowering will actually admit it (entry tag check + + // numeric-preserving writes). Verify with the SAME collector and + // the SAME array selection `emit_range_loop_accumulator_admission` + // uses -- if the two disagree, the clone would contain a dynamic + // `+` (a collecting call) under facts that forbid one. + if accesses.len() != 1 { + return range_loop_reject("accumulator_needs_single_array"); + } + let array_id = *accesses.keys().next().expect("len checked"); + let admitted = super::stable_packed_accumulator::collect_numeric_accumulators( + ctx, body, array_id, counter_id, true, true, + ); + if !pending_accumulators.iter().all(|id| admitted.contains(id)) { + return range_loop_reject("accumulator_not_provable"); + } + } true }; if accesses.is_empty() { @@ -1385,9 +1442,24 @@ fn match_packed_f64_range_loop( return range_loop_reject("static_window_out_of_i32"); } } - if access.counter.is_none() && access.stat.is_none() { + if access.counter.is_none() && access.stat.is_none() && !access.affine { return range_loop_reject("access_has_no_window"); } + // #9253: an affine index has no compile-time window by construction, + // so its entry guard proves the RECEIVER only and each read pays an + // inline bounds check with a side exit. Dense mode's loads have no + // side exit, so the two are mutually exclusive; the matcher only ever + // sets `affine` on the classic path, and this makes that explicit + // rather than implicit in which caller passed the leaf predicate. + if access.affine && dense { + return range_loop_reject("affine_index_in_dense_mode"); + } + if access.affine && access.written { + // A store through an affine index would need the side exit to be + // replay-safe, which the classic mode only guarantees for a single + // statement whose one side effect happens last. Reads only. + return range_loop_reject("affine_index_store"); + } if access.written { // Dense written arrays need only the declared number[]-ness: the // static fact set `packed_f64_eligible_for_guarded_store` consults @@ -1431,6 +1503,89 @@ fn match_packed_f64_range_loop( }) } +/// #9253: is `index` an integer-producing expression over the loop counter and +/// loop-invariant integer locals — `k`, `i * size + k`, `k * size + j`? +/// +/// This is a KIND proof, not a range proof. It deliberately does not try to +/// bound the value: `size` is typically a parameter with no callsite summary, +/// so `int_range_expr` answers `None` and any static window is unobtainable. +/// The read instead pays an unsigned `icmp ult idx, len` against the live +/// length, which rejects a negative index as a huge unsigned one and takes the +/// side exit — so non-negativity is not needed as a static fact either. +/// +/// What IS needed is that the value materialises without wrapping, which the +/// lowering guarantees by computing the index in i64 from operands that are +/// each a proven i32 (`emit_object_array_write_index_i64` is the existing +/// precedent for the same arithmetic). +/// +/// Leaves: the counter, any local proven integer-valued, and integer literals. +/// A non-counter local must additionally be loop-invariant — checked by the +/// caller, which has the loop body — because a leaf written inside the body +/// would make the index and the emitted i64 recomputation disagree. +/// Does `expr` read `id` anywhere? Used to keep the affine index arm to +/// counter-dependent indices; a wholly loop-invariant index belongs to a tier +/// that can prove a compile-time window for it. +fn expr_mentions_local(expr: &perry_hir::Expr, id: u32) -> bool { + use perry_hir::Expr; + if matches!(expr, Expr::LocalGet(found) if *found == id) { + return true; + } + let mut found = false; + perry_hir::walker::walk_expr_children(expr, &mut |child| { + if !found { + found = expr_mentions_local(child, id); + } + }); + found +} + +fn packed_f64_range_loop_index_is_affine_with( + index: &perry_hir::Expr, + counter_id: u32, + leaf_ok: &dyn Fn(u32) -> bool, +) -> bool { + use perry_hir::{BinaryOp, Expr}; + match index { + Expr::Integer(v) => i32::try_from(*v).is_ok(), + Expr::Number(n) => { + n.fract() == 0.0 && *n >= f64::from(i32::MIN) && *n <= f64::from(i32::MAX) + } + Expr::LocalGet(id) => *id == counter_id || leaf_ok(*id), + // Only the operators whose i64 recomputation is exact. `Div`/`Mod`/ + // shifts are excluded: they are not wrong here so much as unproven, + // and admitting a shape the lowering then declines would emit a helper + // call and cost the whole clone (the #9259 cascade). + Expr::Binary { + op: BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul, + left, + right, + } => { + packed_f64_range_loop_index_is_affine_with(left, counter_id, leaf_ok) + && packed_f64_range_loop_index_is_affine_with(right, counter_id, leaf_ok) + } + _ => false, + } +} + +/// Record an affine (window-less) access. Unlike the counter/static recorders +/// this widens nothing: there is no window to merge, and the entry guard will +/// prove the receiver rather than a range. +fn record_packed_f64_range_affine_access( + accesses: &mut std::collections::BTreeMap, + array_id: u32, +) { + let entry = accesses + .entry(array_id) + .or_insert(PackedF64RangeArrayAccess { + array_id, + counter: None, + stat: None, + affine: false, + written: false, + }); + entry.affine = true; +} + fn record_packed_f64_range_access( accesses: &mut std::collections::BTreeMap, array_id: u32, @@ -1443,6 +1598,7 @@ fn record_packed_f64_range_access( array_id, counter: None, stat: None, + affine: false, written, }); entry.counter = Some(match entry.counter { @@ -1479,6 +1635,7 @@ pub(super) fn record_packed_f64_range_static_access( array_id, counter: None, stat: None, + affine: false, written: false, }); entry.stat = Some(match entry.stat { @@ -1524,6 +1681,7 @@ fn packed_f64_range_loop_body_collect( counter_id: u32, bound_local: Option, accesses: &mut std::collections::BTreeMap, + affine_leaf_ok: Option<&dyn Fn(u32) -> bool>, ) -> bool { use perry_hir::Expr; let [Stmt::Expr(expr)] = body else { @@ -1557,10 +1715,22 @@ fn packed_f64_range_loop_body_collect( Expr::LocalSet(id, value) => { *id != counter_id && Some(*id) != bound_local - && packed_f64_range_loop_pure_expr_collect(value, counter_id, false, accesses) + && packed_f64_range_loop_pure_expr_collect( + value, + counter_id, + false, + accesses, + affine_leaf_ok, + ) && !accesses.contains_key(id) } - _ => packed_f64_range_loop_pure_expr_collect(expr, counter_id, false, accesses), + _ => packed_f64_range_loop_pure_expr_collect( + expr, + counter_id, + false, + accesses, + affine_leaf_ok, + ), } } @@ -1616,7 +1786,7 @@ fn packed_f64_range_loop_store_collect( let Some(offset) = packed_f64_range_loop_index_offset(index, counter_id) else { return false; }; - if !packed_f64_range_loop_pure_expr_collect(value, counter_id, false, accesses) { + if !packed_f64_range_loop_pure_expr_collect(value, counter_id, false, accesses, None) { return false; } record_packed_f64_range_access(accesses, *arr_id, offset, true); @@ -1637,6 +1807,7 @@ fn packed_f64_range_loop_dense_body_collect( counter_id: u32, bound_local: Option, accesses: &mut std::collections::BTreeMap, + pending_accumulators: &mut std::collections::BTreeSet, ) -> bool { let mut written: std::collections::HashSet = std::collections::HashSet::new(); packed_f64_range_loop_dense_stmts_collect( @@ -1646,6 +1817,7 @@ fn packed_f64_range_loop_dense_body_collect( bound_local, accesses, &mut written, + pending_accumulators, ) // Written arrays are allowed (masked stores above); a scalar `let`/set // shadowing a tracked array id still rejects. @@ -1666,6 +1838,7 @@ fn packed_f64_range_loop_dense_stmts_collect( bound_local: Option, accesses: &mut std::collections::BTreeMap, written: &mut std::collections::HashSet, + pending_accumulators: &mut std::collections::BTreeSet, ) -> bool { use perry_hir::Expr; for stmt in body { @@ -1676,7 +1849,9 @@ fn packed_f64_range_loop_dense_stmts_collect( .. } => { if !masked_window_expression_is_non_collecting(ctx, init) - || !packed_f64_range_loop_pure_expr_collect(init, counter_id, true, accesses) + || !packed_f64_range_loop_pure_expr_collect( + init, counter_id, true, accesses, None, + ) { return false; } @@ -1689,8 +1864,20 @@ fn packed_f64_range_loop_dense_stmts_collect( if *id == counter_id || Some(*id) == bound_local { return false; } - if !masked_window_expression_is_non_collecting(ctx, value) - || !packed_f64_range_loop_pure_expr_collect(value, counter_id, true, accesses) + // First try the plain proof; a self-accumulating write whose + // only unprovable leaf is the target itself retries with the + // target treated as numeric and records it as PENDING. The + // caller then verifies every pending id against + // `collect_numeric_accumulators` -- the same walk the lowering + // runs -- and rejects the whole dense match otherwise, so the + // clone never contains a write this assumption cannot cover. + if masked_window_expression_proof(ctx, value, None).is_none() { + if masked_window_expression_proof(ctx, value, Some(*id)).is_none() { + return false; + } + pending_accumulators.insert(*id); + } + if !packed_f64_range_loop_pure_expr_collect(value, counter_id, true, accesses, None) { return false; } @@ -1720,10 +1907,10 @@ fn packed_f64_range_loop_dense_stmts_collect( || !masked_window_expression_is_non_collecting(ctx, value) || !dense_masked_store_rhs_is_admissible(ctx, value, counter_id, accesses) || !packed_f64_range_loop_pure_expr_collect( - index, counter_id, true, accesses, + index, counter_id, true, accesses, None, ) || !packed_f64_range_loop_pure_expr_collect( - value, counter_id, true, accesses, + value, counter_id, true, accesses, None, ) { return false; @@ -1738,7 +1925,9 @@ fn packed_f64_range_loop_dense_stmts_collect( continue; } if !masked_window_expression_is_non_collecting(ctx, expr) - || !packed_f64_range_loop_pure_expr_collect(expr, counter_id, true, accesses) + || !packed_f64_range_loop_pure_expr_collect( + expr, counter_id, true, accesses, None, + ) { return false; } @@ -1772,7 +1961,7 @@ fn packed_f64_range_loop_dense_stmts_collect( } => { if !masked_window_expression_is_non_collecting(ctx, condition) || !packed_f64_range_loop_pure_expr_collect( - condition, counter_id, true, accesses, + condition, counter_id, true, accesses, None, ) { return false; @@ -1784,6 +1973,7 @@ fn packed_f64_range_loop_dense_stmts_collect( bound_local, accesses, written, + pending_accumulators, ) { return false; } @@ -1795,6 +1985,7 @@ fn packed_f64_range_loop_dense_stmts_collect( bound_local, accesses, written, + pending_accumulators, ) { return false; } @@ -1865,7 +2056,7 @@ pub(super) fn masked_window_expression_is_non_collecting( ctx: &FnCtx<'_>, expr: &perry_hir::Expr, ) -> bool { - masked_window_expression_proof(ctx, expr).is_some() + masked_window_expression_proof(ctx, expr, None).is_some() } /// Facts about a value whose evaluation has also been proved non-collecting. @@ -1884,6 +2075,7 @@ struct MaskedWindowExpressionProof { fn masked_window_expression_proof( ctx: &FnCtx<'_>, expr: &perry_hir::Expr, + accumulator: Option, ) -> Option { use perry_hir::{BinaryOp, CompareOp, Expr, UnaryOp}; let proof = |inert, numeric| MaskedWindowExpressionProof { inert, numeric }; @@ -1896,12 +2088,22 @@ fn masked_window_expression_proof( if !matches!(object.as_ref(), Expr::LocalGet(_)) { return None; } - masked_window_expression_proof(ctx, index)?; + masked_window_expression_proof(ctx, index, accumulator)?; Some(proof(true, true)) } Expr::Number(_) | Expr::Integer(_) => Some(proof(true, true)), Expr::Bool(_) | Expr::Null | Expr::Undefined => Some(proof(true, false)), - Expr::LocalGet(_) => { + Expr::LocalGet(id) => { + // A pending accumulator is numeric BY CONTRACT, not by static + // proof: the clone's entry emits a genuine-double tag check on it + // and branches to the slow copy otherwise, and the caller verifies + // (with the same collector the lowering runs) that every in-clone + // write keeps it numeric. Static analysis cannot see this because + // the accumulator's own writes read the guarded array, whose + // element proof only exists once the guard has run. + if accumulator == Some(*id) { + return Some(proof(true, true)); + } let inert = crate::rooting::expr_is_inert_primitive(ctx, expr); Some(proof( inert, @@ -1915,8 +2117,8 @@ fn masked_window_expression_proof( crate::rooting::expr_is_inert_primitive(ctx, expr).then(|| proof(true, true)) } Expr::Binary { op, left, right } => { - let left = masked_window_expression_proof(ctx, left)?; - let right = masked_window_expression_proof(ctx, right)?; + let left = masked_window_expression_proof(ctx, left, accumulator)?; + let right = masked_window_expression_proof(ctx, right, accumulator)?; if matches!(op, BinaryOp::Add) { if !left.numeric || !right.numeric { return None; @@ -1927,23 +2129,23 @@ fn masked_window_expression_proof( Some(proof(true, true)) } Expr::Compare { op, left, right } => { - let left = masked_window_expression_proof(ctx, left)?; - let right = masked_window_expression_proof(ctx, right)?; + let left = masked_window_expression_proof(ctx, left, accumulator)?; + let right = masked_window_expression_proof(ctx, right, accumulator)?; if !matches!(op, CompareOp::Eq | CompareOp::Ne) && (!left.inert || !right.inert) { return None; } Some(proof(true, false)) } Expr::Unary { op, operand } => { - let operand = masked_window_expression_proof(ctx, operand)?; + let operand = masked_window_expression_proof(ctx, operand, accumulator)?; if !matches!(op, UnaryOp::Not) && !operand.inert { return None; } Some(proof(true, !matches!(op, UnaryOp::Not))) } Expr::Logical { left, right, .. } => { - let left = masked_window_expression_proof(ctx, left)?; - let right = masked_window_expression_proof(ctx, right)?; + let left = masked_window_expression_proof(ctx, left, accumulator)?; + let right = masked_window_expression_proof(ctx, right, accumulator)?; Some(proof( left.inert && right.inert, left.numeric && right.numeric, @@ -1954,25 +2156,25 @@ fn masked_window_expression_proof( then_expr, else_expr, } => { - masked_window_expression_proof(ctx, condition)?; - let then_expr = masked_window_expression_proof(ctx, then_expr)?; - let else_expr = masked_window_expression_proof(ctx, else_expr)?; + masked_window_expression_proof(ctx, condition, accumulator)?; + let then_expr = masked_window_expression_proof(ctx, then_expr, accumulator)?; + let else_expr = masked_window_expression_proof(ctx, else_expr, accumulator)?; Some(proof( then_expr.inert && else_expr.inert, then_expr.numeric && else_expr.numeric, )) } Expr::Void(value) | Expr::TypeOf(value) | Expr::BooleanCoerce(value) => { - masked_window_expression_proof(ctx, value)?; + masked_window_expression_proof(ctx, value, accumulator)?; Some(proof(true, false)) } Expr::NumberCoerce(value) => { - let value = masked_window_expression_proof(ctx, value)?; + let value = masked_window_expression_proof(ctx, value, accumulator)?; value.inert.then(|| proof(true, true)) } Expr::MathImul(left, right) | Expr::MathPow(left, right) => { for value in [left.as_ref(), right.as_ref()] { - if !masked_window_expression_proof(ctx, value)?.inert { + if !masked_window_expression_proof(ctx, value, accumulator)?.inert { return None; } } @@ -1980,7 +2182,7 @@ fn masked_window_expression_proof( } Expr::MathMin(values) | Expr::MathMax(values) => { for value in values { - if !masked_window_expression_proof(ctx, value)?.inert { + if !masked_window_expression_proof(ctx, value, accumulator)?.inert { return None; } } @@ -1994,7 +2196,7 @@ fn masked_window_expression_proof( | Expr::MathTrunc(value) | Expr::MathSign(value) | Expr::MathF16round(value) => { - let value = masked_window_expression_proof(ctx, value)?; + let value = masked_window_expression_proof(ctx, value, accumulator)?; if !value.inert { return None; } @@ -2014,6 +2216,7 @@ pub(super) fn packed_f64_range_loop_pure_expr_collect( counter_id: u32, allow_static: bool, accesses: &mut std::collections::BTreeMap, + affine_leaf_ok: Option<&dyn Fn(u32) -> bool>, ) -> bool { use perry_hir::Expr; match expr { @@ -2025,6 +2228,23 @@ pub(super) fn packed_f64_range_loop_pure_expr_collect( record_packed_f64_range_access(accesses, *arr_id, offset, false); return true; } + // #9253: an affine integer index (`a[i * size + k]`). Classic mode + // only — the caller passes `None` for dense, whose loads carry no + // side exit and so cannot take a per-read bounds check. + if let Some(leaf_ok) = affine_leaf_ok { + // The index must actually involve the counter. A constant or + // wholly loop-invariant index (`a[0]`, `a[1]`) is affine by the + // grammar but has a compile-time window, and the DENSE tier's + // masked/static-window path serves it better — admitting it + // here makes the classic walker succeed and silently steals the + // loop from that tier, which is a regression, not a win. + if packed_f64_range_loop_index_is_affine_with(index, counter_id, leaf_ok) + && expr_mentions_local(index, counter_id) + { + record_packed_f64_range_affine_access(accesses, *arr_id); + return true; + } + } if !allow_static { return false; } @@ -2035,7 +2255,13 @@ pub(super) fn packed_f64_range_loop_pure_expr_collect( return false; } // The index may nest further tracked reads — walk it too. - if !packed_f64_range_loop_pure_expr_collect(index, counter_id, allow_static, accesses) { + if !packed_f64_range_loop_pure_expr_collect( + index, + counter_id, + allow_static, + accesses, + affine_leaf_ok, + ) { return false; } record_packed_f64_range_static_access(accesses, *arr_id, lo, hi); @@ -2050,51 +2276,68 @@ pub(super) fn packed_f64_range_loop_pure_expr_collect( Expr::Binary { left, right, .. } | Expr::Compare { left, right, .. } | Expr::Logical { left, right, .. } => { - packed_f64_range_loop_pure_expr_collect(left, counter_id, allow_static, accesses) - && packed_f64_range_loop_pure_expr_collect( - right, - counter_id, - allow_static, - accesses, - ) + packed_f64_range_loop_pure_expr_collect( + left, + counter_id, + allow_static, + accesses, + affine_leaf_ok, + ) && packed_f64_range_loop_pure_expr_collect( + right, + counter_id, + allow_static, + accesses, + affine_leaf_ok, + ) } Expr::Unary { operand, .. } | Expr::Void(operand) | Expr::TypeOf(operand) | Expr::NumberCoerce(operand) - | Expr::BooleanCoerce(operand) => { - packed_f64_range_loop_pure_expr_collect(operand, counter_id, allow_static, accesses) - } + | Expr::BooleanCoerce(operand) => packed_f64_range_loop_pure_expr_collect( + operand, + counter_id, + allow_static, + accesses, + affine_leaf_ok, + ), Expr::Conditional { condition, then_expr, else_expr, } => { - packed_f64_range_loop_pure_expr_collect(condition, counter_id, allow_static, accesses) - && packed_f64_range_loop_pure_expr_collect( - then_expr, - counter_id, - allow_static, - accesses, - ) - && packed_f64_range_loop_pure_expr_collect( - else_expr, - counter_id, - allow_static, - accesses, - ) + packed_f64_range_loop_pure_expr_collect( + condition, + counter_id, + allow_static, + accesses, + affine_leaf_ok, + ) && packed_f64_range_loop_pure_expr_collect( + then_expr, + counter_id, + allow_static, + accesses, + affine_leaf_ok, + ) && packed_f64_range_loop_pure_expr_collect( + else_expr, + counter_id, + allow_static, + accesses, + affine_leaf_ok, + ) } Expr::MathImul(left, right) | Expr::MathPow(left, right) => { - packed_f64_range_loop_pure_expr_collect(left, counter_id, allow_static, accesses) + packed_f64_range_loop_pure_expr_collect(left, counter_id, allow_static, accesses, None) && packed_f64_range_loop_pure_expr_collect( right, counter_id, allow_static, accesses, + affine_leaf_ok, ) } Expr::MathMin(values) | Expr::MathMax(values) => values.iter().all(|expr| { - packed_f64_range_loop_pure_expr_collect(expr, counter_id, allow_static, accesses) + packed_f64_range_loop_pure_expr_collect(expr, counter_id, allow_static, accesses, None) }), Expr::MathAbs(value) | Expr::MathSqrt(value) @@ -2104,7 +2347,7 @@ pub(super) fn packed_f64_range_loop_pure_expr_collect( | Expr::MathTrunc(value) | Expr::MathSign(value) | Expr::MathF16round(value) => { - packed_f64_range_loop_pure_expr_collect(value, counter_id, allow_static, accesses) + packed_f64_range_loop_pure_expr_collect(value, counter_id, allow_static, accesses, None) } _ => false, } @@ -2135,6 +2378,32 @@ fn emit_packed_f64_range_guards( "array[packed_f64_range_loop]", TypedFeedbackContract::packed_f64_array_loop(), ); + // #9253: an affine access has no window to validate. The 2-arg + // receiver guard proves exactly what is hoistable — plain-array shape, + // raw-f64 packedness, integrity/frozen state, the 16M length and + // capacity sanity bounds — once in the preheader, which is the whole + // point: those are the per-iteration instructions this issue is about. + // The index itself is validated per read by `icmp ult idx, len`. + if access.affine { + let guard_i32 = ctx.block().call( + I32, + "js_typed_feedback_packed_f64_array_loop_guard", + &[(I64, &feedback_site_id), (DOUBLE, &arr_box)], + ); + let guard_ok = ctx.block().icmp_ne(I32, &guard_i32, "0"); + all_guards_ok = Some(match all_guards_ok { + None => guard_ok, + Some(prev) => ctx.block().and(I1, &prev, &guard_ok), + }); + record_packed_f64_loop_guard_artifacts( + ctx, + access.array_id, + &arr_box, + guard_id, + PackedNumericLoopKind::F64, + ); + continue; + } let (min_idx, max_idx): (String, String) = match (access.counter, access.stat) { (Some((min_off, max_off)), None) => ( (matched.start + i64::from(min_off)).to_string(), @@ -2211,6 +2480,24 @@ fn push_packed_f64_range_facts( // hole-tolerant. allow_holes: !matched.dense, window_validated: true, + affine_indices: false, + numeric_accumulators: numeric_accumulators.to_vec(), + }); + } + // #9253: an affine access publishes a receiver-only fact. No window + // was validated, so reads bounds-check per access; holes are excluded + // because the receiver guard proves fully-packed raw f64. + if access.affine { + ctx.packed_f64_loop_facts.push(PackedF64LoopFact { + index_local_id: matched.counter_id, + array_local_id: access.array_id, + scope_id, + guard_id: guard_id.to_string(), + store_side_exit_label: slow_pre_label.to_string(), + array_kind: PackedNumericLoopKind::F64, + allow_holes: false, + window_validated: false, + affine_indices: true, numeric_accumulators: numeric_accumulators.to_vec(), }); } @@ -2225,6 +2512,7 @@ fn push_packed_f64_range_facts( values_i32, elem: crate::expr::MaskedWindowElem::PlainF64, allows_stores: allow_masked_stores, + numeric_accumulators: numeric_accumulators.to_vec(), }); } } @@ -2327,6 +2615,7 @@ fn lower_masked_window_ta_tier( values_i32, elem, allows_stores: false, + numeric_accumulators: Vec::new(), }); } lower_for_after_init_with_i32_bound( diff --git a/crates/perry-codegen/src/stmt/masked_window_region.rs b/crates/perry-codegen/src/stmt/masked_window_region.rs index b0f5fce47b..15da4ed148 100644 --- a/crates/perry-codegen/src/stmt/masked_window_region.rs +++ b/crates/perry-codegen/src/stmt/masked_window_region.rs @@ -425,6 +425,7 @@ pub(super) fn try_match_masked_window_region( REGION_NO_COUNTER, true, &mut trial, + None, ) { accesses = trial; @@ -463,6 +464,7 @@ pub(super) fn try_match_masked_window_region( REGION_NO_COUNTER, true, &mut trial, + None, ) { accesses = trial; @@ -885,6 +887,7 @@ pub(super) fn lower_masked_window_region( values_i32: true, allows_stores: false, elem: MaskedWindowElem::TaI32 { data_ptr: data_i64 }, + numeric_accumulators: Vec::new(), }); } let privatize = ctx.try_depth == 0; @@ -1001,6 +1004,7 @@ pub(super) fn lower_masked_window_region( values_i32: true, allows_stores: false, elem: MaskedWindowElem::TaI32 { data_ptr }, + numeric_accumulators: Vec::new(), }); } let privatize = ctx.try_depth == 0; @@ -1035,6 +1039,7 @@ pub(super) fn lower_masked_window_region( values_i32: false, allows_stores: false, elem: MaskedWindowElem::PlainF64, + numeric_accumulators: Vec::new(), }); } lower_region_copy( diff --git a/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs b/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs index 2edce30b26..aad3081fbb 100644 --- a/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs +++ b/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs @@ -39,6 +39,7 @@ fn accumulator_rhs_is_numeric( array_id: u32, counter_id: u32, offset_reads_inlined: bool, + masked_reads_validated: bool, candidates: &std::collections::BTreeSet, ) -> bool { match expr { @@ -67,8 +68,21 @@ fn accumulator_rhs_is_numeric( // expression lowers to a tag-test diamond over // `js_dynamic_string_or_number_add` — the same cost #9060 and // #9091 removed for the bare-counter form. - _ if offset_reads_inlined => crate::expr::packed_f64_loop_index_parts(index) - .is_some_and(|(i, _)| i == counter_id), + // `_ if offset_reads_inlined` used to sit above the masked + // arm as its own guarded catch-all — which swallowed every + // non-offset index whenever the flag was set, so the masked + // test below it was unreachable. One combined catch-all keeps + // both reachable. + _ => { + (offset_reads_inlined + && crate::expr::packed_f64_loop_index_parts(index) + .is_some_and(|(i, _)| i == counter_id)) + // Dense masked mode: the entry guard validated the + // union of every static window hole-free, so an + // in-window read is a genuine Number. + || (masked_reads_validated + && crate::collectors::static_index_window(index).is_some()) + } _ => false, } } @@ -82,6 +96,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) && accumulator_rhs_is_numeric( ctx, @@ -89,6 +104,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) } @@ -98,6 +114,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ), Expr::Unary { op, operand } => { @@ -110,6 +127,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) } @@ -126,6 +144,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ), Expr::MathImul(l, r) | Expr::MathPow(l, r) => { @@ -135,6 +154,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) && accumulator_rhs_is_numeric( ctx, @@ -142,6 +162,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) } @@ -152,6 +173,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) }), @@ -305,6 +327,7 @@ pub(super) fn collect_numeric_accumulators( array_id: u32, counter_id: u32, offset_reads_inlined: bool, + masked_reads_validated: bool, ) -> Vec { if !packed_loop_numeric_accumulators_enabled() { return Vec::new(); @@ -337,6 +360,7 @@ pub(super) fn collect_numeric_accumulators( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, &candidates, ), // `Update` (++/--): ToNumeric(Number) ± 1 is a Number. diff --git a/crates/perry-codegen/src/stmt/stable_packed_loop.rs b/crates/perry-codegen/src/stmt/stable_packed_loop.rs index 98ad003f04..3d048b2763 100644 --- a/crates/perry-codegen/src/stmt/stable_packed_loop.rs +++ b/crates/perry-codegen/src/stmt/stable_packed_loop.rs @@ -1659,7 +1659,14 @@ pub(super) fn lower( let numeric_accumulators = if candidate.numeric_elements { // The stable-packed tier is left as it was; widening it needs its own // proof that an offset read lowers inline here. - collect_numeric_accumulators(ctx, body, candidate.array_id, candidate.counter_id, false) + collect_numeric_accumulators( + ctx, + body, + candidate.array_id, + candidate.counter_id, + false, + false, + ) } else { Vec::new() }; diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index b060149ae1..850f324cfc 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -189,6 +189,13 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { // after an entry tag check, and its sole write adds a proven // string length. The fact exists only while lowering that // clone, so the slow copy retains dynamic `+` semantics. + // The dense masked-window clone's twin: same entry tag + // check, same numeric-preserving write proof. + || ctx + .masked_window_array_facts + .iter() + .rev() + .any(|fact| fact.numeric_accumulators.contains(id)) || ctx .string_window_array_facts .iter() diff --git a/crates/perry/tests/dense_accumulator_masked_reads.rs b/crates/perry/tests/dense_accumulator_masked_reads.rs new file mode 100644 index 0000000000..a96c1e153a --- /dev/null +++ b/crates/perry/tests/dense_accumulator_masked_reads.rs @@ -0,0 +1,165 @@ +//! A float accumulator over masked reads earns the dense range clone. +//! +//! `sum = sum * x[i & 63] + x[(i * 7) & 63]` (the `17_loop_data_dependent` +//! shape) was rejected by the dense range tier while `sum = sum * x[i & 63]` +//! was admitted — the discriminator was the accumulator's static numeric +//! proof. `+` can be concatenation, so the per-statement proof demands both +//! operands numeric; a reassigned accumulator has no such proof, because its +//! own writes read the guarded array, whose element proof only exists once +//! the guard has run. Chicken-and-egg, broken previously only for `*`. +//! +//! The matcher now peels the accumulator: it retries the proof with the +//! `LocalSet` target treated as numeric BY CONTRACT, then verifies every such +//! pending local with the same collector the lowering runs. The contract is +//! enforced at run time twice over: the clone's entry emits a genuine-double +//! tag check on the accumulator (a string-seeded accumulator routes to the +//! slow copy), and the dense entry guard validates the whole masked window +//! hole-free (a string ELEMENT fails the guard the same way). +//! +//! Measured: 17_loop_data_dependent 505 ms -> 227 ms against node's 229 ms. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path, source: &str) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn fast_copies(stderr: &str) -> usize { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path) + .expect("read kept LLVM IR") + .lines() + .filter(|l| l.starts_with("packed_f64_range") && l.trim_end().ends_with(':')) + .count() +} + +fn run(bin: &Path, dir: &Path, moving_gc: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if moving_gc { + command + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1"); + } + command.output().expect("run compiled binary") +} + +fn assert_stdout(output: &Output, expected: &str, moving_gc: bool) { + assert!( + output.status.success(), + "binary failed with moving_gc={moving_gc}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(String::from_utf8_lossy(&output.stdout), expected); +} + +/// The data-dependent recurrence gets a dense fast copy, and its result is +/// bit-identical to node's across 200k iterations of float churn. +#[test] +fn a_masked_read_accumulator_earns_the_dense_clone_and_matches_node() { + let source = r#" +function run(x: number[]): number { + let sum = 1.0; + for (let i = 0; i < 200000; i++) sum = sum * x[i & 63] + x[(i * 7) & 63]; + return sum; +} +const x: number[] = []; +for (let i = 0; i < 64; i++) x.push(0.5 + i * 0.01); +console.log("r:" + run(x)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), source); + assert!( + fast_copies(&stderr) > 0, + "the accumulator peel must admit the dense clone; without it the whole \ + loop pays the per-access guard tier (505ms vs node's 229ms)" + ); + for moving_gc in [false, true] { + assert_stdout( + &run(&bin, dir.path(), moving_gc), + "r:44.18806624016606\n", + moving_gc, + ); + } +} + +/// The contract's first enforcement point: an accumulator seeded with a +/// STRING must take the entry tag check into the slow copy and produce node's +/// concatenation, not a raw fadd over a string box. +#[test] +fn a_string_seeded_accumulator_takes_the_slow_copy_and_concatenates() { + let source = r#" +function run(x: number[]): string { + let sum: any = "s"; + for (let i = 0; i < 4; i++) sum = sum + x[i & 63]; + return sum; +} +const x: number[] = []; +for (let i = 0; i < 64; i++) x.push(0.5 + i * 0.01); +console.log("r:" + run(x)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout( + &run(&bin, dir.path(), moving_gc), + "r:s0.50.510.520.53\n", + moving_gc, + ); + } +} + +/// The second enforcement point: a string ELEMENT makes the dense entry guard +/// fail (the window is not hole-free numeric), so the slow copy concatenates +/// exactly as node does. +#[test] +fn a_string_element_fails_the_guard_and_the_slow_copy_matches_node() { + let source = r#" +function run(x: any[]): any { + let sum: any = 0.0; + for (let i = 0; i < 4; i++) sum = sum + x[i & 63]; + return sum; +} +const y: any[] = []; +for (let i = 0; i < 64; i++) y.push(i === 2 ? "boom" : i * 1.0); +console.log("r:" + run(y)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout(&run(&bin, dir.path(), moving_gc), "r:1boom3\n", moving_gc); + } +} diff --git a/crates/perry/tests/issue_9253_affine_range_index.rs b/crates/perry/tests/issue_9253_affine_range_index.rs new file mode 100644 index 0000000000..f4035a8391 --- /dev/null +++ b/crates/perry/tests/issue_9253_affine_range_index.rs @@ -0,0 +1,165 @@ +//! Regression coverage for #9253: a counted loop whose reads use an AFFINE +//! index — `a[i * size + k]` — now hoists its receiver guard into the +//! preheader instead of re-deriving it on every access. +//! +//! `16_matrix_multiply`'s inner loop spends 97% of its time inside generated +//! code with no runtime calls, so the cost was never a missed inlining. Per +//! iteration, for BOTH receivers, it re-derived the pointer tag check, the +//! handle-band check, the header dereference, the `_reserved` flag tests and +//! the two 16,000,000 length/capacity sanity compares — for receivers that are +//! loop-invariant parameters whose headers cannot change inside the loop. +//! LLVM cannot hoist any of it: the guard reloads the header through a pointer +//! it cannot prove unaliased, and the incremental-barrier atomic read is a +//! motion barrier. +//! +//! The packed-f64 RANGE tier already had everything needed except the index +//! shape — a loop-invariant local/parameter bound, N-array guard emission, and +//! GC-safe receiver caching refreshed at the back-edge poll. This adds the +//! affine index: the entry guard proves the RECEIVER once, and each read pays +//! one inline `icmp ult idx, len` with a side exit. +//! +//! Measured on an idle Mac mini, self-timed min of 7: 100 ms -> 69 ms against +//! node's 33 ms (3.03x -> 2.09x). + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path, source: &str) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn ir(stderr: &str) -> String { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path).expect("read kept LLVM IR") +} + +fn run(bin: &Path, dir: &Path, moving_gc: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if moving_gc { + command + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1"); + } + command.output().expect("run compiled binary") +} + +fn assert_stdout(output: &Output, expected: &str, moving_gc: bool) { + assert!( + output.status.success(), + "binary failed with moving_gc={moving_gc}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(String::from_utf8_lossy(&output.stdout), expected); +} + +const MATMUL: &str = r#" +function matmul(a: number[], b: number[], size: number): number { + let acc = 0.0; + for (let i = 0; i < size; i++) + for (let j = 0; j < size; j++) { + let sum = 0.0; + for (let k = 0; k < size; k++) sum = sum + a[i * size + k] * b[k * size + j]; + acc = acc + sum; + } + return acc; +} +const a: number[] = []; const b: number[] = []; +for (let i = 0; i < 64; i++) { a.push(i % 7); b.push(i % 5); } +console.log("matmul:" + matmul(a, b, 8)); +"#; + +/// The affine index shape reaches the clone at all. Without this the guard is +/// re-derived per access for both receivers, which is the whole issue. +#[test] +fn an_affine_index_admits_the_range_clone_and_agrees_with_the_generic_path() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), MATMUL); + let text = ir(&stderr); + assert!( + text.contains("packed_f64_affine"), + "#9253: `a[i * size + k]` must reach the affine read lowering; without it \ + the receiver guard re-executes per access" + ); + for moving_gc in [false, true] { + assert_stdout(&run(&bin, dir.path(), moving_gc), "matmul:2983\n", moving_gc); + } +} + +/// An affine index that runs past the end must take the per-read side exit and +/// produce node's `undefined` semantics, not a raw out-of-bounds load. This is +/// the assertion that the bounds check is real: the entry guard validated the +/// receiver, NOT any index window, so nothing else stands between the affine +/// index and the element storage. +#[test] +fn an_affine_index_past_the_end_side_exits_to_undefined() { + let source = r#" +function overrun(a: number[], size: number): number { + let s = 0.0; + for (let k = 0; k < size; k++) s = s + a[k * 3 + 2]; + return s; +} +const a: number[] = []; +for (let i = 0; i < 64; i++) a.push(i % 7); +console.log("overrun:" + overrun(a, 40)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout(&run(&bin, dir.path(), moving_gc), "overrun:NaN\n", moving_gc); + } +} + +/// A negative affine index. The inline check is an UNSIGNED compare, so a +/// negative value reads as a huge unsigned one and exits rather than indexing +/// backwards out of the element storage. +#[test] +fn a_negative_affine_index_side_exits_instead_of_reading_backwards() { + let source = r#" +function negative(a: number[], size: number): number { + let s = 0.0; + for (let k = 0; k < size; k++) { const v = a[k - 3]; if (v === undefined) s = s + 1.0; } + return s; +} +const a: number[] = []; +for (let i = 0; i < 64; i++) a.push(i % 7); +console.log("negative:" + negative(a, 20)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout(&run(&bin, dir.path(), moving_gc), "negative:3\n", moving_gc); + } +}