diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index fa13e85de0..6209dcd091 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -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 diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index a64a722739..97d800b423 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -464,6 +464,7 @@ pub(crate) fn collect_type_facts( spec_i32_params: &HashSet, spec_numeric_params: &HashSet, spec_number_array_params: &HashSet, + module_global_proven_types: &HashMap, ) -> 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 @@ -560,6 +561,7 @@ pub(crate) fn collect_type_facts( spec_ta_lens, spec_numeric_params, ¬_bigint_locals, + module_global_proven_types, ); let (mut array_facts, effect_facts, materialization_hazards) = collect_array_facts(stmts, params, module_globals, binding_types); @@ -792,6 +794,7 @@ pub(crate) fn collect_native_region_fact_graph( &HashSet::new(), &HashSet::new(), &HashSet::new(), + &HashMap::new(), ) } @@ -815,6 +818,7 @@ pub(crate) fn collect_native_region_fact_graph_with_spec_params( spec_i32_params: &HashSet, spec_numeric_params: &HashSet, spec_number_array_params: &HashSet, + module_global_proven_types: &HashMap, ) -> NativeRegionFactGraph { collect_type_facts( stmts, @@ -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, ) } @@ -861,6 +866,7 @@ pub(crate) fn collect_hir_facts( &HashSet::new(), &HashSet::new(), &HashSet::new(), + &HashMap::new(), ) } diff --git a/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs b/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs index b3419da4f0..d4b5e4dee8 100644 --- a/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs +++ b/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs @@ -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 @@ -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, @@ -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, + guarded_number_array_params: &HashSet, + inside_loop: bool, + out: &mut HashSet, +) { + for stmt in stmts { + // Only a TOP-LEVEL `x = ` 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, diff --git a/crates/perry-codegen/src/collectors/number_by_construction.rs b/crates/perry-codegen/src/collectors/number_by_construction.rs index 085602e8da..fd1a6de595 100644 --- a/crates/perry-codegen/src/collectors/number_by_construction.rs +++ b/crates/perry-codegen/src/collectors/number_by_construction.rs @@ -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 @@ -88,6 +111,7 @@ pub(crate) fn collect_number_by_construction_locals( spec_ta_lens: &HashMap, spec_numeric_params: &HashSet, not_bigint_locals: &HashSet, + module_global_proven_types: &HashMap, ) -> HashSet { if !enabled() { return HashSet::new(); @@ -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 = spec_ta_lens.keys().copied().collect(); + let mut numeric_ta_views: HashSet = 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, diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index ca20194301..1bb8d90e51 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -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, diff --git a/crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs b/crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs new file mode 100644 index 0000000000..1735f3b50b --- /dev/null +++ b/crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs @@ -0,0 +1,224 @@ +//! #9363/#6898: an in-loop additive write is i32-admissible when the loop body +//! RE-SEEDS the local unconditionally on every iteration. +//! +//! `collectors/int_valued_ta_locals.rs` exists for bcryptjs `_encipher` — its +//! module doc is that function — but rejected its own subject's accumulator. +//! The wrap-i32 additive arm was restricted to STRAIGHT-LINE (never in-loop) +//! trees, and `n += S[...]` sits in the Feistel `while`. So `n` stayed an f64 +//! slot holding nothing but int32 values, and every S-box step emitted +//! `sitofp` in and `llvm.aarch64.fjcvtzs` out around the `fadd`. +//! +//! ## Why the restriction was too coarse +//! +//! Its stated hazard is real: an unbounded in-loop chain can carry the true +//! f64 value past 2^53, where it ROUNDS while an i32 slot WRAPS, and rule (2) +//! only guarantees the `ToInt32` image is observed — so the two would then +//! disagree. +//! +//! A per-iteration re-seed removes exactly that hazard, and needs no dominance +//! argument: if the body unconditionally assigns the local a fresh exact-i32 +//! value once per iteration, the chain restarts every iteration no matter +//! WHERE the re-seed sits, so the magnitude never exceeds one body's worth of +//! addends. With each addend below 2^31, a body would need ~4M additive writes +//! to reach 2^53. `_encipher` re-seeds `n` every round and adds at most twice: +//! `|n| < 2^33`. +//! +//! The re-seed must be at the body's TOP level. One nested in an `if` may not +//! run on a given iteration — precisely the case where the chain keeps +//! growing — and an inner loop's re-seed does not bound the OUTER body's +//! chain. Both are pinned below, because they are the shapes that would ship +//! wrapped arithmetic if the rule were widened carelessly. +//! +//! Measured on `bench_typed_array_untyped_access`: typed 1216 -> 257 ms and +//! untyped 1254 -> 258 against node's 290 / 299 — from 4.2x slower to faster +//! than node on both paths. + +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 { + 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") + .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 +} + +fn run(bin: &Path, dir: &Path, gc_stress: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if gc_stress { + command + .env("PERRY_GC_HEAP_LIMIT", "8") + .env("PERRY_GC_FORCE_EVACUATE", "1"); + } + command.output().expect("run compiled binary") +} + +/// Every shape whose additive chain is NOT bounded by an unconditional +/// per-iteration re-seed must keep its f64 representation. Node is the oracle +/// rather than a hand-written expectation, because the whole question is +/// whether perry's value agrees with JavaScript's. +/// +/// The elements are `2^30`, so four of them exceed `i32::MAX`: a local wrongly +/// promoted to an i32 slot prints a WRAPPED negative here, not a slightly +/// different number. Cases A and D deliberately produce values above i32 range +/// (and A above 2^53's neighbourhood) so the failure would be unmissable. +#[test] +fn only_unconditionally_reseeded_accumulators_are_admitted() { + const SOURCE: &str = r#" +const S = new Int32Array(8); +for (let i = 0; i < 8; i++) S[i] = 0x40000000; + +// A: no reseed at all — accumulates across 1e6 iterations. +function noReseed(): number { + let n = S[0]; + for (let i = 0; i < 1000000; i++) { n += S[i & 7]; } + return n; +} +// B: reseed guarded by an `if` — does not run every iteration. +function condReseed(): number { + let n = S[0]; + for (let i = 0; i < 1000; i++) { + if (i === 500) { n = S[1]; } + n += S[i & 7]; + } + return n; +} +// C: unconditional top-level reseed — the admissible shape. +function goodReseed(): number { + let n = 0; let acc = 0; + for (let i = 0; i < 1000; i++) { n = S[i & 7]; n += S[(i + 1) & 7]; acc ^= n; } + return acc; +} +// D: reseed only in an INNER loop — the outer chain is still unbounded. +function innerOnly(): number { + let n = S[0]; + for (let i = 0; i < 1000; i++) { + for (let j = 0; j < 2; j++) { n = S[j]; } + n += S[i & 7]; + } + return n; +} +// E: out-of-range reads mixed in — `undefined` must behave as JS says. +function withOob(): string { + let n = S[0]; + let out = ""; + for (let i = 0; i < 4; i++) { n = S[i + 6]; n += S[i]; out += String(n) + ";"; } + return out; +} +console.log("A:" + noReseed()); +console.log("B:" + condReseed()); +console.log("C:" + goodReseed()); +console.log("D:" + innerOnly()); +console.log("E:" + withOob()); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let bin = compile(dir.path(), SOURCE); + + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!( + node.status.success(), + "node failed on the oracle fixture:\n{}", + String::from_utf8_lossy(&node.stderr) + ); + // Guard the oracle itself: if these stopped exceeding i32 range the test + // would still pass while having lost its ability to detect a wrap. + let expected = String::from_utf8_lossy(&node.stdout).into_owned(); + assert!( + expected.contains("A:1073742897741824") && expected.contains("D:2147483648"), + "the oracle no longer produces values outside i32 range, so a wrongly \ + admitted local would no longer be detectable here:\n{expected}" + ); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!(out.status.success(), "binary failed (gc_stress={stress})"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + expected, + "an accumulator was promoted to an i32 slot without a bounded \ + chain (gc_stress={stress}) — the value wrapped" + ); + } +} + +/// The motivating shape itself: a Feistel round that re-seeds its accumulator +/// and adds twice before a bitwise write. Values must match node exactly. +#[test] +fn feistel_round_accumulator_matches_node() { + const SOURCE: &str = r#" +const P = new Int32Array(18); +const S = new Int32Array(1024); +for (let i = 0; i < P.length; i++) P[i] = (i * 40503 + 7) | 0; +for (let i = 0; i < S.length; i++) S[i] = (i * 2654435761) | 0; + +function encipher(lr: number[], off: number, P: Int32Array, S: Int32Array): void { + let n: number; + let l = lr[off]; + let r = lr[off + 1]; + l ^= P[0]; + let i = 0; + while (i < 16) { + n = S[l >>> 24]; + n += S[0x100 | ((l >> 16) & 0xff)]; + n ^= S[0x200 | ((l >> 8) & 0xff)]; + n += S[0x300 | (l & 0xff)]; + r ^= n ^ P[++i]; + n = S[r >>> 24]; + n += S[0x100 | ((r >> 16) & 0xff)]; + n ^= S[0x200 | ((r >> 8) & 0xff)]; + n += S[0x300 | (r & 0xff)]; + l ^= n ^ P[++i]; + } + lr[off] = r ^ P[17]; + lr[off + 1] = l; +} + +const lr = [0x01234567, 0x89abcdef]; +for (let c = 0; c < 64; c++) encipher(lr, 0, P, S); +console.log(lr[0] + "," + lr[1]); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let bin = compile(dir.path(), SOURCE); + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!(node.status.success(), "node failed"); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!(out.status.success(), "binary failed (gc_stress={stress})"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&node.stdout), + "Feistel state diverged from node (gc_stress={stress})" + ); + } +} diff --git a/crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs b/crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs new file mode 100644 index 0000000000..d5e1e88a9e --- /dev/null +++ b/crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs @@ -0,0 +1,208 @@ +//! #9363 (A): a module-global typed-array receiver must carry the same +//! Number-by-construction proof a body-local `const` view does. +//! +//! `collectors/ptr_shape_numeric.rs` proved `view[i]` is Number-or-`undefined` +//! only from `numeric_ta_views` (spec-proven `TaPtr` params) or +//! `const_local_inits` (a compiler-visible `const` init in the SCANNED body). +//! A module-global `const buf = new Uint8Array(N)` read inside a function had +//! neither, so `acc += buf[i]` lost the accumulator's numeric proof, and the +//! add lowered through the rooted `guarded_add` diamond — a GC shadow-frame +//! load + store + `js_write_barrier_root_nanbox` per element, plus the +//! dynamic-add cold arm. +//! +//! Measured: 444 ms -> 94 ms, identical to the body-local receiver, against +//! node's 79. +//! +//! ATTRIBUTION, corrected by measurement. The missing proof ALSO leaves a +//! per-iteration `load volatile @PERRY_GC_POLL_ARMED` in the loop +//! (`loop_may_allocate` stays conservative when the `+` is not inert), and the +//! obvious story is that the volatile load blocks vectorization. It does not +//! pay: removing the poll under the same construction proof measured 94 ms +//! either way, and did not vectorize either. The whole 4.7x is the rooting +//! diamond. See the note in the test body and #9363. +//! +//! The pin is structural rather than a timing assertion: the dynamic-add +//! helper is the artifact the missing proof produced, and it is absent iff the +//! proof is present. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// `viaGlobal` reads a module-global view; `viaLocalConst` reads a body-local +/// one. The two must compile to the same shape of inner loop — that equality +/// is the fact under test, and it is what makes `viaLocalConst` a control +/// rather than a second assertion. +const SOURCE: &str = r#" +const N = 4096; +const gbuf = new Uint8Array(N); +for (let i = 0; i < N; i++) gbuf[i] = i % 256; + +function viaGlobal(): number { + let s = 0; + for (let i = 0; i < N; i++) s += gbuf[i]; + return s; +} + +function viaLocalConst(): number { + const b = new Uint8Array(N); + for (let i = 0; i < N; i++) b[i] = i % 256; + let s = 0; + for (let i = 0; i < N; i++) s += b[i]; + return s; +} + +console.log(viaGlobal() + "," + viaLocalConst()); +"#; + +const EXPECTED: &str = "522240,522240\n"; + +fn compile(dir: &Path, source: &str, extra_env: &[(&str, &str)]) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let mut cmd = Command::new(perry_bin()); + cmd.current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1"); + for (k, v) in extra_env { + cmd.env(k, v); + } + let compile = cmd.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 kept_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") +} + +/// The body of one emitted function, by its perry symbol suffix. +fn function_body(ir: &str, suffix: &str) -> String { + let mut out = String::new(); + let mut inside = false; + for line in ir.lines() { + if line.starts_with("define ") { + inside = line.contains(&format!("__{suffix}(")); + } else if inside { + if line.starts_with('}') { + break; + } + out.push_str(line); + out.push('\n'); + } + } + assert!(!out.is_empty(), "no emitted body found for `{suffix}`"); + out +} + +fn count(body: &str, needle: &str) -> usize { + body.lines().filter(|l| l.contains(needle)).count() +} + +fn run(bin: &Path, dir: &Path) -> Output { + Command::new(bin) + .current_dir(dir) + .output() + .expect("run compiled binary") +} + +/// A module-global view receiver earns the same numeric proof as a body-local +/// one: a native `fadd` rather than the rooted dynamic-add diamond. +#[test] +fn module_global_view_receiver_earns_the_numeric_proof() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE, &[]); + let ir = kept_ir(&stderr); + + let global = function_body(&ir, "viaGlobal"); + let local = function_body(&ir, "viaLocalConst"); + + // The control must itself be clean, or the equality below proves nothing. + assert_eq!( + count(&local, "js_dynamic_string_or_number_add"), + 0, + "control (body-local receiver) unexpectedly lost its numeric proof — \ + this test can no longer distinguish anything" + ); + + assert_eq!( + count(&global, "js_dynamic_string_or_number_add"), + 0, + "module-global receiver still routes `acc += buf[i]` through the \ + dynamic-add helper: the accumulator lost its Number-by-construction \ + proof" + ); + + // NOT asserted: that the module-global body has no GC poll. It still has + // one, because `can_lower_buffer_access_without_calls` demands a tracked + // `buffer_view_slots` entry that a module global never gets, so + // `loop_may_allocate` stays conservative. Admitting the read as inert + // under the construction proof was implemented and MEASURED FLAT (94 ms + // either way), and it did not unblock vectorization either — the + // remaining blocker is #9360's per-element admission-cache probe, a + // control-flow diamond in the loop body. Since `expr_is_inert_primitive` + // also governs rooting decisions, an unmeasured widening of it does not + // ship. See #9363. + + let out = run(&bin, dir.path()); + assert!(out.status.success(), "binary failed"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + EXPECTED, + "both receivers must sum identically" + ); +} + +/// The proof is a CONSTRUCTION proof, not an annotation: a reassigned +/// module-global must not be admitted, because a later write can put anything +/// in the binding. `module_global_proven_types` already excludes reassigned +/// bindings — this pins that the exclusion is load-bearing here. +#[test] +fn reassigned_module_global_is_not_admitted() { + const REASSIGNED: &str = r#" +const N = 64; +let gbuf: any = new Uint8Array(N); +for (let i = 0; i < N; i++) gbuf[i] = i; + +function sum(): number { + let s = 0; + for (let i = 0; i < N; i++) s += gbuf[i]; + return s; +} + +const first = sum(); +gbuf = "not a buffer"; +console.log(first + "," + typeof gbuf); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _stderr) = compile(dir.path(), REASSIGNED, &[]); + let out = run(&bin, dir.path()); + assert!(out.status.success(), "binary failed"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + "2016,string\n", + "a reassigned module global must still read correctly" + ); +} diff --git a/crates/perry/tests/issue_9379_packed_f64_clone_poll.rs b/crates/perry/tests/issue_9379_packed_f64_clone_poll.rs new file mode 100644 index 0000000000..82cc0cc610 --- /dev/null +++ b/crates/perry/tests/issue_9379_packed_f64_clone_poll.rs @@ -0,0 +1,212 @@ +//! #9379: the packed-f64 loop clone does not need a GC poll, and paid dearly +//! for having one. +//! +//! `stmt/loops.rs` already skips the back-edge poll inside three loop-clone +//! fact scopes, with the reasoning written out there: a poll exists so an +//! ALLOCATING body can defer a collection, `loop_may_allocate` answers from +//! the HIR (where `arr[i] = e` is a generic `IndexSet` that CAN reallocate), +//! and inside a fact scope codegen knows better because the clone is call-free +//! or it is not entered. That comment even anticipates the next clone admitted +//! on the identical argument. +//! +//! The packed-f64 clone IS that clone and was simply not listed. Its entry +//! guard proves a live packed raw-f64 plain Array with the loop window in +//! bounds; reads and writes lower to bare `double` load/store over existing +//! slots, so nothing grows, reallocates, or writes a heap edge; and the +//! matcher admits no calls, closures or awaits into the body. +//! +//! ## Why it cost more than its own instructions +//! +//! The poll's armed word is loaded VOLATILE, which is a clobber inside the +//! loop, so the cached packed receiver base had to be re-derived on every +//! element. That is why striding the poll 1-in-64 (#9316) did not recover the +//! loss while removing it does: the cost was the clobber, not the frequency. +//! +//! Measured on `bench_numeric_array_numeric` (250k x 250, quiet host): +//! 45 -> 38 ms against node's 38. A forced-arm build with polls disabled +//! entirely also lands on 38, so this recovers the whole gap and no more. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// The `bench_numeric_array_numeric` shape: a read-modify-write over a +/// `number[]` that the packed-f64 range tier claims. +const SOURCE: &str = r#" +const SIZE = 4096; +const arr: number[] = []; +for (let i = 0; i < SIZE; i++) arr.push(i); +let checksum = 0; +for (let iter = 0; iter < 8; iter++) { + for (let i = 0; i < arr.length; i++) arr[i] = arr[i] + 1; + checksum = checksum + arr[0] + arr[arr.length - 1]; +} +console.log("checksum:" + checksum); +"#; + +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 kept_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") +} + +/// The emitted lines belonging to the packed-f64 fast clone: from its loop +/// condition block to its exit. Everything outside is another tier's code and +/// is none of this test's business. +fn packed_clone_region(ir: &str) -> String { + let lines: Vec<&str> = ir.lines().collect(); + let start = lines + .iter() + .position(|l| l.starts_with("for.packed_f64_fast.cond")) + .unwrap_or_else(|| panic!("no packed-f64 fast clone in the emitted IR")); + let end = lines[start..] + .iter() + .position(|l| l.starts_with("for.packed_f64_fast.exit")) + .map(|off| start + off) + .unwrap_or(lines.len()); + lines[start..end].join("\n") +} + +fn run(bin: &Path, dir: &Path, gc_stress: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if gc_stress { + command + .env("PERRY_GC_HEAP_LIMIT", "8") + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1"); + } + command.output().expect("run compiled binary") +} + +/// The clone's fast block carries no poll, and the program is still correct — +/// including under forced, verified evacuation, which is the arm that matters +/// when a safepoint has been removed. +#[test] +fn packed_f64_clone_emits_no_poll_and_stays_correct() { + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), SOURCE); + let ir = kept_ir(&stderr); + + // Vacuity guard first: the absence below is only meaningful while the + // fixture still ADMITS the packed-f64 clone. If the tier stops claiming + // this loop, "no poll" would pass for the wrong reason. + assert!( + ir.contains("packed_f64_range_store.fast") || ir.contains("for.packed_f64_fast"), + "fixture no longer admits the packed-f64 loop clone, so the poll \ + assertion below would pass vacuously" + ); + + // Count polls INSIDE the clone only. The module's other loops (the fill + // loop, the outer iteration loop) are not claimed by this tier and keep + // their polls legitimately — counting module-wide would assert something + // this change never claimed. + let clone_polls = packed_clone_region(&ir) + .lines() + .filter(|l| l.contains("PERRY_GC_POLL_ARMED")) + .count(); + assert_eq!( + clone_polls, 0, + "the packed-f64 clone still polls the GC; its volatile armed load is a \ + clobber inside the loop, which forces the cached receiver base to be \ + re-derived per element" + ); + + // Node is the oracle rather than a hand-computed constant. My first draft + // asserted a value I derived by hand and got wrong; perry was right and the + // test was the bug. An oracle cannot make that mistake. + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!(node.status.success(), "node failed on the oracle fixture"); + let expected = String::from_utf8_lossy(&node.stdout).into_owned(); + + for stress in [false, true] { + let out = run(&bin, dir.path(), stress); + assert!( + out.status.success(), + "binary failed (gc_stress={stress})\nstderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + expected, + "packed-f64 clone produced the wrong sum (gc_stress={stress})" + ); + } +} + +/// A loop the clone does NOT claim keeps its poll: the skip is scoped to the +/// fact, not applied to loops generally. Here the body calls out, so the +/// matcher refuses it and `loop_may_allocate` is right to demand a safepoint. +#[test] +fn a_calling_loop_still_polls() { + const CALLING: &str = r#" +const arr: number[] = []; +for (let i = 0; i < 512; i++) arr.push(i); +const box: string[] = []; +for (let i = 0; i < arr.length; i++) { + arr[i] = arr[i] + 1; + box.push("g" + (i % 3)); // allocates: the clone must not claim this + if (box.length > 16) box.length = 0; +} +console.log("n:" + arr[0] + "," + arr[511] + "," + box.length); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), CALLING); + let ir = kept_ir(&stderr); + assert!( + ir.contains("PERRY_GC_POLL_ARMED"), + "an allocating loop body must keep its GC poll — the #9379 skip is \ + scoped to the packed-f64 fact, not a general licence" + ); + let node = Command::new("node") + .current_dir(dir.path()) + .arg("--experimental-strip-types") + .arg(dir.path().join("main.ts")) + .output() + .expect("run node"); + assert!(node.status.success(), "node failed on the oracle fixture"); + let out = run(&bin, dir.path(), true); + assert!(out.status.success(), "binary failed under gc stress"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&node.stdout) + ); +}