diff --git a/changelog.d/affine-window-hoist-and-accumulator-set.md b/changelog.d/affine-window-hoist-and-accumulator-set.md new file mode 100644 index 0000000000..eab1c7622c --- /dev/null +++ b/changelog.d/affine-window-hoist-and-accumulator-set.md @@ -0,0 +1,31 @@ +**`16_matrix_multiply` is now twice as fast as node** (100 ms → 16 ms against +node's 32 on an idle machine; checksums identical) — the last benchmark in +the suite crosses parity, via two changes and one guard fix. + +**The affine window is proven at the loop's endpoints.** An index tree linear +in the counter takes its extremes at the interval's ends, so the entry guard +evaluates each recorded tree at `start` and at `bound − 1` (wrap-free by the +magnitude bound) and unsigned-compares both against the live length — two +compares per tree, once per loop entry, for any coefficient sign. Reads under +a proven window drop both the range clamp and the per-read bounds check, +leaving a bare `trunc` and raw load. Non-linear trees (`k * k`) keep their +per-read checks. + +**The accumulator walk takes the guarded array set.** The old single-array +restriction was why `sum = sum + a[..] * b[..]` never earned its number +proof: every k-iteration wrote `sum` through a boxed shadow-slot store with a +barrier, which profiling showed was the actual bottleneck once the guards +were hoisted — removing the per-read checks alone moved nothing. Every array +in the set is validated by the same AND-reduced entry guard, so a read of any +of them inside the clone is a Number by the same argument that held for one; +affine reads qualify as numeric leaves through the same shared predicate the +matcher and the lowering use (`affine_leaf_admissible`), so the three cannot +drift. + +**A guard hole in the #9294 arm is closed.** Its receiver-only `continue` +also fired for arrays with BOTH counter-offset and affine accesses, skipping +the windowed guard while the counter fact still said `window_validated: +true` — `a[k + 1]` alongside `a[i * size + k]` then read one raw slot past +the loop's window at the boundary. Mixed arrays now fall through to the +windowed guard, with the affine endpoint proof appended to either path, and +a test pins the mixed shape to node's NaN under both collector modes. diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index 21a4b00ba1..b4eae7b1dd 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -42,7 +42,10 @@ use super::{ mod foreign_counter; mod guarded_array; -pub(crate) use foreign_counter::{affine_index_fits_i64, packed_f64_loop_index_parts}; +pub(crate) use foreign_counter::{ + affine_counter_occurrences, affine_index_fits_i64, emit_affine_index_i64_with, + packed_f64_loop_index_parts, +}; use foreign_counter::{ affine_packed_loop_read, emit_affine_index_i64, foreign_packed_loop_read, packed_f64_loop_offset_read, @@ -787,6 +790,18 @@ pub(crate) fn lower_numeric_index_get_for_number_context( 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) { + // Window proven at the loop's endpoints by the entry guard: a + // linear tree's value stays between its endpoint evaluations, + // both checked `< length <= 16M < 2^31`, so the clamp and the + // per-read bounds check are both implied — the read is a bare + // trunc + raw load, which is what lets LLVM strength-reduce + // and vectorize the loop. + if fact.window_validated { + 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, false, + ))); + } // 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. 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 c42f52e089..af52b7ae39 100644 --- a/crates/perry-codegen/src/expr/index_get/foreign_counter.rs +++ b/crates/perry-codegen/src/expr/index_get/foreign_counter.rs @@ -117,6 +117,24 @@ pub(crate) fn packed_f64_loop_offset_read( /// computed in i128 at match time, so admission costs nothing at run time: /// a tree whose worst case fits i63 cannot wrap, and anything else declines /// to the generic path. + +/// How many times the counter appears in an affine tree. A tree LINEAR in +/// the counter (exactly one occurrence, and only under Add/Sub/Mul with +/// invariant co-factors) takes its extreme values at the loop's endpoints, +/// which is what licenses the endpoint window proof in the entry guard. A +/// tree where the counter appears twice (`k * k`) is not monotone and keeps +/// its per-read bounds check. +pub(crate) fn affine_counter_occurrences(index: &Expr, counter_id: u32) -> u32 { + match index { + Expr::LocalGet(id) if *id == counter_id => 1, + Expr::Binary { left, right, .. } => { + affine_counter_occurrences(left, counter_id) + + affine_counter_occurrences(right, counter_id) + } + _ => 0, + } +} + pub(crate) fn affine_index_magnitude_bound(index: &Expr) -> Option { match index { Expr::Integer(v) => Some((*v as i128).unsigned_abs().min(1 << 31) as i128), @@ -196,22 +214,37 @@ pub(crate) fn emit_affine_index_i64( ctx: &mut FnCtx<'_>, index: &Expr, counter_id: u32, +) -> Option { + emit_affine_index_i64_with(ctx, index, counter_id, None) +} + +/// Like [`emit_affine_index_i64`], with the counter's value optionally +/// substituted by a caller-provided i64 SSA value instead of its slot load. +/// The entry guard uses this to evaluate an affine tree at the loop's +/// endpoints (counter = start, counter = bound - 1) while every other leaf +/// still reads its live, loop-invariant slot. +pub(crate) fn emit_affine_index_i64_with( + ctx: &mut FnCtx<'_>, + index: &Expr, + counter_id: u32, + counter_override: Option<&str>, ) -> 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() - }; + if *id == counter_id { + if let Some(value) = counter_override { + return Some(value.to_string()); + } + } + let slot = 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)?; + let l = emit_affine_index_i64_with(ctx, left, counter_id, counter_override)?; + let r = emit_affine_index_i64_with(ctx, right, counter_id, counter_override)?; Some(match op { perry_hir::BinaryOp::Add => ctx.block().add(I64, &l, &r), perry_hir::BinaryOp::Sub => ctx.block().sub(I64, &l, &r), diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index e4f65bee03..65d802f026 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2746,7 +2746,8 @@ mod unary_bigint_tests; #[cfg(test)] mod unary_bitnot_tests; pub(crate) use index_get::{ - affine_index_fits_i64, numeric_index_has_integer_array_index_proof, packed_f64_loop_index_parts, + affine_counter_occurrences, affine_index_fits_i64, emit_affine_index_i64_with, + numeric_index_has_integer_array_index_proof, packed_f64_loop_index_parts, }; pub(crate) use masked_window::masked_window_fact_for_index; /// Rooting coverage for the computed-store arms the TS corpora cannot reach diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 231ff185e3..0422e189cd 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -781,21 +781,21 @@ fn emit_range_loop_accumulator_admission( slow_pre_label: &str, block_prefix: &str, ) -> PackedAccumulatorScope { - // 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(), - }; + // The accumulator walk now takes the whole guarded array SET — the old + // single-array restriction ("no admitted leaf spans them all") was the + // reason `sum = sum + a[..] * b[..]` never earned its number proof, and + // every k-iteration of matmul wrote `sum` through a boxed shadow-slot + // store with a barrier. Every array in the set is validated by the same + // AND-reduced entry guard, so a read of any of them inside the clone is a + // Number by the same argument that held for one. + let array_ids: std::collections::BTreeSet = + matched.arrays.iter().map(|a| a.array_id).collect(); + let masked_reads_validated = matched.arrays.iter().any(|a| a.stat.is_some()); + let affine_reads = matched.arrays.iter().any(|a| a.affine); emit_packed_numeric_accumulator_admission( ctx, body, - array_id, + &array_ids, matched.counter_id, slow_pre_label, block_prefix, @@ -804,6 +804,7 @@ fn emit_range_loop_accumulator_admission( // Number. See `accumulator_rhs_is_numeric`. true, masked_reads_validated, + affine_reads, ) } @@ -1112,20 +1113,22 @@ pub(crate) fn flush_packed_accumulator_locals(ctx: &mut FnCtx<'_>) { fn emit_packed_numeric_accumulator_admission( ctx: &mut FnCtx<'_>, body: &[Stmt], - array_id: u32, + array_ids: &std::collections::BTreeSet, counter_id: u32, slow_pre_label: &str, block_prefix: &str, offset_reads_inlined: bool, masked_reads_validated: bool, + affine_reads: bool, ) -> PackedAccumulatorScope { let accumulators = super::stable_packed_accumulator::collect_numeric_accumulators( ctx, body, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, ); // Integer (`c++`) accumulators admit independently of the float set — // a pure count loop has no float accumulator at all. @@ -1302,7 +1305,7 @@ fn lower_packed_f64_versioned_for( let mut acc_scope = emit_packed_numeric_accumulator_admission( ctx, body, - matched.array_id, + &std::collections::BTreeSet::from([matched.array_id]), matched.counter_id, &slow_pre_label, loop_label, @@ -1314,6 +1317,8 @@ fn lower_packed_f64_versioned_for( // No masked windows either: this tier's guard proves the receiver and // the length bound, not a hole-free window. false, + // And no affine reads: this tier's facts publish affine_indices:false. + false, ); acc_scope.hoist_receivers(ctx, &[matched.array_id]); ctx.packed_f64_loop_facts.push(PackedF64LoopFact { @@ -1392,7 +1397,7 @@ enum PackedF64RangeLoopBound { Local(u32), } -#[derive(Clone, Copy)] +#[derive(Clone)] pub(super) struct PackedF64RangeArrayAccess { pub(super) array_id: u32, /// Counter-relative accesses: smallest / largest constant offset `c` over @@ -1409,6 +1414,14 @@ pub(super) struct PackedF64RangeArrayAccess { /// 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, + /// The affine index trees themselves, recorded so the entry guard can try + /// to prove the loop's whole index WINDOW: a tree linear in the counter + /// takes its extremes at the loop's endpoints, so two preheader + /// evaluations (counter = start, counter = bound - 1) bound every index + /// the loop touches, and one unsigned compare each against the length + /// licenses bounds-free raw loads in the clone. #9318's magnitude bound + /// makes the preheader evaluations wrap-free by construction. + pub(super) affine_exprs: Vec, pub(super) written: bool, } @@ -1611,7 +1624,13 @@ fn match_packed_f64_range_loop( } 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, + ctx, + body, + &std::collections::BTreeSet::from([array_id]), + counter_id, + true, + true, + false, ); if !pending_accumulators.iter().all(|id| admitted.contains(id)) { return range_loop_reject("accumulator_not_provable"); @@ -1785,6 +1804,26 @@ fn expr_mentions_local(expr: &perry_hir::Expr, id: u32) -> bool { found } +/// The one affine-index admissibility test shared by the range matcher, the +/// read lowering, and the accumulator walk. If any of the three drifts, the +/// clone can contain a read the others assumed away (#9259's cascade shape), +/// so they all call this. +pub(super) fn affine_leaf_admissible( + ctx: &FnCtx<'_>, + index: &perry_hir::Expr, + counter_id: u32, +) -> bool { + let leaf_ok = |id: u32| -> bool { + ctx.integer_locals.contains(&id) + && ctx.i32_counter_slots.contains_key(&id) + && !ctx.boxed_vars.contains(&id) + && !ctx.closure_captures.contains_key(&id) + }; + packed_f64_range_loop_index_is_affine_with(index, counter_id, &leaf_ok) + && expr_mentions_local(index, counter_id) + && crate::expr::affine_index_fits_i64(index) +} + fn packed_f64_range_loop_index_is_affine_with( index: &perry_hir::Expr, counter_id: u32, @@ -1819,6 +1858,7 @@ fn packed_f64_range_loop_index_is_affine_with( fn record_packed_f64_range_affine_access( accesses: &mut std::collections::BTreeMap, array_id: u32, + index: &perry_hir::Expr, ) { let entry = accesses .entry(array_id) @@ -1827,9 +1867,11 @@ fn record_packed_f64_range_affine_access( counter: None, stat: None, affine: false, + affine_exprs: Vec::new(), written: false, }); entry.affine = true; + entry.affine_exprs.push(index.clone()); } fn record_packed_f64_range_access( @@ -1845,6 +1887,7 @@ fn record_packed_f64_range_access( counter: None, stat: None, affine: false, + affine_exprs: Vec::new(), written, }); entry.counter = Some(match entry.counter { @@ -1882,6 +1925,7 @@ pub(super) fn record_packed_f64_range_static_access( counter: None, stat: None, affine: false, + affine_exprs: Vec::new(), written: false, }); entry.stat = Some(match entry.stat { @@ -2491,7 +2535,7 @@ pub(super) fn packed_f64_range_loop_pure_expr_collect( // the lowering so the two cannot disagree. && crate::expr::affine_index_fits_i64(index) { - record_packed_f64_range_affine_access(accesses, *arr_id); + record_packed_f64_range_affine_access(accesses, *arr_id, index); return true; } } @@ -2618,8 +2662,9 @@ fn emit_packed_f64_range_guards( bound_i32: &str, guard_fn: &str, guard_id: &str, -) -> Result { +) -> Result<(String, std::collections::BTreeSet)> { let mut all_guards_ok: Option = None; + let mut affine_window_proven: std::collections::BTreeSet = Default::default(); for access in &matched.arrays { let arr_box = lower_expr(ctx, &perry_hir::Expr::LocalGet(access.array_id))?; let feedback_site_id = emit_typed_feedback_register_site( @@ -2628,13 +2673,19 @@ 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 { + // #9253: an affine access has no static window. A PURELY affine + // array takes the 2-arg receiver guard (plain-array shape, raw-f64 + // packedness, integrity, the 16M sanity bounds — the per-iteration + // instructions the issue measured). An array that ALSO has counter + // or masked accesses must NOT skip the windowed guard below: #9294 + // took this `continue` for mixed arrays too, which left their + // counter windows unvalidated while the counter fact still said + // `window_validated: true` — `a[k + 1]` then read one raw slot past + // the end at the boundary. Mixed arrays now fall through to the + // 4-arg windowed guard, and the affine endpoint proof is appended + // to either path. + let purely_affine = access.affine && access.counter.is_none() && access.stat.is_none(); + if purely_affine { let guard_i32 = ctx.block().call( I32, "js_typed_feedback_packed_f64_array_loop_guard", @@ -2652,6 +2703,69 @@ fn emit_packed_f64_range_guards( guard_id, PackedNumericLoopKind::F64, ); + } + // Affine WINDOW proof: a tree linear in the counter takes its + // extremes at the endpoints, so evaluating it at `start` and at + // `bound - 1` and unsigned-comparing both against the live length + // bounds every index the loop touches — any coefficient sign, since + // a linear function's extremes are at the interval's ends, and a + // negative value reads as huge unsigned. Every leaf is i32 and the + // #9318 magnitude bound admitted the tree, so the i64 endpoint + // evaluations cannot wrap. Non-linear trees (`k * k`) keep their + // per-read checks; the fact records which case this array is in. + if access.affine { + let all_linear = access + .affine_exprs + .iter() + .all(|e| crate::expr::affine_counter_occurrences(e, matched.counter_id) == 1); + if all_linear && !access.affine_exprs.is_empty() { + let (len64, start_i64, end_minus_1) = { + let blk = ctx.block(); + let arr_bits = blk.bitcast_double_to_i64(&arr_box); + let arr_handle = blk.and(I64, &arr_bits, crate::nanbox::POINTER_MASK_I64); + let arr_ptr = blk.inttoptr(I64, &arr_handle); + let len32 = blk.load(I32, &arr_ptr); + let len64 = blk.zext(I32, &len32, I64); + let end64 = blk.sext(I32, bound_i32, I64); + let end_minus_1 = blk.sub(I64, &end64, "1"); + (len64, matched.start.to_string(), end_minus_1) + }; + let mut window_ok: Option = None; + let mut window_failed = false; + 'exprs: for expr in &access.affine_exprs { + for endpoint in [start_i64.as_str(), end_minus_1.as_str()] { + let value = match crate::expr::emit_affine_index_i64_with( + ctx, + expr, + matched.counter_id, + Some(endpoint), + ) { + Some(value) => value, + None => { + window_failed = true; + break 'exprs; + } + }; + let in_bounds = ctx.block().icmp_ult(I64, &value, &len64); + window_ok = Some(match window_ok { + None => in_bounds, + Some(prev) => ctx.block().and(I1, &prev, &in_bounds), + }); + } + } + if window_failed { + window_ok = None; + } + if let Some(ok) = window_ok { + all_guards_ok = Some(match all_guards_ok { + None => ok, + Some(prev) => ctx.block().and(I1, &prev, &ok), + }); + affine_window_proven.insert(access.array_id); + } + } + } + if purely_affine { continue; } let (min_idx, max_idx): (String, String) = match (access.counter, access.stat) { @@ -2699,7 +2813,10 @@ fn emit_packed_f64_range_guards( PackedNumericLoopKind::F64, ); } - Ok(all_guards_ok.expect("range loop matcher requires >= 1 array")) + Ok(( + all_guards_ok.expect("range loop matcher requires >= 1 array"), + affine_window_proven, + )) } /// Push the per-array facts for one fast-loop copy: counter accesses get a @@ -2715,6 +2832,7 @@ fn push_packed_f64_range_facts( values_i32: bool, allow_masked_stores: bool, numeric_accumulators: &[u32], + affine_window_proven: &std::collections::BTreeSet, ) { for access in &matched.arrays { if access.counter.is_some() { @@ -2746,7 +2864,10 @@ fn push_packed_f64_range_facts( store_side_exit_label: slow_pre_label.to_string(), array_kind: PackedNumericLoopKind::F64, allow_holes: false, - window_validated: false, + // True when the entry guard proved this array's whole affine + // window at the loop's endpoints — the reads then skip both + // the range clamp and the per-read bounds check. + window_validated: affine_window_proven.contains(&access.array_id), affine_indices: true, numeric_accumulators: numeric_accumulators.to_vec(), }); @@ -3120,7 +3241,7 @@ fn lower_packed_f64_range_versioned_for( // proof mid-loop, and the f64 tier's raw loads/stores need no such // claim. if has_stores { - let ok_f64 = emit_packed_f64_range_guards( + let (ok_f64, _) = emit_packed_f64_range_guards( ctx, &matched, &bound_i32, @@ -3135,7 +3256,7 @@ fn lower_packed_f64_range_versioned_for( let fast_i32_pre_idx = ctx.new_block("packed_f64_range.loop.fast_i32.preheader"); let fast_i32_pre_label = ctx.block_label(fast_i32_pre_idx); - let ok_i32 = emit_packed_f64_range_guards( + let (ok_i32, _) = emit_packed_f64_range_guards( ctx, &matched, &bound_i32, @@ -3146,7 +3267,7 @@ fn lower_packed_f64_range_versioned_for( .cond_br(&ok_i32, &fast_i32_pre_label, &try_f64_label); ctx.current_block = try_f64_idx; - let ok_f64 = emit_packed_f64_range_guards( + let (ok_f64, _) = emit_packed_f64_range_guards( ctx, &matched, &bound_i32, @@ -3181,6 +3302,7 @@ fn lower_packed_f64_range_versioned_for( true, false, &acc_scope.accumulators, + &Default::default(), ); let saved_stride = ctx.poll_stride_counter_slot.take(); ctx.poll_stride_counter_slot = ctx.i32_counter_slots.get(&matched.counter_id).cloned(); @@ -3229,6 +3351,7 @@ fn lower_packed_f64_range_versioned_for( false, has_stores, &acc_scope.accumulators, + &Default::default(), ); let saved_stride = ctx.poll_stride_counter_slot.take(); ctx.poll_stride_counter_slot = ctx.i32_counter_slots.get(&matched.counter_id).cloned(); @@ -3251,7 +3374,7 @@ fn lower_packed_f64_range_versioned_for( ctx.block().br(&merge_label); } } else { - let all_guards_ok = emit_packed_f64_range_guards( + let (all_guards_ok, affine_window_proven) = emit_packed_f64_range_guards( ctx, &matched, &bound_i32, @@ -3287,6 +3410,7 @@ fn lower_packed_f64_range_versioned_for( false, false, &acc_scope.accumulators, + &affine_window_proven, ); let saved_stride = ctx.poll_stride_counter_slot.take(); ctx.poll_stride_counter_slot = ctx.i32_counter_slots.get(&matched.counter_id).cloned(); diff --git a/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs b/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs index 08d4ce3591..e11f87660c 100644 --- a/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs +++ b/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs @@ -36,10 +36,11 @@ pub(super) fn packed_loop_numeric_accumulators_enabled() -> bool { fn accumulator_rhs_is_numeric( ctx: &FnCtx<'_>, expr: &Expr, - array_id: u32, + array_ids: &std::collections::BTreeSet, counter_id: u32, offset_reads_inlined: bool, masked_reads_validated: bool, + affine_reads: bool, candidates: &std::collections::BTreeSet, ) -> bool { match expr { @@ -48,7 +49,7 @@ fn accumulator_rhs_is_numeric( let Expr::LocalGet(a) = object.as_ref() else { return false; }; - if *a != array_id { + if !array_ids.contains(a) { return false; } match index.as_ref() { @@ -82,6 +83,13 @@ fn accumulator_rhs_is_numeric( // in-window read is a genuine Number. || (masked_reads_validated && crate::collectors::static_index_window(index).is_some()) + // Classic affine mode (#9253 family): the read is + // either a window-proven raw load or a checked load + // that side-exits before producing a value — numeric + // either way. Admission must mirror the matcher's, + // via the shared predicate. + || (affine_reads + && super::loops::affine_leaf_admissible(ctx, index, counter_id)) } } } @@ -92,28 +100,31 @@ fn accumulator_rhs_is_numeric( accumulator_rhs_is_numeric( ctx, left, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ) && accumulator_rhs_is_numeric( ctx, right, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ) } Expr::NumberCoerce(operand) => accumulator_rhs_is_numeric( ctx, operand, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ), Expr::Unary { op, operand } => { @@ -123,10 +134,11 @@ fn accumulator_rhs_is_numeric( ) && accumulator_rhs_is_numeric( ctx, operand, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ) } @@ -140,28 +152,31 @@ fn accumulator_rhs_is_numeric( | Expr::MathFround(v) => accumulator_rhs_is_numeric( ctx, v, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ), Expr::MathImul(l, r) | Expr::MathPow(l, r) => { accumulator_rhs_is_numeric( ctx, l, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ) && accumulator_rhs_is_numeric( ctx, r, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ) } @@ -169,10 +184,11 @@ fn accumulator_rhs_is_numeric( accumulator_rhs_is_numeric( ctx, v, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, candidates, ) }), @@ -323,10 +339,16 @@ pub(super) fn collect_local_writes<'a>( pub(super) fn collect_numeric_accumulators( ctx: &FnCtx<'_>, body: &[Stmt], - array_id: u32, + array_ids: &std::collections::BTreeSet, counter_id: u32, offset_reads_inlined: bool, masked_reads_validated: bool, + // Reads with an affine index over the counter qualify as numeric leaves: + // inside the clone they are either window-proven raw loads or checked + // loads whose failure side-exits before producing a value. The predicate + // must match the matcher's admission exactly (the #9259 drift rule) — + // `affine_leaf_admissible` is shared for that reason. + affine_reads: bool, ) -> Vec { if !packed_loop_numeric_accumulators_enabled() { return Vec::new(); @@ -337,7 +359,7 @@ pub(super) fn collect_numeric_accumulators( .keys() .copied() .filter(|id| { - *id != array_id + !array_ids.contains(id) && *id != counter_id && ctx.locals.contains_key(id) && !ctx.boxed_vars.contains(id) @@ -356,10 +378,11 @@ pub(super) fn collect_numeric_accumulators( Some(rhs) => accumulator_rhs_is_numeric( ctx, rhs, - array_id, + array_ids, counter_id, offset_reads_inlined, masked_reads_validated, + affine_reads, &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 3d048b2763..eb4ba96279 100644 --- a/crates/perry-codegen/src/stmt/stable_packed_loop.rs +++ b/crates/perry-codegen/src/stmt/stable_packed_loop.rs @@ -1662,10 +1662,11 @@ pub(super) fn lower( collect_numeric_accumulators( ctx, body, - candidate.array_id, + &std::collections::BTreeSet::from([candidate.array_id]), candidate.counter_id, false, false, + false, ) } else { Vec::new() diff --git a/crates/perry/tests/issue_9253_affine_range_index.rs b/crates/perry/tests/issue_9253_affine_range_index.rs index 05f179f670..8c80686436 100644 --- a/crates/perry/tests/issue_9253_affine_range_index.rs +++ b/crates/perry/tests/issue_9253_affine_range_index.rs @@ -108,9 +108,16 @@ 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); + // The admission signal: the 2-arg receiver-only guard is emitted only by + // the affine arm (the versioned tier also uses this symbol, but it cannot + // fire here — the bound is a parameter, not `arr.length`). The old + // detector looked for the per-read `packed_f64_affine.index_fits` block, + // which the window-hoist legitimately removed: a window proven at the + // loop's endpoints leaves the read as a bare trunc + raw load with no + // named block at all. assert!( - text.contains("packed_f64_affine"), - "#9253: `a[i * size + k]` must reach the affine read lowering; without it \ + text.contains("js_typed_feedback_packed_f64_array_loop_guard"), + "#9253: `a[i * size + k]` must reach the affine tier; without it \ the receiver guard re-executes per access" ); for moving_gc in [false, true] { @@ -171,3 +178,32 @@ console.log("negative:" + negative(a, 20)); assert_stdout(&run(&bin, dir.path(), moving_gc), "negative:3\n", moving_gc); } } + +/// The #9294 guard arm took its receiver-only `continue` for arrays with +/// BOTH counter-offset and affine accesses, skipping the windowed guard while +/// the counter fact still said `window_validated: true` — `a[k + 1]` then +/// read one raw slot past the loop's window at the boundary. Mixed arrays +/// now fall through to the windowed guard. Node's answer: the last +/// iteration's `a[k + 1]` is `a[size]`, out of bounds, `undefined`, NaN. +#[test] +fn a_mixed_offset_and_affine_array_validates_its_counter_window() { + let source = r#" +function run(a: number[], size: number): number { + let s = 0.0; + for (let i = 0; i < 1; i++) { + for (let k = 0; k < size; k++) { + s = s * 1.0 + a[k + 1] + a[i * size + k]; + } + } + return s; +} +const a: number[] = []; +for (let i = 0; i < 64; i++) a.push(1.0); +console.log("s:" + run(a, 64)); +"#; + 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), "s:NaN\n", moving_gc); + } +}