diff --git a/.github/workflows/gc-native-roots.yml b/.github/workflows/gc-native-roots.yml index e187683f7a..a0c8bf679b 100644 --- a/.github/workflows/gc-native-roots.yml +++ b/.github/workflows/gc-native-roots.yml @@ -535,26 +535,15 @@ jobs: # Same answer as the shadow stack, and a collection that actually # moved something. Without the movement assert this passes with the # conservative scan doing all the rooting (#7336, #7338). - # - # PERRY_GC_DIAG=1 is what makes the evacuation assert able to see - # anything: `[gc-copy-minor] ran copied_objects=...` is printed only - # under that variable (`gc/copying.rs`). Without it the trace holds - # nothing but the probe's own `#gcmetric` lines, the assert reads - # 0 copying minors / 0 objects copied off an empty file, and the step - # fails no matter how the collector behaved — which is how this arm - # read on its first-ever execution (three of four arms in this matrix - # were permanently queued until #7393). Diagnostics go to stderr only, - # so the control diff below is unaffected. ./target/perry-dev/perry "$probe" -o /tmp/inproc-09-control PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \ /tmp/inproc-09-control > /tmp/inproc-09.control.out 2>/dev/null - PERRY_GC_DIAG=1 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 \ + PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 \ PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \ /tmp/inproc-09 > /tmp/inproc-09.out 2> /tmp/inproc-09.err diff /tmp/inproc-09.control.out /tmp/inproc-09.out \ || { echo "::error::in-process RS4GC diverged from the shadow-stack control"; exit 1; } - python3 scripts/gc_evacuation_liveness_assert.py /tmp/inproc-09.err \ - --probe "09_try_catch_roots (in-process)" + python3 scripts/gc_evacuation_liveness_assert.py /tmp/inproc-09.err # And it must be RS4GC doing the lowering, not a per-function bail to # the bridge -- which would make this arm green while testing the diff --git a/changelog.d/7414-macos-rs4gc-inprocess-gc-diag.md b/changelog.d/7414-macos-rs4gc-inprocess-gc-diag.md deleted file mode 100644 index 3290451409..0000000000 --- a/changelog.d/7414-macos-rs4gc-inprocess-gc-diag.md +++ /dev/null @@ -1,12 +0,0 @@ -**Fixed** the macOS `native-roots-rs4gc` arm's in-process step could never pass. - -It asserts that a copying minor moved objects by counting -`[gc-copy-minor] ran copied_objects=` lines, but both prints are gated on -`PERRY_GC_DIAG` and the step never set it. The trace held only the probe's own -`#gcmetric` lines, so the assert read 0/0 off an effectively empty file and -failed regardless of collector behaviour — the inverse of a gate that cannot -fail. It shipped with the step in #7339 and had never run, because three of four -arms in this matrix were permanently queued until #7393. - -The liveness assert now distinguishes "no collector diagnostics at all" from -"the collector moved nothing", which the old message conflated. diff --git a/changelog.d/7415-dominance-corpus.md b/changelog.d/7415-dominance-corpus.md deleted file mode 100644 index 500887f85f..0000000000 --- a/changelog.d/7415-dominance-corpus.md +++ /dev/null @@ -1,27 +0,0 @@ -**Fixed** the `GC Root Dominance` gate has been unable to return a verdict since -#7370 made statepoints the default. - -The checker's entire vocabulary is `call void @js_shadow_slot_bind(...)`. Under -the stack-map lowering the final IR pass resolves those indices to native allocas -and **removes the calls** (`FunctionCodegen::stack_map_slot_count`), so the corpus -compiled 144 modules containing **zero** root stores. The gate reported -`violations: 0` and then correctly refused to pass, because its `--min-binds` -liveness floor caught that its own subject never ran — CLAUDE.md's fourth hazard, -working as designed. - -Both corpus scripts now pin `PERRY_RS4GC=0`. That is sound rather than a dodge: -#7340 split the root-set *analysis* from its lowering, and this gate is about the -analysis, which both backends share. The shadow stack also remains the production -lowering wherever the runtime cannot walk frames. - -Measured, same binary and source, only the knob differing: - -``` -arm=default js_shadow_slot_bind calls = 0 ← reproduces the CI signature -arm=rs4gc0 js_shadow_slot_bind calls = 9 -``` - -#7370 already fixed the equivalent breakage in the unit tests — `helpers.rs` -records that eight tests broke when the default flipped and were given -`NativeRootsPin::shadow()`. The corpus shell scripts were the same breakage in a -different idiom, and were missed. diff --git a/changelog.d/7416-i64-integer-locals.md b/changelog.d/7416-i64-integer-locals.md new file mode 100644 index 0000000000..e6db0db3a4 --- /dev/null +++ b/changelog.d/7416-i64-integer-locals.md @@ -0,0 +1,40 @@ +**Fixed** JS `%` on an integer-valued local lowered to `frem`, which is not an +aarch64 instruction and becomes an `fmod` library call. `bench_bitwise` was +**20.4x slower than Node**; it is now **1.39x** — a 14.6x improvement, 55108ms to +3763ms, with the Node-verified `CHECKSUM:525000000` unchanged. + +A guarded `srem` fast path already existed, but every gate in front of it asked +the wrong question. `is_integer_valued_expr` resolves a `LocalGet` through +`integer_locals`, which is an **i32-range** property — it also gates i32 shadow +slots, so widening it would have placed an i32-overflowing value into an i32 slot. +The `%` path converts to **i64** and only needs integer-valued-within-i64. + +A new `collect_int_valued_i64_locals` supplies that weaker property as a +magnitude lattice, read only by the `%` gate. `integer_locals` is untouched. + +Three holes were found and closed while building it, each of which would have +produced silently wrong arithmetic: + +* **`Mul` blowup.** A pure integrality predicate let `(a*b*c) % n` reach + `fptosi ... to i64` with a 93-bit product — poison. The lattice tracks + magnitude and gates at 2^62. This also closes the same pre-existing hole for + i32 locals. +* **Zero divisor.** An early version routed `1000 % d` into `srem` where `d` + decrements through zero: `srem(x, 0)` is UB where JS requires NaN. The divisor + is now restricted to a non-zero integer literal. +* **The saturation argument.** "±constant cannot leave i64 in finite time" is + false for large deltas — `a = a + 1e18` escapes in ~10 iterations. Replaced + with a hard IEEE-754 bound: once `ulp(v) >= 4D`, `v ± d` rounds back to `v` + exactly, so `|L| <= 2^(55+log2 D)` forever. Deltas are capped at 64. + +Also of note: the gate that actually decides this is +`expr/mod.rs::lower_numeric_binary_value`, which intercepts numeric binary ops +before `binary::lower` and only handed `Mod` off when the dividend had an i32 +counter slot. There are six `frem` emission sites; widening the predicate alone +changed nothing, and the IR acceptance test caught that. + +Verified: hot-function IR goes `frem 4 -> 0`, `srem 0 -> 4`; the edge-case +differential (including both `-0` cases, via `Object.is` rather than `===`) is +byte-identical to Node 26.5.1; `cargo test -p perry-codegen --lib` 632 passed; +26/27 math/number/int gap tests pass with the one failure pre-existing and +unrelated. diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index 7924a5d97e..e32aef92db 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -967,6 +967,7 @@ pub(super) fn compile_closure( try_depth: 0, pending_declares: Vec::new(), integer_locals: native_facts.integer_locals(), + int_valued_i64_locals: native_facts.int_valued_i64_locals(), not_bigint_locals: native_facts.not_bigint_locals(), unsigned_i32_locals: native_facts.unsigned_i32_locals(), // Conservative: treat every slot as possibly-bound (param binds are diff --git a/crates/perry-codegen/src/codegen/entry.rs b/crates/perry-codegen/src/codegen/entry.rs index 4ca37d538f..682251cbb2 100644 --- a/crates/perry-codegen/src/codegen/entry.rs +++ b/crates/perry-codegen/src/codegen/entry.rs @@ -776,6 +776,7 @@ pub(super) fn compile_module_entry( try_depth: 0, pending_declares: Vec::new(), integer_locals: main_native_facts.integer_locals(), + int_valued_i64_locals: main_native_facts.int_valued_i64_locals(), not_bigint_locals: main_native_facts.not_bigint_locals(), unsigned_i32_locals: main_native_facts.unsigned_i32_locals(), shadow_slots_bound: main_shadow_slot_map.values().copied().collect(), @@ -1439,6 +1440,7 @@ pub(super) fn compile_module_entry( try_depth: 0, pending_declares: Vec::new(), integer_locals: init_native_facts.integer_locals(), + int_valued_i64_locals: init_native_facts.int_valued_i64_locals(), not_bigint_locals: init_native_facts.not_bigint_locals(), unsigned_i32_locals: init_native_facts.unsigned_i32_locals(), shadow_slots_bound: init_shadow_slot_map.values().copied().collect(), diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index 4acec095d4..3d11fb52ad 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -763,6 +763,7 @@ pub(super) fn compile_function( try_depth: 0, pending_declares: Vec::new(), integer_locals: native_facts.integer_locals(), + int_valued_i64_locals: native_facts.int_valued_i64_locals(), not_bigint_locals: native_facts.not_bigint_locals(), unsigned_i32_locals: native_facts.unsigned_i32_locals(), shadow_slot_map, diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 2bfe4f8bf5..25d628ecde 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -498,6 +498,7 @@ pub(super) fn compile_method( try_depth: 0, pending_declares: Vec::new(), integer_locals: native_facts.integer_locals(), + int_valued_i64_locals: native_facts.int_valued_i64_locals(), not_bigint_locals: native_facts.not_bigint_locals(), unsigned_i32_locals: native_facts.unsigned_i32_locals(), // Conservative: treat every slot as possibly-bound (param binds are @@ -1555,6 +1556,7 @@ pub(super) fn compile_static_method( try_depth: 0, pending_declares: Vec::new(), integer_locals: native_facts.integer_locals(), + int_valued_i64_locals: native_facts.int_valued_i64_locals(), not_bigint_locals: native_facts.not_bigint_locals(), unsigned_i32_locals: native_facts.unsigned_i32_locals(), // Conservative: treat every slot as possibly-bound (param binds are diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index 6c596777f4..c3928efd08 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -34,6 +34,15 @@ pub(crate) type NativeRegionFactGraph = TypeFacts; #[derive(Debug, Clone, Default)] pub(crate) struct RepresentationFacts { pub integer_locals: HashSet, + /// Locals that are integer-valued within **i64** range but NOT provably + /// within i32 range, mapped to a conservative `log2(|value|)` bound. + /// + /// Deliberately separate from `integer_locals`, which is an i32-RANGE set + /// feeding i32 shadow slots (`needs_i32_slot`) — widening that would place + /// an i32-overflowing value into an i32 slot. The `%` fast path converts to + /// i64 and only needs i64-range integrality, so it consults this set too. + /// See `collectors/int_valued_i64_locals.rs`. This is the ONLY consumer. + pub int_valued_i64_locals: std::collections::HashMap, pub unsigned_i32_locals: HashSet, /// Locals whose runtime value provably can never be a BigInt (every write /// is a non-BigInt expression). Seeds `is_provably_not_bigint`, which gates @@ -164,6 +173,10 @@ impl TypeFacts { &self.representation.integer_locals } + pub(crate) fn int_valued_i64_locals(&self) -> &std::collections::HashMap { + &self.representation.int_valued_i64_locals + } + pub(crate) fn unsigned_i32_locals(&self) -> &HashSet { &self.representation.unsigned_i32_locals } @@ -564,9 +577,14 @@ pub(crate) fn collect_type_facts( compile_time_constants, &integer_locals, ); + // i64-range integer-valued locals for the `%` fast path. Independent of + // `integer_locals` (which is i32-RANGE and drives i32 shadow slots); this + // one is consumed only by `type_analysis::numeric::integer_magnitude_bits`. + let int_valued_i64_locals = super::int_valued_i64_locals::collect_int_valued_i64_locals(stmts); let graph = TypeFacts { representation: RepresentationFacts { integer_locals: integer_locals.clone(), + int_valued_i64_locals, unsigned_i32_locals, not_bigint_locals, int_valued_ta_locals, diff --git a/crates/perry-codegen/src/collectors/int_valued_i64_locals.rs b/crates/perry-codegen/src/collectors/int_valued_i64_locals.rs new file mode 100644 index 0000000000..0e3ba22df5 --- /dev/null +++ b/crates/perry-codegen/src/collectors/int_valued_i64_locals.rs @@ -0,0 +1,735 @@ +//! Flow analysis: locals that are integer-valued **within i64 range**, even +//! though they are *not* provably within i32 range. +//! +//! ## Why this is a separate set from `integer_locals` +//! +//! `collectors::integer_locals` answers "is this local i32-RANGE?", because its +//! consumers (`needs_i32_slot`, `canonical_i32_value_eligible` in +//! `stmt/let_stmt.rs`, the loop-counter lanes in `stmt/loops.rs`) put the value +//! into an **i32 shadow slot**. A local like `a` in `a = a + 1` is correctly +//! *rejected* there: an unbounded increment chain can exceed i32, and admitting +//! it would silently truncate. That judgment must not be widened. +//! +//! But the `%` integer fast path in `expr/binary.rs` converts with +//! `fptosi double -> **i64**`, so it does not need i32-range at all — it needs +//! "integer-valued, and small enough that `fptosi` to i64 is exact and +//! in-range". It was asking `integer_locals` the wrong question, so +//! `bench_bitwise`'s `a % 1000` / `(a * 3) % 10000` fell through to +//! `frem double`, which on AArch64 is not an instruction and lowers to a +//! `bl _fmod` libm call — the dominant cost in that benchmark. +//! +//! This module answers the *right* question for that consumer, and nothing +//! else consumes it. +//! +//! ## Admission rule +//! +//! A local `L` is admitted only when ALL of the following hold: +//! +//! 1. `L` is declared by exactly one `Stmt::Let` whose init is an +//! `Expr::Integer(v)` literal with `|v| <= 2^31`. (Params are excluded by +//! construction — their incoming argument is an unmodeled write. A second +//! `Let` for the same id, or a non-literal init, rejects.) +//! 2. Every write to `L` anywhere in the function is one of: +//! - `LocalSet(L, Integer(v))` with `|v| <= 2^31` +//! - `LocalSet(L, Add(LocalGet(L), Integer(d)))` with `|d| <= 64` +//! - `LocalSet(L, Add(Integer(d), LocalGet(L)))` with `|d| <= 64` +//! - `LocalSet(L, Sub(LocalGet(L), Integer(d)))` with `|d| <= 64` +//! - `Update { id: L, .. }` (`++` / `--`, `d = 1`) +//! Any other write shape rejects `L`. In particular +//! `Sub(Integer(d), LocalGet(L))` (`L = d - L`) is **rejected**: it negates +//! `L`, so a step is no longer a bounded *translation* and the saturation +//! argument below collapses. `Mul` is rejected outright — repeated +//! multiplication leaves i64 range in a few dozen iterations. +//! 3. `L` is never written inside a closure body and never appears in a +//! closure's `mutable_captures` (mirrors how `integer_locals.rs` treats +//! `closure_written`), and is not a `catch` clause parameter. +//! +//! ## Soundness: why the value stays inside i64 +//! +//! `L` starts as an i64-representable integer, and rule (2) makes every write +//! either a reset to a literal `<= 2^31` or a translation by a compile-time +//! constant `d` with `|d| <= D <= 64`. So `L` is always an integral f64, and it +//! can only grow by `<= D` per step. +//! +//! The bound is not a hand-wave about iteration counts — IEEE-754 makes it a +//! hard ceiling. For an f64 `v` with `2^e <= |v| < 2^(e+1)`, +//! `ulp(v) = 2^(e-52)`. Once `ulp(v) >= 4D` — i.e. once `e >= 54 + log2(D)` — +//! adding `+-d` with `|d| <= D <= ulp/4` is *strictly* inside the +//! round-to-nearest half-ulp window, so `v + d` rounds back to `v` exactly. +//! The value becomes a **fixed point and can never grow again**. Approaching +//! that threshold from below, one final step can overshoot by at most +//! `D + ulp/2`, so +//! +//! ```text +//! |L| <= max(2^31, 2^(55 + log2 D)) for all time. +//! ``` +//! +//! We record `56 + ceil_log2(D)` per local (one bit of slack) as that local's +//! magnitude bound, and `MAX_DELTA = 64` caps it at `62` bits. The `%` gate +//! admits an expression only when its derived magnitude is `<= 2^62`, which +//! keeps `fptosi double -> i64` exact and in range (`i64::MAX ~= 2^63`) with a +//! full bit to spare. A local whose writes are all literal resets never grows +//! at all and is recorded at `31`. +//! +//! This is deliberately an under-approximation: anything unproven is simply +//! left to `frem`, which is always correct. + +use std::collections::{HashMap, HashSet}; + +use perry_hir::{BinaryOp, Expr, Stmt}; + +/// Largest `|delta|` admitted for a `L = L +- d` write. Caps the recorded +/// magnitude at `56 + 6 = 62` bits, which is the `%` gate's ceiling. +const MAX_DELTA: i64 = 64; + +/// Largest `|v|` admitted for a literal initialiser / literal reset. +const MAX_LITERAL: i64 = 1 << 31; + +/// Magnitude recorded for a local whose every write is a literal reset — it +/// never grows, so `|L| <= 2^31`. +const LITERAL_ONLY_BITS: u32 = 31; + +/// `56 + ceil_log2(D)`: the saturation ceiling of a `+-D` translation chain +/// (`2^(55 + log2 D)`) plus one bit of slack. +const STEP_BITS_BASE: u32 = 56; + +/// Smallest `bits` with `|v| <= 2^bits`. `ceil(log2(|v|))`, and `0` for `v == 0`. +pub(crate) fn ceil_log2_abs(v: i64) -> u32 { + let m = v.unsigned_abs(); + if m <= 1 { + return 0; + } + 64 - (m - 1).leading_zeros() +} + +/// Per-local accumulated state during the walk. +#[derive(Clone, Copy)] +struct Cand { + /// Largest `|d|` seen across `L = L +- d` writes; `0` if only literal + /// resets have been seen. + max_delta: i64, +} + +impl Cand { + fn magnitude_bits(self) -> u32 { + if self.max_delta == 0 { + LITERAL_ONLY_BITS + } else { + STEP_BITS_BASE + ceil_log2_abs(self.max_delta) + } + } +} + +/// Locals that are integer-valued within i64 range, mapped to a conservative +/// upper bound on `log2(|value|)`. Consumed only by the `%` integer fast path +/// via `type_analysis::numeric::integer_magnitude_bits`. +pub fn collect_int_valued_i64_locals(stmts: &[Stmt]) -> HashMap { + let mut w = Walk { + cands: HashMap::new(), + rejected: HashSet::new(), + }; + // Pass 1: seed candidates from integer-literal `Let` inits. + w.seed_stmts(stmts); + // Pass 2: judge every write in the function against the whitelist. + w.judge_stmts(stmts); + + w.cands + .into_iter() + .filter(|(id, _)| !w.rejected.contains(id)) + .map(|(id, c)| (id, c.magnitude_bits())) + .collect() +} + +struct Walk { + cands: HashMap, + rejected: HashSet, +} + +impl Walk { + fn reject(&mut self, id: u32) { + self.rejected.insert(id); + } + + // ---- Pass 1: seeding ------------------------------------------------- + + fn seed_stmts(&mut self, stmts: &[Stmt]) { + for s in stmts { + match s { + Stmt::Let { id, init, .. } => { + let ok = matches!(init, Some(Expr::Integer(v)) if v.unsigned_abs() <= MAX_LITERAL as u64); + if ok { + // A second `Let` for the same id is not expected + // (LocalIds are unique per function); treat it as an + // unmodeled rebinding and reject rather than trust it. + if self.cands.insert(*id, Cand { max_delta: 0 }).is_some() { + self.reject(*id); + } + } else { + self.reject(*id); + } + if let Some(e) = init { + self.seed_expr(e); + } + } + Stmt::Expr(e) | Stmt::Throw(e) => self.seed_expr(e), + Stmt::Return(opt) => { + if let Some(e) = opt { + self.seed_expr(e); + } + } + Stmt::If { + condition, + then_branch, + else_branch, + } => { + self.seed_expr(condition); + self.seed_stmts(then_branch); + if let Some(eb) = else_branch { + self.seed_stmts(eb); + } + } + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + self.seed_expr(condition); + self.seed_stmts(body); + } + Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(i) = init { + self.seed_stmts(std::slice::from_ref(i)); + } + if let Some(c) = condition { + self.seed_expr(c); + } + if let Some(u) = update { + self.seed_expr(u); + } + self.seed_stmts(body); + } + Stmt::Try { + body, + catch, + finally, + } => { + self.seed_stmts(body); + if let Some(c) = catch { + // A catch parameter is an unmodeled binding. + if let Some((pid, _)) = &c.param { + self.reject(*pid); + } + self.seed_stmts(&c.body); + } + if let Some(f) = finally { + self.seed_stmts(f); + } + } + Stmt::Switch { + discriminant, + cases, + } => { + self.seed_expr(discriminant); + for c in cases { + if let Some(t) = &c.test { + self.seed_expr(t); + } + self.seed_stmts(&c.body); + } + } + Stmt::Labeled { body, .. } => self.seed_stmts(std::slice::from_ref(body.as_ref())), + _ => {} + } + } + } + + /// Seeding only needs to reach `Let`s nested inside closure bodies so that + /// their ids are *known*; the judging pass rejects anything a closure + /// writes, so no candidate can survive on a closure-local basis. + fn seed_expr(&mut self, e: &Expr) { + if let Expr::Closure { body, .. } = e { + self.seed_stmts(body); + } + perry_hir::walker::walk_expr_children(e, &mut |c| self.seed_expr(c)); + } + + // ---- Pass 2: judging every write ------------------------------------- + + fn judge_stmts(&mut self, stmts: &[Stmt]) { + for s in stmts { + match s { + Stmt::Let { init, .. } => { + if let Some(e) = init { + self.judge_expr(e); + } + } + Stmt::Expr(e) | Stmt::Throw(e) => self.judge_expr(e), + Stmt::Return(opt) => { + if let Some(e) = opt { + self.judge_expr(e); + } + } + Stmt::If { + condition, + then_branch, + else_branch, + } => { + self.judge_expr(condition); + self.judge_stmts(then_branch); + if let Some(eb) = else_branch { + self.judge_stmts(eb); + } + } + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + self.judge_expr(condition); + self.judge_stmts(body); + } + Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(i) = init { + self.judge_stmts(std::slice::from_ref(i)); + } + if let Some(c) = condition { + self.judge_expr(c); + } + if let Some(u) = update { + self.judge_expr(u); + } + self.judge_stmts(body); + } + Stmt::Try { + body, + catch, + finally, + } => { + self.judge_stmts(body); + if let Some(c) = catch { + self.judge_stmts(&c.body); + } + if let Some(f) = finally { + self.judge_stmts(f); + } + } + Stmt::Switch { + discriminant, + cases, + } => { + self.judge_expr(discriminant); + for c in cases { + if let Some(t) = &c.test { + self.judge_expr(t); + } + self.judge_stmts(&c.body); + } + } + Stmt::Labeled { body, .. } => self.judge_stmts(std::slice::from_ref(body.as_ref())), + _ => {} + } + } + } + + fn judge_expr(&mut self, e: &Expr) { + match e { + Expr::LocalSet(id, rhs) => { + if self.cands.contains_key(id) { + match write_delta(*id, rhs) { + Some(d) => { + let c = self.cands.get_mut(id).expect("candidate present"); + c.max_delta = c.max_delta.max(d); + } + None => self.reject(*id), + } + } + self.judge_expr(rhs); + } + Expr::Update { id, .. } => { + // `++` / `--`: a translation by exactly 1. + if let Some(c) = self.cands.get_mut(id) { + c.max_delta = c.max_delta.max(1); + } + } + Expr::Closure { + body, + mutable_captures, + .. + } => { + // A closure can write the local out of line; the enclosing + // analysis cannot see when. Reject unconditionally, matching + // `integer_locals.rs`'s `closure_written` handling. + for id in mutable_captures { + self.reject(*id); + } + let mut written = HashSet::new(); + collect_written_ids(body, &mut written); + for id in written { + self.reject(id); + } + perry_hir::walker::walk_expr_children(e, &mut |c| self.judge_expr(c)); + } + _ => { + perry_hir::walker::walk_expr_children(e, &mut |c| self.judge_expr(c)); + } + } + } +} + +/// Classify a `LocalSet(id, rhs)` write. Returns the translation magnitude +/// (`0` for a literal reset), or `None` when the shape is not admissible. +fn write_delta(id: u32, rhs: &Expr) -> Option { + match rhs { + // Literal reset. + Expr::Integer(v) if v.unsigned_abs() <= MAX_LITERAL as u64 => Some(0), + Expr::Binary { op, left, right } => { + let d = match (op, left.as_ref(), right.as_ref()) { + // L = L + d / L = L - d + (BinaryOp::Add | BinaryOp::Sub, Expr::LocalGet(l), Expr::Integer(d)) + if *l == id => + { + *d + } + // L = d + L (commutative, same translation). + // NOTE: `L = d - L` is deliberately NOT admitted — it negates + // L, so the step is not a bounded translation. + (BinaryOp::Add, Expr::Integer(d), Expr::LocalGet(l)) if *l == id => *d, + _ => return None, + }; + let d = d.unsigned_abs(); + if d <= MAX_DELTA as u64 { + Some(d as i64) + } else { + None + } + } + _ => None, + } +} + +/// Every local id written (`LocalSet` or `Update`) anywhere in `stmts`, +/// including inside nested closures. +fn collect_written_ids(stmts: &[Stmt], out: &mut HashSet) { + struct W<'a>(&'a mut HashSet); + impl W<'_> { + fn stmts(&mut self, stmts: &[Stmt]) { + for s in stmts { + match s { + Stmt::Let { init, .. } => { + if let Some(e) = init { + self.expr(e); + } + } + Stmt::Expr(e) | Stmt::Throw(e) => self.expr(e), + Stmt::Return(opt) => { + if let Some(e) = opt { + self.expr(e); + } + } + Stmt::If { + condition, + then_branch, + else_branch, + } => { + self.expr(condition); + self.stmts(then_branch); + if let Some(eb) = else_branch { + self.stmts(eb); + } + } + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + self.expr(condition); + self.stmts(body); + } + Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(i) = init { + self.stmts(std::slice::from_ref(i)); + } + if let Some(c) = condition { + self.expr(c); + } + if let Some(u) = update { + self.expr(u); + } + self.stmts(body); + } + Stmt::Try { + body, + catch, + finally, + } => { + self.stmts(body); + if let Some(c) = catch { + self.stmts(&c.body); + } + if let Some(f) = finally { + self.stmts(f); + } + } + Stmt::Switch { + discriminant, + cases, + } => { + self.expr(discriminant); + for c in cases { + if let Some(t) = &c.test { + self.expr(t); + } + self.stmts(&c.body); + } + } + Stmt::Labeled { body, .. } => self.stmts(std::slice::from_ref(body.as_ref())), + _ => {} + } + } + } + fn expr(&mut self, e: &Expr) { + match e { + Expr::LocalSet(id, _) | Expr::Update { id, .. } => { + self.0.insert(*id); + } + Expr::Closure { body, .. } => self.stmts(body), + _ => {} + } + perry_hir::walker::walk_expr_children(e, &mut |c| self.expr(c)); + } + } + W(out).stmts(stmts); +} + +#[cfg(test)] +mod tests { + use super::*; + use perry_hir::types::Type; + + fn let_int(id: u32, v: i64) -> Stmt { + Stmt::Let { + id, + name: format!("v{id}"), + ty: Type::Number, + mutable: true, + init: Some(Expr::Integer(v)), + } + } + + fn set(id: u32, rhs: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet(id, Box::new(rhs))) + } + + fn add(id: u32, d: i64) -> Expr { + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(id)), + right: Box::new(Expr::Integer(d)), + } + } + + #[test] + fn ceil_log2_abs_is_an_upper_bound() { + for v in [0i64, 1, 2, 3, 4, 5, 7, 8, 1000, 1024, 1025, 12345678] { + let b = ceil_log2_abs(v); + assert!( + (v.unsigned_abs() as u128) <= 1u128 << b, + "{v} > 2^{b} — bound is not an upper bound" + ); + assert_eq!(b, ceil_log2_abs(-v), "sign must not matter for {v}"); + } + assert_eq!(ceil_log2_abs(3), 2); + assert_eq!(ceil_log2_abs(1000), 10); + } + + #[test] + fn admits_literal_init_with_unit_steps() { + // let a = 12345678; a = a + 1; a = 12345678; (the bench_bitwise shape) + let stmts = vec![ + let_int(9, 12345678), + set(9, add(9, 1)), + set(9, Expr::Integer(12345678)), + ]; + let out = collect_int_valued_i64_locals(&stmts); + assert_eq!(out.get(&9), Some(&56), "unit-step local should be 56 bits"); + } + + #[test] + fn literal_only_writes_stay_at_31_bits() { + let stmts = vec![let_int(1, 5), set(1, Expr::Integer(7))]; + assert_eq!(collect_int_valued_i64_locals(&stmts).get(&1), Some(&31)); + } + + #[test] + fn step_widens_the_recorded_magnitude() { + let stmts = vec![let_int(1, 0), set(1, add(1, 64))]; + assert_eq!( + collect_int_valued_i64_locals(&stmts).get(&1), + Some(&62), + "delta 64 must record 56+6 bits" + ); + } + + #[test] + fn rejects_oversized_delta() { + let stmts = vec![let_int(1, 0), set(1, add(1, 65))]; + assert!(collect_int_valued_i64_locals(&stmts).get(&1).is_none()); + } + + #[test] + fn rejects_multiplication_write() { + let stmts = vec![ + let_int(1, 2), + set( + 1, + Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(Expr::LocalGet(1)), + right: Box::new(Expr::Integer(3)), + }, + ), + ]; + assert!( + collect_int_valued_i64_locals(&stmts).get(&1).is_none(), + "L = L * 3 leaves i64 in a few dozen iterations" + ); + } + + #[test] + fn rejects_negating_write() { + // `L = 5 - L` is a reflection, not a translation. + let stmts = vec![ + let_int(1, 2), + set( + 1, + Expr::Binary { + op: BinaryOp::Sub, + left: Box::new(Expr::Integer(5)), + right: Box::new(Expr::LocalGet(1)), + }, + ), + ]; + assert!(collect_int_valued_i64_locals(&stmts).get(&1).is_none()); + } + + #[test] + fn rejects_write_of_another_local() { + let stmts = vec![let_int(1, 2), let_int(2, 3), set(1, Expr::LocalGet(2))]; + assert!(collect_int_valued_i64_locals(&stmts).get(&1).is_none()); + } + + #[test] + fn rejects_non_literal_init() { + let stmts = vec![ + Stmt::Let { + id: 1, + name: "a".into(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Number(1.5)), + }, + set(1, add(1, 1)), + ]; + assert!(collect_int_valued_i64_locals(&stmts).get(&1).is_none()); + } + + #[test] + fn rejects_oversized_literal_init() { + let stmts = vec![let_int(1, (1i64 << 31) + 1), set(1, add(1, 1))]; + assert!(collect_int_valued_i64_locals(&stmts).get(&1).is_none()); + } + + #[test] + fn rejects_closure_written_local() { + let stmts = vec![ + let_int(1, 0), + Stmt::Expr(Expr::Closure { + func_id: 0, + params: vec![], + return_type: Type::Void, + body: vec![set(1, add(1, 1))], + captures: vec![1], + mutable_captures: vec![], + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: false, + }), + ]; + assert!( + collect_int_valued_i64_locals(&stmts).get(&1).is_none(), + "a closure can write the local out of line" + ); + } + + #[test] + fn rejects_mutable_capture() { + let stmts = vec![ + let_int(1, 0), + Stmt::Expr(Expr::Closure { + func_id: 0, + params: vec![], + return_type: Type::Void, + body: vec![], + captures: vec![1], + mutable_captures: vec![1], + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: false, + }), + ]; + assert!(collect_int_valued_i64_locals(&stmts).get(&1).is_none()); + } + + #[test] + fn admits_update_expression_writes() { + let stmts = vec![ + let_int(1, 0), + Stmt::Expr(Expr::Update { + id: 1, + op: perry_hir::UpdateOp::Increment, + prefix: false, + }), + ]; + assert_eq!(collect_int_valued_i64_locals(&stmts).get(&1), Some(&56)); + } + + #[test] + fn finds_writes_nested_in_control_flow() { + // A write buried in an `if` inside a `for` must still be judged. + let stmts = vec![ + let_int(1, 0), + Stmt::For { + init: None, + condition: None, + update: None, + body: vec![Stmt::If { + condition: Expr::Bool(true), + then_branch: vec![set( + 1, + Expr::Binary { + op: BinaryOp::Mul, + left: Box::new(Expr::LocalGet(1)), + right: Box::new(Expr::Integer(3)), + }, + )], + else_branch: None, + }], + }, + ]; + assert!( + collect_int_valued_i64_locals(&stmts).get(&1).is_none(), + "a nested Mul write must reject, not be skipped" + ); + } +} diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 4e217c28fb..ab23649bb1 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -17,6 +17,7 @@ mod hir_facts; mod hot_callees; mod i32_locals; mod index_uses; +mod int_valued_i64_locals; mod int_valued_ta_locals; mod integer_locals; mod local_refs; @@ -62,6 +63,7 @@ pub(crate) use i32_locals::{ collect_integer_let_ids, collect_localset_ids_in_stmts, is_strictly_i32_bounded_expr, is_ushr_zero, }; +pub(crate) use int_valued_i64_locals::ceil_log2_abs; pub(crate) use integer_locals::{ collect_flat_row_aliases, is_int32_producing_expr, static_index_window, }; diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index e94cde7ccc..7cfe31b07c 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -587,7 +587,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { || matches!(**right, Expr::Number(v) if v == 0.0); if matches!(op, BinaryOp::Mod) && crate::type_analysis::is_integer_valued_expr(ctx, left) - && crate::type_analysis::is_integer_valued_expr(ctx, right) + && crate::type_analysis::is_integer_valued_divisor(ctx, right) && !right_is_known_zero { let l_raw = lower_expr(ctx, left)?; diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index dc7190c134..043a74f63e 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -648,6 +648,17 @@ pub(crate) struct FnCtx<'a> { /// (`sum += i % 1000` in a 100M loop) from 1550ms → ~150ms on ARM. pub integer_locals: &'a std::collections::HashSet, + /// LocalIds that are integer-valued within **i64** range but not provably + /// within i32 range, mapped to a conservative `log2(|value|)` bound. + /// + /// `integer_locals` above is an i32-RANGE set — it gates i32 shadow slots, + /// so it must stay narrow. The `%` fast path converts with + /// `fptosi double -> i64` and only needs i64-range integrality, so it + /// additionally consults this map. Sole consumer: + /// `type_analysis::numeric::integer_magnitude_bits`. Populated per function + /// by `collectors::int_valued_i64_locals::collect_int_valued_i64_locals`. + pub int_valued_i64_locals: &'a std::collections::HashMap, + /// LocalIds whose writes are all explicit `>>> 0` u32 casts. These locals /// can use the same i32 bit-pattern slot as signed integer locals for /// bitwise consumers, but ordinary JS reads must convert with `uitofp` so @@ -2219,6 +2230,30 @@ fn lower_numeric_binary_value( return Ok(None); } + // #7404: the same hand-off for a remainder whose operands the integer + // fast path can prove, but whose dividend has no i32 counter slot — the + // `bench_bitwise` shape (`let a = 12345678; … a = a + 1; a % 1000`). + // + // The condition above only recognises an `i32_counter_slots` dividend, so + // an i64-range integer local fell through to the `frem double` below, + // which on AArch64 is a `bl _fmod` libm call. + // + // The DIVISOR is restricted to a non-zero integer literal, exactly like the + // hand-off above. That is not incidental tidiness: `binary::lower`'s own + // Mod gate accepts any `integer_locals` divisor, and `srem(x, 0)` is UB in + // LLVM while JS requires NaN. A decrementing counter that walks through + // zero (`for (let d = 10; d >= 0; d--) … x % d`) IS in `integer_locals`, + // so widening the hand-off to non-literal divisors would newly route it + // into `srem` — main keeps it on `frem` only because this hand-off never + // fires for it. Literal divisors cannot be zero here, so the dividend is + // the only side this widens. + if matches!(op, BinaryOp::Mod) + && matches!(right, Expr::Integer(divisor) if *divisor != 0) + && crate::type_analysis::is_integer_valued_expr(ctx, left) + { + return Ok(None); + } + let Some(left) = lower_numeric_operand_value(ctx, left)? else { return Ok(None); }; diff --git a/crates/perry-codegen/src/type_analysis.rs b/crates/perry-codegen/src/type_analysis.rs index e6c4b85b4a..34840e3ac0 100644 --- a/crates/perry-codegen/src/type_analysis.rs +++ b/crates/perry-codegen/src/type_analysis.rs @@ -32,8 +32,8 @@ mod refine; mod strings; pub(crate) use numeric::{ - expr_produces_canonical_raw_f64, is_bigint_expr, is_bool_expr, is_integer_valued_expr, - is_numeric_expr, is_provably_not_bigint, + expr_produces_canonical_raw_f64, is_bigint_expr, is_bool_expr, is_integer_valued_divisor, + is_integer_valued_expr, is_numeric_expr, is_provably_not_bigint, }; pub(crate) use pod::{ add_operands_have_pod_materialization_hazard, diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index de376cc3e0..ace03d7cab 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -595,33 +595,111 @@ pub(crate) fn is_provably_not_bigint(ctx: &FnCtx<'_>, e: &Expr) -> bool { /// /// Recognizes: /// - `Expr::Integer(_)` — integer literal -/// - `Expr::LocalGet(id)` for locals pre-analyzed as integer-valued by -/// `collectors::collect_integer_locals` (for-loop counters etc.) +/// - `Expr::LocalGet(id)` for locals pre-analyzed as integer-valued, either by +/// `collectors::collect_integer_locals` (i32-range: for-loop counters etc.) +/// or by `collectors::int_valued_i64_locals` (i64-range: literal-initialised +/// locals whose every write is a bounded constant translation) /// - `Expr::Update { .. }` — `i++`/`i--`, whose value is always integer /// if the underlying local is integer-valued /// - `Expr::Binary { Add/Sub/Mul/Mod }` recursively when both operands are /// integer-valued (closed under integer arithmetic; Div is excluded /// because `1 / 2` is 0.5 in JS, not 0) /// - bitwise ops: always integer by JS ToInt32 semantics +/// +/// The result additionally guarantees the value fits `fptosi double -> i64` +/// — see `integer_magnitude_bits`, which this delegates to. `fptosi` is +/// **poison** when the operand is out of the target's range, so proving +/// integrality alone is not enough for the `%` lowering. pub(crate) fn is_integer_valued_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { + integer_magnitude_bits_inner(ctx, e, true).is_some_and(|bits| bits <= MAX_FPTOSI_I64_BITS) +} + +/// The same judgment for the **divisor** of `%`, with the i64-range local set +/// deliberately withheld. +/// +/// `srem(x, 0)` is UB in LLVM while JS requires `NaN`, and the caller's +/// `right_is_known_zero` guard only recognises a literal `0` — a *local* that +/// happens to hold `0` slips past it. The i64-range set admits exactly the +/// counters that walk through zero (`for (let d = 10; d >= 0; d--) … x % d`), +/// so letting it widen the divisor would introduce a UB window that +/// `integer_locals` alone does not open. The dividend has no such constraint. +pub(crate) fn is_integer_valued_divisor(ctx: &FnCtx<'_>, e: &Expr) -> bool { + integer_magnitude_bits_inner(ctx, e, false).is_some_and(|bits| bits <= MAX_FPTOSI_I64_BITS) +} + +/// Largest magnitude (as `log2`) an expression may have and still convert +/// exactly and in-range through `fptosi double -> i64`. `i64::MAX` is +/// `2^63 - 1`, so `2^62` leaves a full bit of headroom. +const MAX_FPTOSI_I64_BITS: u32 = 62; + +/// Conservative upper bound on `log2(|value|)` for a provably integer-valued +/// expression; `None` when the expression is not provably integer-valued. +/// +/// This is a magnitude *lattice*, not just an integrality predicate, because +/// the `%` fast path emits `fptosi double -> i64` and LLVM makes that **poison** +/// for an operand outside i64 range. A plain "is it an integer?" answer lets +/// `(a * b * c) % n` through even when the product needs 93 bits. +/// +/// Leaf bounds: +/// - `integer_locals` — proven i32-range, so `|v| <= 2^31` → 31 bits. +/// - `int_valued_i64_locals` — per-local bound recorded by the collector +/// (56 bits for the common `+-1` step chain; see that module for the +/// IEEE-754 saturation proof that makes it a hard ceiling). +/// - bitwise results — `ToInt32` gives `|v| <= 2^31`; `>>>` is `ToUint32`, +/// so `|v| <= 2^32`. +/// - `Uint8ArrayGet` / `BufferIndexGet` — a byte, `|v| <= 2^8`. +/// +/// Composition mirrors ordinary magnitude arithmetic: `Add`/`Sub` add a bit, +/// `Mul` adds the exponents, and `%` is bounded by the smaller operand +/// (`|a % b| <= min(|a|, |b|)`). +fn integer_magnitude_bits_inner(ctx: &FnCtx<'_>, e: &Expr, allow_i64_locals: bool) -> Option { + let recurse = |sub: &Expr| integer_magnitude_bits_inner(ctx, sub, allow_i64_locals); match e { - Expr::Integer(_) => true, - Expr::Uint8ArrayGet { .. } | Expr::BufferIndexGet { .. } => true, - Expr::LocalGet(id) => ctx.integer_locals.contains(id), - Expr::Update { id, .. } => ctx.integer_locals.contains(id), + Expr::Integer(v) => Some(crate::collectors::ceil_log2_abs(*v)), + // A byte value. + Expr::Uint8ArrayGet { .. } | Expr::BufferIndexGet { .. } => Some(8), + Expr::LocalGet(id) | Expr::Update { id, .. } => { + if ctx.integer_locals.contains(id) { + Some(31) + } else if allow_i64_locals { + ctx.int_valued_i64_locals.get(id).copied() + } else { + None + } + } Expr::Binary { op, left, right } => match op { - BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul | BinaryOp::Mod => { - is_integer_valued_expr(ctx, left) && is_integer_valued_expr(ctx, right) + BinaryOp::Add | BinaryOp::Sub => { + let l = recurse(left)?; + let r = recurse(right)?; + Some(l.max(r).saturating_add(1)) + } + BinaryOp::Mul => { + let l = recurse(left)?; + let r = recurse(right)?; + Some(l.saturating_add(r)) } + // `|a % b| <= min(|a|, |b|)`. Admitted only against a NON-ZERO + // integer literal divisor: `x % 0` is NaN, and `fptosi(NaN)` is + // poison, so a nested `%` by a possibly-zero divisor must not be + // treated as an integer. + BinaryOp::Mod => match right.as_ref() { + Expr::Integer(d) if *d != 0 => { + let l = recurse(left)?; + Some(l.min(crate::collectors::ceil_log2_abs(*d))) + } + _ => None, + }, + // ToInt32 → |v| <= 2^31. BinaryOp::BitAnd | BinaryOp::BitOr | BinaryOp::BitXor | BinaryOp::Shl - | BinaryOp::Shr - | BinaryOp::UShr => true, - _ => false, + | BinaryOp::Shr => Some(31), + // ToUint32 → |v| <= 2^32. + BinaryOp::UShr => Some(32), + _ => None, }, - _ => false, + _ => None, } } diff --git a/crates/perry-codegen/src/type_analysis/numeric/tests.rs b/crates/perry-codegen/src/type_analysis/numeric/tests.rs index 8ff7e2f03c..49131bee4f 100644 --- a/crates/perry-codegen/src/type_analysis/numeric/tests.rs +++ b/crates/perry-codegen/src/type_analysis/numeric/tests.rs @@ -222,3 +222,132 @@ fn dynamic_operand_multiply_keeps_bigint_aware_helper() { multiply routing:\n{ir}" ); } + +// --------------------------------------------------------------------------- +// #7404 — the `%` integer fast path must fire for locals that are +// integer-valued within i64 range but NOT provably i32-range. +// +// These assert on the emitted IR rather than on a predicate, because the whole +// failure mode being fixed was a gate that was live but asking the wrong +// question: a test that only checked "nothing threw" would have passed against +// the broken compiler. +// --------------------------------------------------------------------------- + +fn mod_(left: Expr, right: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Mod, + left: Box::new(left), + right: Box::new(right), + } +} + +fn add(left: Expr, right: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(left), + right: Box::new(right), + } +} + +/// `let a = 12345678; for (…) { acc += a % 1000; a = a + 1; }` +/// +/// `a` is mutated by an unbounded `+ 1` chain, so it is (correctly) NOT in the +/// i32-range `integer_locals` set — before #7404 this fell through to +/// `frem double`, i.e. a `bl _fmod` libm call on AArch64. +#[test] +fn i64_range_local_reaches_the_integer_modulo_fast_path() { + let ir = emitted_ir(probe_module( + "mod_i64_local_unit.ts", + Vec::new(), + vec![ + number_let(1, "acc", true, Expr::Integer(0)), + number_let(2, "a", true, Expr::Integer(12345678)), + Stmt::For { + init: Some(Box::new(number_let(3, "i", true, Expr::Integer(0)))), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(3)), + right: Box::new(Expr::Integer(64)), + }), + update: Some(Expr::Update { + id: 3, + op: UpdateOp::Increment, + prefix: false, + }), + body: vec![ + Stmt::Expr(Expr::LocalSet( + 1, + Box::new(add( + Expr::LocalGet(1), + mod_(Expr::LocalGet(2), Expr::Integer(1000)), + )), + )), + Stmt::Expr(Expr::LocalSet( + 2, + Box::new(add(Expr::LocalGet(2), Expr::Integer(1))), + )), + ], + }, + Stmt::Return(Some(Expr::LocalGet(1))), + ], + )); + assert!( + ir.contains("srem i64"), + "`a % 1000` for an i64-range increment counter must lower to srem, \ + not a frem/fmod libm call:\n{ir}" + ); +} + +/// The dividend may come from the i64-range set; the **divisor** may not. +/// +/// `srem(x, 0)` is UB in LLVM while JS requires NaN, and the lowering's +/// zero guard only recognises a literal `0`. A counter that walks through zero +/// (`d = d - 1`) is exactly what the i64-range set admits, so it must stay on +/// `frem`. +#[test] +fn i64_range_local_is_refused_as_a_modulo_divisor() { + let ir = emitted_ir(probe_module( + "mod_i64_divisor_unit.ts", + Vec::new(), + vec![ + number_let(1, "acc", true, Expr::Integer(0)), + number_let(2, "d", true, Expr::Integer(10)), + Stmt::For { + init: Some(Box::new(number_let(3, "i", true, Expr::Integer(0)))), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(3)), + right: Box::new(Expr::Integer(64)), + }), + update: Some(Expr::Update { + id: 3, + op: UpdateOp::Increment, + prefix: false, + }), + body: vec![ + Stmt::Expr(Expr::LocalSet( + 1, + Box::new(add( + Expr::LocalGet(1), + mod_(Expr::Integer(1000), Expr::LocalGet(2)), + )), + )), + Stmt::Expr(Expr::LocalSet( + 2, + Box::new(Expr::Binary { + op: BinaryOp::Sub, + left: Box::new(Expr::LocalGet(2)), + right: Box::new(Expr::Integer(1)), + }), + )), + ], + }, + Stmt::Return(Some(Expr::LocalGet(1))), + ], + )); + assert!( + !ir.contains("srem i64"), + "a divisor that can walk through zero must NOT reach srem \ + (srem by 0 is UB; JS requires NaN):\n{ir}" + ); +} diff --git a/scripts/gc_evacuation_liveness_assert.py b/scripts/gc_evacuation_liveness_assert.py index 0525534027..5501bf25ef 100755 --- a/scripts/gc_evacuation_liveness_assert.py +++ b/scripts/gc_evacuation_liveness_assert.py @@ -26,13 +26,6 @@ COPIED = re.compile(r"copied_objects=(\d+)") ELIGIBLE = re.compile(r"\[gc-copy-minor\] eligible=(\w+)(?: fallback=(\S+))?") MANUAL = re.compile(r"\[gc-scan-fallback\] site=manual_collect") -# Any collector diagnostic at all. Every one of them is printed behind -# `PERRY_GC_DIAG`, so a trace with none of them was produced by a run that did -# not set it — which is indistinguishable, by counts alone, from a collector -# that moved nothing. Told apart below, because guessing wrong costs a build: -# the `gc-native-roots` in-process arm read as "evacuated NOTHING" on its -# first-ever execution purely for want of the variable. -ANY_DIAG = re.compile(r"^\[gc-[a-z-]+\]", re.MULTILINE) def main() -> int: @@ -51,14 +44,6 @@ def main() -> int: print(f"{args.probe}: evacuation live — {ran} copying minor(s), {copied} objects copied") return 0 - if not ANY_DIAG.search(text): - print(f"::error::{args.probe}: {args.trace} carries no collector diagnostics at all, " - "so this assert measured nothing about the collector. Every line it reads is " - "printed behind PERRY_GC_DIAG; re-run the binary with PERRY_GC_DIAG=1 set " - "alongside PERRY_GC_FORCE_EVACUATE=1. (Diagnostics go to stderr, so this does " - "not disturb an stdout oracle diff.)") - return 1 - print(f"::error::{args.probe}: the forced-evacuation arm evacuated NOTHING " f"({ran} copying minors, {copied} objects copied). The arm is vacuous: " f"it proves the program ran, not that a moving collector did (#7336).")