Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions crates/perry-codegen/src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,10 @@ pub(super) fn compile_function(
&spec_i32_params,
&spec_numeric_params,
&spec_number_array_params,
// #9363: module-scope bindings whose CONSTRUCTION (not annotation)
// proves a numeric typed-array/Uint8Array kind, so `g[i]` off one is
// Number-or-`undefined` exactly as a body-local `const` view is.
&cross_module.module_global_proven_types,
);

// A Number-by-construction local cannot ever hold a GC pointer, so it
Expand Down
6 changes: 6 additions & 0 deletions crates/perry-codegen/src/collectors/hir_facts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ pub(crate) fn collect_type_facts(
spec_i32_params: &HashSet<u32>,
spec_numeric_params: &HashSet<u32>,
spec_number_array_params: &HashSet<u32>,
module_global_proven_types: &HashMap<u32, perry_hir::types::Type>,
) -> TypeFacts {
// #7700: which locals hold a NUMBER, so a `u8[k]` keyed on one is a byte
// read rather than a property read. Computed once here because
Expand Down Expand Up @@ -560,6 +561,7 @@ pub(crate) fn collect_type_facts(
spec_ta_lens,
spec_numeric_params,
&not_bigint_locals,
module_global_proven_types,
);
let (mut array_facts, effect_facts, materialization_hazards) =
collect_array_facts(stmts, params, module_globals, binding_types);
Expand Down Expand Up @@ -792,6 +794,7 @@ pub(crate) fn collect_native_region_fact_graph(
&HashSet::new(),
&HashSet::new(),
&HashSet::new(),
&HashMap::new(),
)
}

Expand All @@ -815,6 +818,7 @@ pub(crate) fn collect_native_region_fact_graph_with_spec_params(
spec_i32_params: &HashSet<u32>,
spec_numeric_params: &HashSet<u32>,
spec_number_array_params: &HashSet<u32>,
module_global_proven_types: &HashMap<u32, perry_hir::types::Type>,
) -> NativeRegionFactGraph {
collect_type_facts(
stmts,
Expand All @@ -832,6 +836,7 @@ pub(crate) fn collect_native_region_fact_graph_with_spec_params(
spec_i32_params,
spec_numeric_params,
spec_number_array_params,
module_global_proven_types,
)
}

Expand Down Expand Up @@ -861,6 +866,7 @@ pub(crate) fn collect_hir_facts(
&HashSet::new(),
&HashSet::new(),
&HashSet::new(),
&HashMap::new(),
)
}

Expand Down
131 changes: 130 additions & 1 deletion crates/perry-codegen/src/collectors/int_valued_ta_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,14 @@ pub fn collect_int_valued_ta_locals(
// (an `undefined`-able operand breaks `image == ToInt32(true)` through a
// float add — `undefined + 1` is NaN→0, the image path would say 1).
let additive_invalid = additive_flow_invalid_targets(stmts, &types, &ta_lens, &numeric_locals);
// #9363: locals a loop body re-seeds unconditionally every iteration, which
// bounds an in-loop additive chain to one body's worth. See
// `collect_loop_reseeded_locals` for why no dominance argument is needed.
let loop_reseeded = {
let mut out = HashSet::new();
collect_loop_reseeded_locals(stmts, &types, guarded_number_array_params, false, &mut out);
out
};

// Rule (1) admission. A candidate is a `let`-declared local with ≥1
// int-TA-read write, whose EVERY write is i32-producing-safe (or, in the
Expand All @@ -729,7 +737,7 @@ pub fn collect_int_valued_ta_locals(
pool.retain(|id| {
facts.writes[id].iter().all(|(w, in_loop)| {
write_is_i32_producing_safe(w, &types, guarded_number_array_params, &numeric_locals)
|| (!in_loop
|| ((!in_loop || loop_reseeded.contains(id))
&& !additive_invalid.contains(id)
&& additive_write_admissible(
w,
Expand Down Expand Up @@ -978,6 +986,127 @@ fn collect_facts<'a>(
}
}

/// Locals that a loop body RE-SEEDS on every iteration with a non-additive,
/// i32-producing write (#9363/#6898 follow-up).
///
/// The wrap-i32 additive arm is otherwise restricted to straight-line trees
/// because an in-loop chain can carry the true f64 value past 2^53, where it
/// rounds while the i32 slot wraps — and rule (2) only guarantees the ToInt32
/// image is observed, so the two would then disagree.
///
/// A re-seed removes exactly that hazard, and does so WITHOUT needing a
/// dominance argument: if a loop body unconditionally assigns the local a
/// fresh exact-i32 value on every iteration, then whatever additive writes the
/// same body performs, the local's magnitude never exceeds one body's worth of
/// them — the order of the re-seed within the body does not matter, because
/// the chain restarts once per iteration either way. With every addend below
/// 2^31, the true value stays under `(writes_per_body + 1) * 2^31`, and a body
/// would need ~4M additive writes to reach 2^53.
///
/// This is why the scan is deliberately narrow: the re-seed must sit at the
/// loop body's TOP level. A re-seed nested in an `if`/`switch`/`try` may not
/// run on a given iteration, which is precisely the case where the chain can
/// keep growing. Nested loops are scanned as their own bodies, so an inner
/// loop is judged by its own re-seeds, never by the outer body's.
///
/// The motivating shape is bcryptjs `_encipher`, this module's own subject:
/// `n = S[l >>> 24]` re-seeds every round, followed by at most two `+=` before
/// a bitwise write, so `|n| < 2^33`.
fn collect_loop_reseeded_locals<'a>(
stmts: &'a [Stmt],
types: &HashMap<u32, HirType>,
guarded_number_array_params: &HashSet<u32>,
inside_loop: bool,
out: &mut HashSet<u32>,
) {
for stmt in stmts {
// Only a TOP-LEVEL `x = <rhs>` in a loop body counts as a re-seed.
if inside_loop {
if let Stmt::Expr(Expr::LocalSet(id, rhs)) = stmt {
if write_is_i32_producing_safe(
rhs,
types,
guarded_number_array_params,
&HashSet::new(),
) {
out.insert(*id);
}
}
}
match stmt {
Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => {
collect_loop_reseeded_locals(body, types, guarded_number_array_params, true, out);
}
Stmt::For { body, init, .. } => {
if let Some(init) = init.as_deref() {
collect_loop_reseeded_locals(
std::slice::from_ref(init),
types,
guarded_number_array_params,
inside_loop,
out,
);
}
collect_loop_reseeded_locals(body, types, guarded_number_array_params, true, out);
}
// Conditional and unwinding scaffolding: walk for NESTED loops, but
// nothing directly inside them is an unconditional re-seed of the
// enclosing body, so the flag is cleared.
Stmt::If {
then_branch,
else_branch,
..
} => {
collect_loop_reseeded_locals(
then_branch,
types,
guarded_number_array_params,
false,
out,
);
if let Some(eb) = else_branch {
collect_loop_reseeded_locals(
eb,
types,
guarded_number_array_params,
false,
out,
);
}
}
Stmt::Try {
body,
catch,
finally,
} => {
collect_loop_reseeded_locals(body, types, guarded_number_array_params, false, out);
if let Some(c) = catch {
collect_loop_reseeded_locals(
&c.body,
types,
guarded_number_array_params,
false,
out,
);
}
if let Some(f) = finally {
collect_loop_reseeded_locals(f, types, guarded_number_array_params, false, out);
}
}
Stmt::Labeled { body, .. } => {
collect_loop_reseeded_locals(
std::slice::from_ref(body),
types,
guarded_number_array_params,
inside_loop,
out,
);
}
_ => {}
}
}
}

fn record_write<'a>(
id: u32,
rhs: &'a Expr,
Expand Down
48 changes: 47 additions & 1 deletion crates/perry-codegen/src/collectors/number_by_construction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,29 @@ pub(crate) fn enabled() -> bool {
)
}

/// Typed-array/`Uint8Array` class names whose elements are Numbers.
///
/// The BigInt kinds are deliberately absent: `BigInt64Array`/`BigUint64Array`
/// elements are BigInts, not Numbers, and `+` on one throws when mixed. Kept
/// as a name list rather than reusing a kind table because
/// `module_global_proven_types` records the CLASS NAME the initializer
/// constructed.
fn class_name_is_number_valued_view(name: &str) -> bool {
matches!(
name,
"Uint8Array"
| "Uint8ClampedArray"
| "Int8Array"
| "Int16Array"
| "Uint16Array"
| "Int32Array"
| "Uint32Array"
| "Float32Array"
| "Float64Array"
| "Buffer"
)
}

/// Locals whose every write is number-producing by construction.
///
/// Deliberately independent of `PERRY_PTR_SHAPE_LOCALS`: the fact is about
Expand All @@ -88,6 +111,7 @@ pub(crate) fn collect_number_by_construction_locals(
spec_ta_lens: &HashMap<u32, i64>,
spec_numeric_params: &HashSet<u32>,
not_bigint_locals: &HashSet<u32>,
module_global_proven_types: &HashMap<u32, HirType>,
) -> HashSet<u32> {
if !enabled() {
return HashSet::new();
Expand All @@ -101,7 +125,29 @@ pub(crate) fn collect_number_by_construction_locals(
// pointer/string, so the fixpoint may treat it like a compiler-visible
// local typed-view constructor on one side of `+` (where `undefined`
// becomes the Number NaN rather than selecting string concatenation).
let numeric_ta_views: HashSet<u32> = spec_ta_lens.keys().copied().collect();
let mut numeric_ta_views: HashSet<u32> = spec_ta_lens.keys().copied().collect();
// #9363: a MODULE-GLOBAL typed array carries the same construction proof a
// body-local `const view = new Uint8Array(n)` does, and for the same
// reason: `module_global_proven_types` is derived from the initializer
// expression (`Expr::Uint8ArrayNew` / `TypedArrayNew`) on a single-`Let`,
// never-reassigned binding — it is not a declared type, which this
// collector correctly refuses to treat as evidence (#7773).
//
// Without this the fixpoint saw `const buf = new Uint8Array(N)` at module
// scope and answered "unknown receiver", so `acc += buf[i]` inside a
// function lost the accumulator's Number-by-construction proof. The cost
// was not the read: the missing proof made the `+` non-inert, which made
// `loop_purity::loop_may_allocate` answer `true`, which kept a per-
// iteration `load volatile @PERRY_GC_POLL_ARMED` in the inner loop —
// blocking vectorization and pinning the accumulator in memory — and
// routed the add through the rooted `guarded_add` diamond. Measured 444 ms
// vs 94 ms for the identical loop over a body-local receiver.
numeric_ta_views.extend(
module_global_proven_types
.iter()
.filter(|(_, ty)| matches!(ty, HirType::Named(name) if class_name_is_number_valued_view(name)))
.map(|(id, _)| *id),
);
let mut numeric = super::ptr_shape::collect_numeric_by_construction_locals_for_type_analysis(
stmts,
boxed_vars,
Expand Down
17 changes: 17 additions & 0 deletions crates/perry-codegen/src/stmt/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7415,6 +7415,23 @@ pub(crate) fn emit_gc_loop_safepoint(
if !ctx.element_shape_loop_facts.is_empty()
|| !ctx.class_field_loop_facts.is_empty()
|| !ctx.stable_packed_loop_facts.is_empty()
// #9379: the packed-f64 loop clone is the next body the paragraph above
// predicted — "the next body shape admitted to the matcher that is not
// provably inert would delete that clone the same way", except this one
// IS provably inert and was simply not listed. Its entry guard proves a
// live packed raw-f64 plain Array with the window in bounds, its reads
// and writes lower to bare `double` load/store on existing slots (so no
// growth, no realloc, no barrier), and its matcher admits no calls,
// closures or awaits into the body. `loop_may_allocate` cannot see any
// of that: it answers from the HIR, where `arr[i] = e` is a generic
// `IndexSet` that CAN reallocate, which is why it demanded a poll here.
//
// The poll was not merely costing its own instructions. Its volatile
// load is a clobber inside the loop, so the cached receiver base had to
// be re-derived per element — which is why striding it 1-in-64 (#9316)
// did not recover the loss and removing it does. Measured on
// `bench_numeric_array_numeric`: 45 -> 38 ms against node's 38.
|| !ctx.packed_f64_loop_facts.is_empty()
|| ctx.versioned_indexed_loop_facts.last().is_some_and(|fact| {
matches!(
fact.guard_mode,
Expand Down
Loading
Loading