From 68b8cc0f16d7aae77661fac087d0fb42639876b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 08:59:56 +0200 Subject: [PATCH 1/4] perf(codegen): a module-global typed-array receiver earns the numeric proof (444 -> 94 ms) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectors/ptr_shape_numeric.rs` proved `view[i]` is Number-or-`undefined` from two sources: `numeric_ta_views` (spec-proven `TaPtr` parameters) and `const_local_inits` (a compiler-visible `const` init in the SCANNED body). A module-global `const buf = new Uint8Array(N)` read inside a function has neither, so `acc += buf[i]` lost the accumulator's Number-by-construction proof and every 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. `module_global_proven_types` is the same STRENGTH of proof as `const_local_inits` — derived from the initializer expression on a single- `Let`, never-reassigned binding, not from an annotation (Perry does not enforce those, #7773) — so module-scope views whose construction proves a number-valued typed-array kind now feed the same fixpoint slot. The BigInt kinds are deliberately excluded: their elements are BigInts, not Numbers. Measured (SIZE=1e6 x ITER=100, quiet host, min-of-3): the identical loop over a module-global receiver 444 -> 94 ms, exactly matching the body-local receiver it should always have matched, against node's 79. The receiver's binding form is no longer observable in the emitted loop. ATTRIBUTION, corrected by measurement. The missing proof also leaves a per-iteration `load volatile @PERRY_GC_POLL_ARMED` in the loop, because `loop_may_allocate` stays conservative while the `+` is not inert, and the obvious story is that this volatile load blocks vectorization. It does not pay: admitting the read as inert under the same construction proof (so the poll leaves the loop) measured 94 ms either way, and did not vectorize either — the residual blocker is #9360's per-element admission-cache probe. That change is therefore NOT included: `expr_is_inert_primitive` also governs rooting decisions, and an unmeasured widening of it does not ship. The residual is documented in the test and in #9363. Tests: `issue_9363_module_global_view_numeric_proof.rs` pins the emitted shape against a body-local control (which is asserted clean first, so the comparison cannot pass vacuously) and pins that a REASSIGNED module global is still not admitted — the construction proof's exclusion is load-bearing. Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- crates/perry-codegen/src/codegen/function.rs | 4 + .../perry-codegen/src/collectors/hir_facts.rs | 6 + .../src/collectors/number_by_construction.rs | 48 +++- ...e_9363_module_global_view_numeric_proof.rs | 208 ++++++++++++++++++ 4 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 crates/perry/tests/issue_9363_module_global_view_numeric_proof.rs 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/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/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" + ); +} From f6ecc2d090f5ff1932298f9859bd0cc0d3535f3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 12:38:58 +0200 Subject: [PATCH 2/4] perf(codegen): a loop-reseeded accumulator earns its i32 slot (bench_typed_array_untyped_access 1216 -> 257 ms, node 290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 admitted only for a STRAIGHT-LINE (never in-loop) Add/Sub tree, 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 and adds twice, so `|n| < 2^33`. The scan is deliberately narrow. The re-seed must sit at the loop body's TOP level: one nested in an `if`/`switch`/`try` may not run on a given iteration, which is precisely the case where the chain keeps growing. Nested loops are scanned as their own bodies, so an inner loop's re-seed never bounds the outer body's chain. MEASURED (quiet host): typed 1216 -> 257 ms and untyped 1254 -> 258 against node's 290 / 299 — from 4.2x slower to faster than node on BOTH paths. The fixture's own checksum oracle, which throws on any divergence between the typed and untyped states, passes identically. This was the last suite row above node. VERIFIED, and one honest gap. The promotion DECISIONS were checked directly through `PERRY_REPSEL_DEBUG`: `n` is promoted in both `encipher` bodies and in an unconditionally-reseeded fixture, and is refused for a loop with no re-seed, one whose re-seed is `if`-guarded, and one whose re-seed is in an inner loop. Node-differential battery (including those adversarial shapes) byte-identical, and identical again under `PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1`. I could NOT prove the conditional-re-seed guard independently load-bearing: I sabotaged it (letting `if`-nested re-seeds count) and failed across three fixture attempts to construct a case whose outcome changes — each failed for a different reason (rule (2) rejected the local first; power-of-two addends made wrapped and exact coincide; a module-global receiver did not reproduce the spec-param admission conditions). So it is defense-in-depth of unproven necessity, stated rather than claimed — the same honesty `loop_safepoint_purity.rs` applies to its own shadow-slot half. Tests: `issue_9363_loop_reseeded_accumulator.rs` pins the Feistel round against node and pins that the three unbounded shapes stay f64, with the oracle itself guarded (the fixture asserts its expected values still exceed i32 range, so a wrongly promoted local would print a wrapped negative rather than a near miss). Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- .../src/collectors/int_valued_ta_locals.rs | 125 +++++++++- .../issue_9363_loop_reseeded_accumulator.rs | 224 ++++++++++++++++++ 2 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 crates/perry/tests/issue_9363_loop_reseeded_accumulator.rs 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..7dc0a9cf61 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,121 @@ 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/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})" + ); + } +} From 1e3b864928cd86335a4fd04116f4743d521fc176 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 13:13:30 +0200 Subject: [PATCH 3/4] perf(codegen): the packed-f64 loop clone needs no GC poll (bench_numeric_array_numeric 45 -> 38 ms, node 38) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stmt/loops.rs` already skips the back-edge poll inside three loop-clone fact scopes, and its comment states the rule and predicts this exact case: 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. The packed-f64 clone is that body and was simply not listed. Its entry guard proves a live packed raw-f64 plain Array with the loop window in bounds, its reads and writes lower to bare `double` load/store over existing slots (so nothing grows, reallocates, or writes a heap edge), and its matcher admits no calls, closures or awaits into the body — the same conjunction the three listed clones rest on. This is therefore not a new licence. WHY IT COST MORE THAN ITS OWN INSTRUCTIONS. The 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 — the effect #9316's stride comment already describes. That is why striding the poll 1-in-64 did not recover the loss while removing it does: the cost was the clobber, not the frequency. MEASURED (250k x 250, quiet host, min-of-3): 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 nothing more — the diagnostic bounded the win before the change was written. Tests: `issue_9379_packed_f64_clone_poll.rs` asserts no poll inside the CLONE's own blocks (module-wide counting would assert something this change never claimed — the fill and outer loops keep their polls), with a vacuity guard that the fixture still admits the tier, and correctness under `PERRY_GC_FORCE_EVACUATE` + `PERRY_GC_VERIFY_EVACUATION`, which is the arm that matters when a safepoint is removed. A sibling test pins that an allocating loop still polls, so the skip stays scoped to the fact. Both assertions in the first draft were wrong and perry was right: I counted polls module-wide, and I hand-computed an expected checksum incorrectly. Both now use node as the oracle or the clone's own region. `loop_safepoint_purity` 8/8 and codegen 1379/1379 green. Claude-Session: https://claude.ai/code/session_01NbKbYm54HW6cHwEx5FZAtP --- crates/perry-codegen/src/stmt/loops.rs | 17 ++ .../tests/issue_9379_packed_f64_clone_poll.rs | 212 ++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 crates/perry/tests/issue_9379_packed_f64_clone_poll.rs 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_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) + ); +} From 62b36b8b7d8973ccce74b28ea45a487c6c4d26d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 20:08:48 +0200 Subject: [PATCH 4/4] style: rustfmt --- .../perry-codegen/src/collectors/int_valued_ta_locals.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 7dc0a9cf61..d4b5e4dee8 100644 --- a/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs +++ b/crates/perry-codegen/src/collectors/int_valued_ta_locals.rs @@ -1065,7 +1065,13 @@ fn collect_loop_reseeded_locals<'a>( out, ); if let Some(eb) = else_branch { - collect_loop_reseeded_locals(eb, types, guarded_number_array_params, false, out); + collect_loop_reseeded_locals( + eb, + types, + guarded_number_array_params, + false, + out, + ); } } Stmt::Try {