Skip to content

fix: keep main green after the #9302/#9303/#9307 batch #9313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
proggeramlug merged 5 commits into from
Aug 31, 2026
Merged

fix: keep main green after the #9302/#9303/#9307 batch #9313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2eb6255
codegen: a float accumulator over masked reads earns the dense range …
Aug 31, 2026
698d352
refactor(runtime): split ic_miss.rs and array/tests.rs under the file…
Aug 31, 2026
57ec7b4
fix(codegen): drop the now-dead catch-all after the combined accumula…
Aug 31, 2026
774c5d1
style: rustfmt
Aug 31, 2026
bbab1e4
chore: changelog fragment for the train13 follow-up
Aug 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog.d/9313-train13-followup.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
### Internal

- **Keeps `ic_miss.rs` and `array/tests.rs` under the 2000-line file gate.**
#9302 and #9307 each landed on a file already within ~35 lines of the cap.
The C3C PIC test module and the `Array.prototype` method-discriminator tests
move to sibling files; no behaviour change.

- **Drops the dead catch-all left behind the combined accumulator arm.** #9303
added a combined `_ =>` arm without removing the `_ => false` it supersedes,
which is an unreachable pattern and so a `-D warnings` failure. This is the
deletion #9308 identified.
32 changes: 32 additions & 0 deletions changelog.d/dense-accumulator-masked-reads.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions crates/perry-codegen/src/expr/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2064,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<u32>,
/// 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
Expand Down
118 changes: 86 additions & 32 deletions crates/perry-codegen/src/stmt/loops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,17 +572,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,
Expand All @@ -594,6 +594,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,
)
}

Expand Down Expand Up @@ -907,13 +908,15 @@ 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,
body,
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.
Expand Down Expand Up @@ -1099,6 +1102,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 {
Expand Down Expand Up @@ -1373,15 +1379,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() {
Expand Down Expand Up @@ -1818,6 +1844,7 @@ fn packed_f64_range_loop_dense_body_collect(
counter_id: u32,
bound_local: Option<u32>,
accesses: &mut std::collections::BTreeMap<u32, PackedF64RangeArrayAccess>,
pending_accumulators: &mut std::collections::BTreeSet<u32>,
) -> bool {
let mut written: std::collections::HashSet<u32> = std::collections::HashSet::new();
packed_f64_range_loop_dense_stmts_collect(
Expand All @@ -1827,6 +1854,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.
Expand All @@ -1847,6 +1875,7 @@ fn packed_f64_range_loop_dense_stmts_collect(
bound_local: Option<u32>,
accesses: &mut std::collections::BTreeMap<u32, PackedF64RangeArrayAccess>,
written: &mut std::collections::HashSet<u32>,
pending_accumulators: &mut std::collections::BTreeSet<u32>,
) -> bool {
use perry_hir::Expr;
for stmt in body {
Expand All @@ -1872,10 +1901,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, None,
)
// 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;
}
Expand Down Expand Up @@ -1971,6 +2010,7 @@ fn packed_f64_range_loop_dense_stmts_collect(
bound_local,
accesses,
written,
pending_accumulators,
) {
return false;
}
Expand All @@ -1982,6 +2022,7 @@ fn packed_f64_range_loop_dense_stmts_collect(
bound_local,
accesses,
written,
pending_accumulators,
) {
return false;
}
Expand Down Expand Up @@ -2052,7 +2093,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.
Expand All @@ -2071,6 +2112,7 @@ struct MaskedWindowExpressionProof {
fn masked_window_expression_proof(
ctx: &FnCtx<'_>,
expr: &perry_hir::Expr,
accumulator: Option<u32>,
) -> Option<MaskedWindowExpressionProof> {
use perry_hir::{BinaryOp, CompareOp, Expr, UnaryOp};
let proof = |inert, numeric| MaskedWindowExpressionProof { inert, numeric };
Expand All @@ -2083,12 +2125,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,
Expand All @@ -2102,8 +2154,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;
Expand All @@ -2114,23 +2166,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,
Expand All @@ -2141,33 +2193,33 @@ 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;
}
}
Some(proof(true, true))
}
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;
}
}
Expand All @@ -2181,7 +2233,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;
}
Expand Down Expand Up @@ -2497,6 +2549,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(),
});
}
}
Expand Down Expand Up @@ -2599,6 +2652,7 @@ fn lower_masked_window_ta_tier(
values_i32,
elem,
allows_stores: false,
numeric_accumulators: Vec::new(),
});
}
lower_for_after_init_with_i32_bound(
Expand Down
3 changes: 3 additions & 0 deletions crates/perry-codegen/src/stmt/masked_window_region.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,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;
Expand Down Expand Up @@ -1003,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;
Expand Down Expand Up @@ -1037,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(
Expand Down
Loading
Loading