diff --git a/changelog.d/9161-packed-clone-foreign-counter-reads.md b/changelog.d/9161-packed-clone-foreign-counter-reads.md new file mode 100644 index 0000000000..9dae8fd4bc --- /dev/null +++ b/changelog.d/9161-packed-clone-foreign-counter-reads.md @@ -0,0 +1,32 @@ +A packed loop's fast clone can now read its array at an **enclosing** loop's +counter, not only at its own. + +The clone's element read is a raw slot load with no bounds check, licensed by the +index being the loop's own induction variable — which the loop bound already +proved in range. Any other index had no such proof, so `arr[i]` beside `arr[j]` +in a nested loop was rejected outright by the body matcher, and the whole inner +loop fell to the typed-feedback tier: a registered guard **call** plus a boxed +fallback per element, for both reads, on every iteration. + +A foreign index now takes the same raw load behind one inline `icmp ult idx, len` +against the live `ArrayHeader.length` — the same word `expr/index.rs`'s store +guard reads — and branches to the fact's existing side exit when it fails. That +exit is the one the hole arm already uses, so mid-body exit needed no new +machinery. + +**Read-only bodies only.** A side exit re-executes the iteration in the slow +clone, which is harmless for a read and would double-apply a store, so the +relaxation lives in `expr_is_packed_f64_loop_safe` and is deliberately not shared +with the store matchers beside it. + +Measured on an idle Mac mini, self-timed, min of three: `benchmarks/suite/10_nested_loops.ts` +51 → 17 ms against Node's 16 — 3.3× Node to parity. `16_matrix_multiply` also +improves (121 → 100 ms) since its inner reads are the same shape. No other +benchmark in the 17-entry sweep moved. + +Verified against Node on a five-case differential covering the shapes where a +side exit is observable: the plain nested accumulation, a labelled `break` out of +the inner loop mid-iteration, an `if` guarding the accumulation, a mixed-layout +array whose guard must fail into the slow clone, and an accumulator read by the +outer loop between inner passes. All byte-identical. `perry-codegen`: 30 suites +pass, including a new IR pin. diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index 1b618bdc3d..5e58a3c12e 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -40,7 +40,10 @@ use super::{ TypedFeedbackContract, TypedFeedbackKind, }; +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_fact_for_index}; mod inline_dyn_typed_array; use guarded_array::{ @@ -198,55 +201,6 @@ pub(crate) fn numeric_index_has_integer_array_index_proof(ctx: &FnCtx<'_>, index use super::proven_view_access::bitand_has_nonnegative_i32_mask; -/// #6011: decompose a packed-loop index expression into `(counter_local_id, -/// constant_offset)`. Matches `i`, `i + c`, `c + i`, and `i - c` with a small -/// |c| — exactly the shapes the packed-f64 range loop matcher admits, so any -/// offset seen here on a fact-carrying (array, counter) pair is inside the -/// range guard's validated window. -pub(crate) fn packed_f64_loop_index_parts(index: &Expr) -> Option<(u32, i32)> { - use perry_hir::BinaryOp; - match index { - Expr::LocalGet(id) => Some((*id, 0)), - Expr::Binary { op, left, right } if matches!(op, BinaryOp::Add | BinaryOp::Sub) => { - let (id, offset) = match (left.as_ref(), right.as_ref()) { - (Expr::LocalGet(id), Expr::Integer(c)) => { - let offset = if matches!(op, BinaryOp::Sub) { - c.checked_neg()? - } else { - *c - }; - (*id, offset) - } - (Expr::Integer(c), Expr::LocalGet(id)) if matches!(op, BinaryOp::Add) => (*id, *c), - _ => return None, - }; - let offset = i32::try_from(offset).ok()?; - if offset.unsigned_abs() > 64 { - return None; - } - Some((id, offset)) - } - _ => None, - } -} - -/// Look up a packed-f64 loop fact for `(arr, index-expr)`. Zero-offset -/// indices match any fact; non-zero offsets only match hole-tolerant facts -/// (established by the range guard, which validated the whole offset window — -/// the length-bound guard of the classic matcher only proves `i` itself). -fn packed_f64_loop_fact_for_index( - ctx: &FnCtx<'_>, - arr_id: u32, - index: &Expr, -) -> Option<(PackedF64LoopFact, u32, i32)> { - let (idx_id, offset) = packed_f64_loop_index_parts(index)?; - let fact = packed_f64_loop_fact(ctx, arr_id, idx_id)?; - if offset != 0 && !fact.allow_holes && !fact.window_validated { - return None; - } - Some((fact, idx_id, offset)) -} - /// Load the packed-loop counter's i32 shadow slot and apply the constant /// index offset. fn load_packed_loop_index_i32(ctx: &mut FnCtx<'_>, i32_slot: &str, offset: i32) -> String { @@ -802,7 +756,21 @@ pub(crate) fn lower_numeric_index_get_for_number_context( let arr_box = lower_expr(ctx, object)?; let idx_i32 = load_packed_loop_index_i32(ctx, &i32_slot, offset); return Ok(Some(lower_packed_f64_loop_index_get( - ctx, *arr_id, &arr_box, &idx_i32, &fact, + ctx, *arr_id, &arr_box, &idx_i32, &fact, false, + ))); + } + } + // The same clone, read at an index that is NOT its counter: an + // enclosing loop's i32 counter, admitted by + // `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. + 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)?; + let idx_i32 = ctx.block().load(I32, &i32_slot); + return Ok(Some(lower_packed_f64_loop_index_get( + ctx, *arr_id, &arr_box, &idx_i32, &fact, true, ))); } } @@ -1667,7 +1635,18 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let arr_box = lower_expr(ctx, object)?; let idx_i32 = load_packed_loop_index_i32(ctx, &i32_slot, offset); return Ok(lower_packed_f64_loop_index_get( - ctx, *arr_id, &arr_box, &idx_i32, &fact, + ctx, *arr_id, &arr_box, &idx_i32, &fact, false, + )); + } + } + 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)?; + let idx_i32 = ctx.block().load(I32, &i32_slot); + return Ok(lower_packed_f64_loop_index_get( + ctx, *arr_id, &arr_box, &idx_i32, &fact, true, )); } } diff --git a/crates/perry-codegen/src/expr/index_get/foreign_counter.rs b/crates/perry-codegen/src/expr/index_get/foreign_counter.rs new file mode 100644 index 0000000000..84a6a46937 --- /dev/null +++ b/crates/perry-codegen/src/expr/index_get/foreign_counter.rs @@ -0,0 +1,87 @@ +//! The foreign-counter packed-clone read (#9161). +//! +//! Split out of `index_get.rs`, which sits at the 2000-line cap. This is the +//! `arr[i]` case where `i` is a live i32 counter of some OTHER loop than the +//! clone's own — see `foreign_packed_loop_read` for why a fact carrying its +//! own per-element exit is declined. + +use super::super::*; +use super::packed_f64_loop_fact; + +/// An active packed-loop fact for `arr_id` plus a foreign i32 index local: +/// `arr[i]` where `i` is not the clone's counter. Declines a fact that already +/// carries its own per-element exit condition (holes, a validated window), so +/// the bounds-checked load never stacks two side exits on one read. +pub(crate) fn foreign_packed_loop_read( + ctx: &FnCtx<'_>, + arr_id: u32, + index: &Expr, +) -> Option<(PackedF64LoopFact, u32)> { + let Expr::LocalGet(idx_id) = index else { + return None; + }; + if !ctx.i32_counter_slots.contains_key(idx_id) || !ctx.integer_locals.contains(idx_id) { + return None; + } + let fact = ctx + .packed_f64_loop_facts + .iter() + .rev() + .find(|fact| { + fact.array_local_id == arr_id + && fact.index_local_id != *idx_id + && !fact.allow_holes + && !fact.window_validated + })? + .clone(); + Some((fact, *idx_id)) +} + +/// #6011: decompose a packed-loop index expression into `(counter_local_id, +/// constant_offset)`. Matches `i`, `i + c`, `c + i`, and `i - c` with a small +/// |c| — exactly the shapes the packed-f64 range loop matcher admits, so any +/// offset seen here on a fact-carrying (array, counter) pair is inside the +/// range guard's validated window. +pub(crate) fn packed_f64_loop_index_parts(index: &Expr) -> Option<(u32, i32)> { + use perry_hir::BinaryOp; + match index { + Expr::LocalGet(id) => Some((*id, 0)), + Expr::Binary { op, left, right } if matches!(op, BinaryOp::Add | BinaryOp::Sub) => { + let (id, offset) = match (left.as_ref(), right.as_ref()) { + (Expr::LocalGet(id), Expr::Integer(c)) => { + let offset = if matches!(op, BinaryOp::Sub) { + c.checked_neg()? + } else { + *c + }; + (*id, offset) + } + (Expr::Integer(c), Expr::LocalGet(id)) if matches!(op, BinaryOp::Add) => (*id, *c), + _ => return None, + }; + let offset = i32::try_from(offset).ok()?; + if offset.unsigned_abs() > 64 { + return None; + } + Some((id, offset)) + } + _ => None, + } +} + +/// Look up a packed-f64 loop fact for `(arr, index-expr)`. Zero-offset +/// indices match any fact; non-zero offsets only match hole-tolerant facts +/// (established by the range guard, which validated the whole offset window — +/// the length-bound guard of the classic matcher only proves `i` itself). +pub(crate) fn packed_f64_loop_fact_for_index( + ctx: &FnCtx<'_>, + arr_id: u32, + index: &Expr, +) -> Option<(PackedF64LoopFact, u32, i32)> { + let (idx_id, offset) = packed_f64_loop_index_parts(index)?; + let fact = packed_f64_loop_fact(ctx, arr_id, idx_id)?; + if offset != 0 && !fact.allow_holes && !fact.window_validated { + return None; + } + Some((fact, idx_id, offset)) +} diff --git a/crates/perry-codegen/src/expr/index_get/guarded_array.rs b/crates/perry-codegen/src/expr/index_get/guarded_array.rs index c54878c435..a56d5c0c75 100644 --- a/crates/perry-codegen/src/expr/index_get/guarded_array.rs +++ b/crates/perry-codegen/src/expr/index_get/guarded_array.rs @@ -529,9 +529,30 @@ pub(super) fn lower_packed_f64_loop_index_get( arr_box: &str, idx_i32: &str, fact: &PackedF64LoopFact, + bounds_check: bool, ) -> String { let guard_id = fact.guard_id.as_str(); let array_kind = fact.array_kind; + // A foreign index carries no in-range proof from the loop bound, so test it + // against the live length (`ArrayHeader.length`, i32 at offset 0 — the same + // word `expr/index.rs`'s store guard reads) and take the fact's side exit + // when it fails. One compare and a never-taken branch, against the + // typed-feedback guard CALL plus boxed fallback this replaces. + if bounds_check { + let in_bounds = { + let blk = ctx.block(); + let arr_bits = blk.bitcast_double_to_i64(arr_box); + let arr_handle = blk.and(I64, &arr_bits, POINTER_MASK_I64); + let arr_ptr = blk.inttoptr(I64, &arr_handle); + let length = blk.load(I32, &arr_ptr); + blk.icmp_ult(I32, idx_i32, &length) + }; + let cont_idx = ctx.new_block("packed_f64_loop.foreign.inbounds"); + let cont_label = ctx.block_label(cont_idx); + ctx.block() + .cond_br(&in_bounds, &cont_label, &fact.store_side_exit_label); + ctx.current_block = cont_idx; + } let value = { let blk = ctx.block(); let arr_bits = blk.bitcast_double_to_i64(arr_box); diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 2df875cc4f..6829a9febc 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -5188,7 +5188,7 @@ fn expr_is_packed_f64_loop_safe( use perry_hir::{ArrayElement, Expr}; match expr { Expr::IndexGet { object, index } => { - is_packed_f64_loop_index(object, index, arr_id, counter_id) + is_packed_f64_loop_foreign_read_index(ctx, object, index, arr_id, counter_id) } // A numeric-store fallback can downgrade/invalidate raw-f64 layout. // Without a loop restart, later packed-loop loads would keep using the @@ -5274,6 +5274,45 @@ fn expr_is_packed_f64_loop_safe( } } +/// `arr[i]` inside a READ-ONLY matched body where `i` is an i32 counter of an +/// ENCLOSING loop rather than this loop's own. +/// +/// The clone's raw slot load is licensed by the counter being the loop's own +/// induction variable, which its bound proves in range. A foreign index has no +/// such proof, so the read site pays one inline `icmp ult idx, len` and takes +/// the fact's existing side exit when it fails — the same mid-body exit the +/// hole arm already uses. +/// +/// Read-only bodies only, and that is what calling this from +/// `expr_is_packed_f64_loop_safe` (never from the store matchers) buys: a side +/// exit re-executes the iteration in the slow clone, which is harmless for +/// reads and would double-apply a store. `sum = sum + arr[i] + arr[j]` in +/// `benchmarks/suite/10_nested_loops.ts` is exactly this shape, and paid two +/// typed-feedback guard calls plus two boxed fallbacks per iteration for it. +fn is_packed_f64_loop_foreign_read_index( + ctx: &FnCtx<'_>, + object: &perry_hir::Expr, + index: &perry_hir::Expr, + arr_id: u32, + counter_id: u32, +) -> bool { + if is_packed_f64_loop_index(object, index, arr_id, counter_id) { + return true; + } + let (perry_hir::Expr::LocalGet(object_id), perry_hir::Expr::LocalGet(index_id)) = + (object, index) + else { + return false; + }; + *object_id == arr_id + && *index_id != counter_id + && *index_id != arr_id + && ctx.integer_locals.contains(index_id) + && ctx.i32_counter_slots.contains_key(index_id) + && !ctx.boxed_vars.contains(index_id) + && !ctx.closure_captures.contains_key(index_id) +} + fn is_packed_f64_loop_index( object: &perry_hir::Expr, index: &perry_hir::Expr, diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index 9d0d6a24af..a0d10e886d 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -15605,3 +15605,49 @@ mod integer_modulo; #[path = "native_proof_regressions/math_mul_fastpath.rs"] mod math_mul_fastpath; + +// `sum = sum + arr[i] + arr[j]` in a nested counted loop (suite +// `10_nested_loops`): the inner clone reads `arr` at its own counter AND at the +// outer loop's. The foreign read used to fall to the typed-feedback tier — a +// registered guard CALL plus a boxed fallback per element — while the sibling +// read beside it was a raw slot load. It now takes the same raw load behind one +// inline bounds check, exiting to the clone's existing side exit when it fails. +#[test] +fn packed_clone_reads_a_foreign_counter_without_the_feedback_call() { + let add = |left: Expr, right: Expr| Expr::Binary { + op: BinaryOp::Add, + left: Box::new(left), + right: Box::new(right), + }; + let body = vec![ + number_array_let(1, "arr", vec![1, 2, 3, 4, 5, 6, 7, 8]), + Stmt::Let { + id: 2, + name: "sum".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Number(0.0)), + }, + for_loop( + 3, + length(1), + vec![for_loop( + 4, + length(1), + vec![Stmt::Expr(Expr::LocalSet( + 2, + Box::new(add( + add(local(2), index_get(1, local(3))), + index_get(1, local(4)), + )), + ))], + )], + ), + Stmt::Return(Some(local(2))), + ]; + let ir = compile_ir("packed_clone_foreign_read.ts", body); + assert!( + ir.contains("packed_f64_loop.foreign.inbounds"), + "the foreign-counter read should take the bounds-checked clone load:\n{ir}" + ); +}