From 5d0e81ba1779607f0914915ad7a21e5a3844e1a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 10:49:59 +0200 Subject: [PATCH 01/12] perf(repsel): refuse canonical i32 when every hot consumer wants a double (#7128) WIP: profitability model, pending measurement. --- .../perry-codegen/src/collectors/hir_facts.rs | 34 ++ crates/perry-codegen/src/collectors/mod.rs | 1 + .../src/collectors/repsel_benefit.rs | 414 +++++++++++++++++ .../src/collectors/repsel_benefit/tests.rs | 419 ++++++++++++++++++ crates/perry-codegen/src/expr/slot_rep.rs | 55 +++ crates/perry-codegen/src/stmt/let_stmt.rs | 14 + 6 files changed, 937 insertions(+) create mode 100644 crates/perry-codegen/src/collectors/repsel_benefit.rs create mode 100644 crates/perry-codegen/src/collectors/repsel_benefit/tests.rs diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index 7b0f1b8c3d..ecdfb74945 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -55,6 +55,15 @@ pub(crate) struct RepresentationFacts { /// — it never widens the parallel-shadow `needs_i32_slot` gate. See /// `collectors/loop_bounded_i32.rs`. pub loop_bounded_i32_locals: HashSet, + /// Locals whose canonical-i32 promotion is PROVABLE but not PROFITABLE + /// (#7128): written after declaration, no i32-consuming read anywhere in + /// the body, and at least one double-consuming read inside a loop — so the + /// i32 slot only ever converts back, emitting strictly more work than the + /// boxed representation. A refusal set, subtracted from the canonical-i32 + /// eligibility conjunction and from nothing else. See + /// `collectors/repsel_benefit.rs` for the +14.87% `15_mandelbrot` + /// measurement that motivates it. + pub unprofitable_canonical_i32_locals: HashSet, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -167,6 +176,10 @@ impl TypeFacts { &self.representation.loop_bounded_i32_locals } + pub(crate) fn unprofitable_canonical_i32_locals(&self) -> &HashSet { + &self.representation.unprofitable_canonical_i32_locals + } + pub(crate) fn not_bigint_locals(&self) -> &HashSet { &self.representation.not_bigint_locals } @@ -456,6 +469,26 @@ pub(crate) fn collect_type_facts( clamp_fn_ids, strict_int_ta_views, ); + // #7128: the profitability half of canonical-i32 selection. Every term + // above answers "may we?"; this one answers "should we?", and it is + // computed here — after the storage facts it consults — rather than folded + // into any of them, so a future widening of a range proof cannot silently + // move the benefit verdict (and vice versa). Skipped entirely when + // canonical selection is off: that arm selects nothing, so a refusal set + // for it would be dead work. + let unprofitable_canonical_i32_locals = if crate::expr::canonical_i32_locals_enabled() { + super::repsel_benefit::collect_unprofitable_canonical_i32_locals( + stmts, + &super::repsel_benefit::I32StorageFacts { + index_used: &index_used_locals, + strictly_bounded: &strictly_i32_bounded_locals, + unsigned: &unsigned_i32_locals, + int_valued_ta: &int_valued_ta_locals, + }, + ) + } else { + HashSet::new() + }; let known_noalias_buffer_locals = collect_known_noalias_buffer_locals(stmts); let non_escaping_news = super::escape_news::collect_non_escaping_news( stmts, @@ -525,6 +558,7 @@ pub(crate) fn collect_type_facts( not_bigint_locals, int_valued_ta_locals, loop_bounded_i32_locals, + unprofitable_canonical_i32_locals, }, arrays: array_facts, effect: effect_facts, diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index a411133a7e..6ca59f5339 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -30,6 +30,7 @@ mod ptr_shape; mod ptr_shape_report; mod ptr_shape_returns; mod refs; +mod repsel_benefit; mod scalar_method_dispatch; mod scalar_methods; mod shadow_slots; diff --git a/crates/perry-codegen/src/collectors/repsel_benefit.rs b/crates/perry-codegen/src/collectors/repsel_benefit.rs new file mode 100644 index 0000000000..6623f429bb --- /dev/null +++ b/crates/perry-codegen/src/collectors/repsel_benefit.rs @@ -0,0 +1,414 @@ +//! Repsel profitability model (#7128): refuse a canonical-i32 promotion whose +//! every hot consumer wants a double. +//! +//! ## The measurement this exists for +//! +//! #7128 measured the #7106/#7034 coverage campaign on a quiet Raspberry Pi 5 +//! with `perf stat`, at a 0.02% noise floor. canonical-i32 went from 2/18 to +//! 17/18 real workloads and bought: one −1.05% (`11_prime_sieve`), fifteen +//! exact zeros, and **one +14.87% regression** (`15_mandelbrot`), bisected by +//! binary hash to #7121. +//! +//! The mechanism is visible in the emitted AArch64, not inferred. In +//! `15_mandelbrot` the innermost loop is +//! +//! ```text +//! while (x * x + y * y <= 4.0 && iter < MAX_ITER) { …; iter = iter + 1; } +//! … +//! totalIter = totalIter + iter; +//! ``` +//! +//! With `iter` a double the whole loop is **one basic block of 12 +//! instructions**: LLVM fuses the two exit tests into `fcmp` + `fccmp` and +//! branches once. With `iter` in a canonical i32 slot the integer test cannot +//! fuse with the FP test, the loop splits into **two blocks of 14 instructions +//! total** (an extra compare-and-branch plus a register copy for the induction +//! phi), and the accumulator join needs a `ucvtf` it did not need before. Two +//! instructions × 8,011,148 innermost iterations ≈ 16.0M, against a measured +//! +15.63M. That is the whole regression. +//! +//! ## Why this is a profitability rule and not a soundness rule +//! +//! Nothing about the promotion is *wrong*. #7122's monotone loop-induction +//! interval proves `iter ∈ [0, 100]`, which is true. The defect is that the +//! promotion was chosen on **provability** rather than on **benefit**: the +//! Let-site gate is a conjunction of "may we?" terms with no "should we?" term +//! anywhere in it, so any widening of the proof automatically widens the +//! emission. +//! +//! The benefit argument is short. For values inside i32 range, `double` is a +//! *lossless and equal-cost* representation of `+`, `-` and comparison — one +//! instruction either way on every target Perry ships. The i32 representation +//! only *buys* something where the consumer cannot take a double at all +//! without a conversion: +//! +//! * an array / typed-array **index** (`a[v]`), +//! * a **bitwise** operand (`&`, `|`, `^`, `<<`, `>>`, `>>>`, `~`), +//! * `Math.imul`. +//! +//! Everywhere else it is at best neutral — and it becomes a *cost* the moment +//! a consumer needs the double back, because that consumer now emits a +//! `sitofp`/`uitofp` the boxed representation never needed. +//! +//! So the rule is: **a local that is written after its declaration, has no +//! i32-consuming read anywhere, and has at least one double-consuming read +//! inside a loop, does not select canonical i32.** +//! +//! Each of the three conjuncts earns its place: +//! +//! * *written after declaration* — a single-assignment local is loop-invariant +//! at every read, so any conversion is hoisted out of the loop by LICM and +//! costs O(1), not O(iterations). `const WIDTH = 800` in `15_mandelbrot` is +//! read as a double twice per inner iteration and still costs nothing, +//! because the value is a constant. Judging it would be pure census churn. +//! * *no i32-consuming read* — one array index is enough to make the +//! representation pay for itself; `11_prime_sieve`'s counters are all +//! index-used, which is why its −1.05% survives this rule untouched. +//! * *a double-consuming read inside a loop* — a conversion outside every loop +//! runs once. `return iter` after the loop is not a reason to refuse. +//! +//! ## Deliberate under-approximation +//! +//! This analysis can only ever **remove** selections, so every uncertainty is +//! resolved toward "not a cost": an expression form this module does not model +//! contributes `Neutral`, never `Double`. Two consequences worth naming: +//! +//! * **Comparison is always neutral**, on both sides, even against a +//! non-integer operand. `for (let i = 0; i < n; i++)` with a `number` +//! parameter `n` is the single most common loop in JavaScript; classifying +//! its guard as a double consumer would refuse nearly every counter in the +//! corpus to buy nothing measurable. +//! * **A write of a local into its own slot is neutral** (`iter = iter + 1`), +//! because the value never leaves the representation it arrived in. +//! +//! A missed refusal is today's behaviour. A spurious refusal is a lost +//! promotion, so the model is built to err in the first direction. +//! +//! ## Scope +//! +//! Consumed only by the canonical-i32 gate (`canonical_i32_value_eligible` in +//! `stmt/let_stmt.rs`), never by the parallel-shadow `needs_i32_slot` gate — so +//! `PERRY_CANONICAL_I32_LOCALS=0` still reproduces the pre-phase model +//! bit-for-bit, the same containment `int_valued_ta_locals` and +//! `loop_bounded_i32_locals` use. canonical `Str` and `Ptr<*>` are untouched; +//! they have the same disease (#7128 findings C and D) and are the next +//! consumers this model is shaped for, but widening it to them is a separate +//! measurement. + +use std::collections::HashSet; + +use perry_hir::{BinaryOp, Expr, Stmt, UnaryOp}; + +/// What the *consumer* of a read wants the value to be. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum UseCtx { + /// The consumer takes an i32 operand directly: an index, a bitwise + /// operand, `Math.imul`, or a write into another local that already has + /// i32 storage. This is what canonical i32 buys. + Int, + /// The consumer needs an f64: a call/`new` argument (Perry's ABI passes + /// NaN-boxed doubles), a `/` operand, a `return`, or a write into a local + /// whose storage is a boxed double. This is what canonical i32 costs. + Double, + /// Equal cost in both representations, or a form this model does not + /// claim to understand. Contributes to neither side. + Neutral, +} + +/// The locals that already hold i32 storage under the *parallel-shadow* model, +/// i.e. independently of any canonical selection. Writing into one of these is +/// an i32-consuming use; writing into anything else is a double-consuming use. +pub(crate) struct I32StorageFacts<'a> { + pub index_used: &'a HashSet, + pub strictly_bounded: &'a HashSet, + pub unsigned: &'a HashSet, + pub int_valued_ta: &'a HashSet, +} + +impl I32StorageFacts<'_> { + fn holds_i32(&self, id: u32) -> bool { + self.index_used.contains(&id) + || self.strictly_bounded.contains(&id) + || self.unsigned.contains(&id) + || self.int_valued_ta.contains(&id) + } +} + +#[derive(Default, Clone, Copy)] +struct Tally { + declared: bool, + writes_after_decl: u32, + int_reads: u32, + /// Double-consuming reads at loop depth ≥ 1 only. A conversion outside + /// every loop runs once and is not a reason to refuse a representation. + hot_double_reads: u32, +} + +impl Tally { + fn unprofitable(&self) -> bool { + self.declared + && self.writes_after_decl >= 1 + && self.int_reads == 0 + && self.hot_double_reads >= 1 + } +} + +struct Model<'a> { + storage: &'a I32StorageFacts<'a>, + tallies: std::collections::HashMap, + /// The local currently being written, if the walk is inside the RHS of + /// `LocalSet(t, …)`. A read of `t` there is representation-preserving. + self_target: Option, +} + +impl<'a> Model<'a> { + fn entry(&mut self, id: u32) -> &mut Tally { + self.tallies.entry(id).or_default() + } + + fn read(&mut self, id: u32, ctx: UseCtx, depth: u32) { + if self.self_target == Some(id) { + return; + } + match ctx { + UseCtx::Int => self.entry(id).int_reads += 1, + UseCtx::Double if depth >= 1 => self.entry(id).hot_double_reads += 1, + _ => {} + } + } + + /// The context a value acquires by being stored into `target`. + fn target_ctx(&self, target: u32) -> UseCtx { + if self.storage.holds_i32(target) { + UseCtx::Int + } else { + UseCtx::Double + } + } + + fn expr(&mut self, e: &Expr, ctx: UseCtx, depth: u32) { + match e { + Expr::LocalGet(id) => self.read(*id, ctx, depth), + Expr::LocalSet(id, value) => { + self.entry(*id).writes_after_decl += 1; + let target_ctx = self.target_ctx(*id); + let saved = self.self_target.replace(*id); + self.expr(value, target_ctx, depth); + self.self_target = saved; + } + Expr::Update { id, .. } => { + self.entry(*id).writes_after_decl += 1; + } + // An index is the canonical i32-consuming position: with a boxed + // local this is exactly the `fptosi` per access the representation + // exists to delete. + Expr::IndexGet { object, index } => { + self.expr(object, UseCtx::Neutral, depth); + self.expr(index, UseCtx::Int, depth); + } + Expr::IndexSet { + object, + index, + value, + } => { + self.expr(object, UseCtx::Neutral, depth); + self.expr(index, UseCtx::Int, depth); + // The element's own representation depends on the array kind, + // which this model does not track: neutral, not a cost. + self.expr(value, UseCtx::Neutral, depth); + } + Expr::IndexUpdate { object, index, .. } => { + self.expr(object, UseCtx::Neutral, depth); + self.expr(index, UseCtx::Int, depth); + } + Expr::Binary { op, left, right } => { + let child = match op { + // Bitwise operands are ToInt32-coerced by the language, so + // an i32 slot feeds them with no conversion at all. + BinaryOp::BitAnd + | BinaryOp::BitOr + | BinaryOp::BitXor + | BinaryOp::Shl + | BinaryOp::Shr + | BinaryOp::UShr => UseCtx::Int, + // `/` and `**` are floating-point in JS regardless of the + // operands, so an i32 operand must be converted. + BinaryOp::Div | BinaryOp::Pow => UseCtx::Double, + // Additive / multiplicative chains inherit their root: + // `a[i * 4 + k]` keeps `i` and `k` integral, `sum + i * 2` + // makes them doubles. + BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul | BinaryOp::Mod => ctx, + }; + self.expr(left, child, depth); + self.expr(right, child, depth); + } + Expr::Unary { op, operand } => { + let child = match op { + UnaryOp::BitNot => UseCtx::Int, + UnaryOp::Neg | UnaryOp::Pos => ctx, + UnaryOp::Not => UseCtx::Neutral, + }; + self.expr(operand, child, depth); + } + // A comparison costs one instruction in either representation; see + // the module header on why this is neutral rather than a cost. + Expr::Compare { left, right, .. } => { + self.expr(left, UseCtx::Neutral, depth); + self.expr(right, UseCtx::Neutral, depth); + } + Expr::MathImul(a, b) => { + self.expr(a, UseCtx::Int, depth); + self.expr(b, UseCtx::Int, depth); + } + // Perry's calling convention passes NaN-boxed doubles, so every + // argument position converts an i32 operand back. + Expr::Call { callee, args, .. } => { + self.expr(callee, UseCtx::Neutral, depth); + for a in args { + self.expr(a, UseCtx::Double, depth); + } + } + Expr::New { args, .. } => { + for a in args { + self.expr(a, UseCtx::Double, depth); + } + } + Expr::Conditional { + condition, + then_expr, + else_expr, + } => { + self.expr(condition, UseCtx::Neutral, depth); + self.expr(then_expr, ctx, depth); + self.expr(else_expr, ctx, depth); + } + // Closure bodies are not walked: closure-referenced locals are + // excluded from canonical selection wholesale by + // `collect_closure_referenced_locals`, so nothing inside one can + // change a verdict. + Expr::Closure { .. } => {} + // Everything else: recurse neutrally. Under-approximating the cost + // is the safe direction for a rule that can only refuse. + _ => { + let saved = self.self_target.take(); + perry_hir::walker::walk_expr_children(e, &mut |child| { + self.expr(child, UseCtx::Neutral, depth) + }); + self.self_target = saved; + } + } + } + + fn stmts(&mut self, stmts: &[Stmt], depth: u32) { + for s in stmts { + self.stmt(s, depth); + } + } + + fn stmt(&mut self, s: &Stmt, depth: u32) { + match s { + Stmt::Let { id, init, .. } => { + self.entry(*id).declared = true; + if let Some(e) = init { + let ctx = self.target_ctx(*id); + self.expr(e, ctx, depth); + } + } + Stmt::Expr(e) => self.expr(e, UseCtx::Neutral, depth), + Stmt::Return(Some(e)) | Stmt::Throw(e) => self.expr(e, UseCtx::Double, depth), + Stmt::If { + condition, + then_branch, + else_branch, + } => { + self.expr(condition, UseCtx::Neutral, depth); + self.stmts(then_branch, depth); + if let Some(eb) = else_branch { + self.stmts(eb, depth); + } + } + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + self.expr(condition, UseCtx::Neutral, depth + 1); + self.stmts(body, depth + 1); + } + Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(i) = init { + self.stmt(i, depth); + } + if let Some(c) = condition { + self.expr(c, UseCtx::Neutral, depth + 1); + } + if let Some(u) = update { + self.expr(u, UseCtx::Neutral, depth + 1); + } + self.stmts(body, depth + 1); + } + Stmt::Labeled { body, .. } => self.stmt(body, depth), + Stmt::Try { + body, + catch, + finally, + } => { + self.stmts(body, depth); + if let Some(c) = catch { + self.stmts(&c.body, depth); + } + if let Some(f) = finally { + self.stmts(f, depth); + } + } + Stmt::Switch { + discriminant, + cases, + } => { + self.expr(discriminant, UseCtx::Neutral, depth); + for c in cases { + if let Some(t) = &c.test { + self.expr(t, UseCtx::Neutral, depth); + } + self.stmts(&c.body, depth); + } + } + Stmt::Return(None) + | Stmt::Break + | Stmt::Continue + | Stmt::LabeledBreak(_) + | Stmt::LabeledContinue(_) + | Stmt::PreallocateBoxes(_) + | Stmt::PreallocateTdzBoxes(_) => {} + } + } +} + +/// Locals whose canonical-i32 promotion would emit strictly more work than the +/// boxed representation: written after declaration, no i32-consuming read +/// anywhere in the body, and at least one double-consuming read inside a loop. +/// +/// The result is a **refusal** set — subtracted from the Let-site eligibility +/// conjunction, never added to it. +pub(crate) fn collect_unprofitable_canonical_i32_locals( + stmts: &[Stmt], + storage: &I32StorageFacts<'_>, +) -> HashSet { + let mut model = Model { + storage, + tallies: std::collections::HashMap::new(), + self_target: None, + }; + model.stmts(stmts, 0); + model + .tallies + .into_iter() + .filter(|(_, t)| t.unprofitable()) + .map(|(id, _)| id) + .collect() +} + +#[cfg(test)] +#[path = "repsel_benefit/tests.rs"] +mod tests; diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs new file mode 100644 index 0000000000..02ac55ab32 --- /dev/null +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -0,0 +1,419 @@ +//! Unit tests for the repsel profitability model (#7128). +//! +//! The load-bearing assertion is `mandelbrot_iter_is_refused`: it reproduces +//! the exact HIR shape that measured **+14.87% instructions retired** on a +//! quiet Raspberry Pi 5, and it fails against a compiler without this module. +//! Every other test exists to pin the rule's *boundaries*, because a refusal +//! rule that over-fires is a silent coverage loss with no symptom — so each +//! conjunct of the rule has a test that removing the conjunct turns red. + +use super::*; +use perry_hir::types::Type as HirType; +use perry_hir::{CompareOp, LogicalOp, UpdateOp}; + +fn let_mut(id: u32, init: Option) -> Stmt { + Stmt::Let { + id, + name: format!("v{id}"), + ty: HirType::Number, + mutable: true, + init, + } +} + +fn get(id: u32) -> Expr { + Expr::LocalGet(id) +} + +fn set(id: u32, rhs: Expr) -> Stmt { + Stmt::Expr(Expr::LocalSet(id, Box::new(rhs))) +} + +fn inc(id: u32) -> Expr { + Expr::Update { + id, + op: UpdateOp::Increment, + prefix: false, + } +} + +fn bin(op: BinaryOp, l: Expr, r: Expr) -> Expr { + Expr::Binary { + op, + left: Box::new(l), + right: Box::new(r), + } +} + +fn cmp(l: Expr, r: Expr) -> Expr { + Expr::Compare { + op: CompareOp::Lt, + left: Box::new(l), + right: Box::new(r), + } +} + +fn index(object: Expr, idx: Expr) -> Expr { + Expr::IndexGet { + object: Box::new(object), + index: Box::new(idx), + } +} + +fn call(args: Vec) -> Expr { + Expr::Call { + callee: Box::new(Expr::FuncRef(0)), + args, + type_args: vec![], + byte_offset: 0, + } +} + +fn for_loop(init: Stmt, condition: Expr, update: Expr, body: Vec) -> Stmt { + Stmt::For { + init: Some(Box::new(init)), + condition: Some(condition), + update: Some(update), + body, + } +} + +/// No local anywhere holds parallel-shadow i32 storage — the state every +/// #7110-admitted counter is in, and the one that makes the model's verdict +/// the deciding one. +fn no_i32_storage() -> (HashSet, HashSet, HashSet, HashSet) { + ( + HashSet::new(), + HashSet::new(), + HashSet::new(), + HashSet::new(), + ) +} + +fn run(stmts: &[Stmt], index_used: &HashSet) -> HashSet { + let (_, strictly_bounded, unsigned, int_valued_ta) = no_i32_storage(); + collect_unprofitable_canonical_i32_locals( + stmts, + &I32StorageFacts { + index_used, + strictly_bounded: &strictly_bounded, + unsigned: &unsigned, + int_valued_ta: &int_valued_ta, + }, + ) +} + +/// ```text +/// let totalIter = 0; +/// for (let py = 0; py < 800; py++) { +/// for (let px = 0; px < 800; px++) { +/// const cx = (px - 400.0) * 4.0; +/// let iter = 0; +/// while (fp && iter < 100) { iter = iter + 1; } +/// totalIter = totalIter + iter; +/// } +/// } +/// ``` +/// +/// `benchmarks/suite/15_mandelbrot.ts`, reduced. `iter`, `px` and `py` are all +/// proven-integer, all admitted by #7110's interval proof, and all three are +/// consumed only as doubles inside a loop. This is the +14.87% regression. +#[test] +fn mandelbrot_iter_is_refused() { + // 1 = totalIter, 2 = py, 3 = px, 4 = cx, 5 = iter + let inner_while = Stmt::While { + condition: Expr::Logical { + op: LogicalOp::And, + left: Box::new(cmp(Expr::Number(1.0), Expr::Number(4.0))), + right: Box::new(cmp(get(5), Expr::Integer(100))), + }, + body: vec![set(5, bin(BinaryOp::Add, get(5), Expr::Integer(1)))], + }; + let px_body = vec![ + // const cx = (px - 400.0) * 4.0 → px read in a double chain + Stmt::Let { + id: 4, + name: "cx".into(), + ty: HirType::Number, + mutable: false, + init: Some(bin( + BinaryOp::Mul, + bin(BinaryOp::Sub, get(3), Expr::Number(400.0)), + Expr::Number(4.0), + )), + }, + let_mut(5, Some(Expr::Integer(0))), + inner_while, + // totalIter = totalIter + iter → iter read into a boxed accumulator + set(1, bin(BinaryOp::Add, get(1), get(5))), + ]; + let stmts = vec![ + let_mut(1, Some(Expr::Integer(0))), + for_loop( + let_mut(2, Some(Expr::Integer(0))), + cmp(get(2), Expr::Integer(800)), + inc(2), + vec![for_loop( + let_mut(3, Some(Expr::Integer(0))), + cmp(get(3), Expr::Integer(800)), + inc(3), + px_body, + )], + ), + ]; + + let out = run(&stmts, &HashSet::new()); + assert!(out.contains(&5), "iter must be refused: {out:?}"); + assert!(out.contains(&3), "px must be refused: {out:?}"); + assert!(out.contains(&2), "py must be refused: {out:?}"); +} + +/// `benchmarks/suite/11_prime_sieve.ts`, reduced — the −1.05% win. One array +/// index is enough to pay for the representation, so the counter must survive +/// even though `count = count + 1` never reads it. +#[test] +fn index_used_counter_survives() { + // 1 = sieve, 2 = i + let stmts = vec![ + let_mut(1, None), + for_loop( + let_mut(2, Some(Expr::Integer(0))), + cmp(get(2), Expr::Integer(1_000_000)), + inc(2), + vec![Stmt::Expr(Expr::IndexSet { + object: Box::new(get(1)), + index: Box::new(get(2)), + value: Box::new(Expr::Bool(false)), + })], + ), + ]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&2), "index-used counter refused: {out:?}"); +} + +/// A counter whose only in-loop double consumer sits *behind* an index is +/// still profitable: `sum = sum + arr[i]` reads `i` in an index position. +#[test] +fn index_behind_a_boxed_accumulator_survives() { + // 1 = arr, 2 = sum, 3 = i + let stmts = vec![ + let_mut(1, None), + let_mut(2, Some(Expr::Integer(0))), + for_loop( + let_mut(3, Some(Expr::Integer(0))), + cmp(get(3), Expr::Integer(100)), + inc(3), + vec![set(2, bin(BinaryOp::Add, get(2), index(get(1), get(3))))], + ), + ]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&3), "indexed counter refused: {out:?}"); +} + +/// `benchmarks/suite/02_loop_overhead.ts` and `08_string_concat.ts`: a counter +/// whose every read is a guard. Nothing converts, so nothing is refused — +/// this is the shape #7110 exists for and the one the −4.12% `Str` workload +/// carries. +#[test] +fn bare_guard_only_counter_survives() { + let stmts = vec![for_loop( + let_mut(1, Some(Expr::Integer(0))), + cmp(get(1), Expr::Integer(100_000)), + inc(1), + vec![], + )]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "guard-only counter refused: {out:?}"); +} + +/// `for (let i = 0; i < n; i++)` with a `number` parameter bound. The guard is +/// neutral on BOTH sides by construction; classifying it as a double consumer +/// would refuse the most common loop in JavaScript. +#[test] +fn guard_against_a_non_integer_bound_is_not_a_cost() { + // 9 = a parameter (never declared, never in any integer set) + let stmts = vec![for_loop( + let_mut(1, Some(Expr::Integer(0))), + cmp(get(1), get(9)), + inc(1), + vec![], + )]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "param-bounded counter refused: {out:?}"); +} + +/// `iter = iter + 1` writes the value back into its own slot, which is +/// representation-preserving. Without the self-target exemption every `while` +/// counter in the corpus would be refused — including +/// `fixture_loop_bounded_i32.ts`'s `iterate()`, whose census floor is the +/// liveness proof for #7110 itself. +#[test] +fn self_step_is_not_a_cost() { + let stmts = vec![ + let_mut(1, Some(Expr::Integer(0))), + Stmt::While { + condition: cmp(get(1), Expr::Integer(100)), + body: vec![set(1, bin(BinaryOp::Add, get(1), Expr::Integer(1)))], + }, + ]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "self-stepping counter refused: {out:?}"); +} + +/// A conversion outside every loop runs once. `return iter` (and the +/// `console.log` at the end of every benchmark) must not refuse a counter — +/// `fixture_loop_bounded_i32.ts`'s `iterate()` returns its counter. +#[test] +fn double_use_outside_a_loop_is_not_a_cost() { + let stmts = vec![ + let_mut(1, Some(Expr::Integer(0))), + Stmt::While { + condition: cmp(get(1), Expr::Integer(100)), + body: vec![Stmt::Expr(inc(1))], + }, + Stmt::Return(Some(get(1))), + ]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "post-loop return refused: {out:?}"); +} + +/// A single-assignment local is loop-invariant at every read, so LICM hoists +/// any conversion out of the loop. `const WIDTH = 800` in `15_mandelbrot` is +/// read as a double twice per inner iteration and still costs nothing. +#[test] +fn write_once_local_is_out_of_scope() { + let stmts = vec![ + Stmt::Let { + id: 1, + name: "WIDTH".into(), + ty: HirType::Number, + mutable: false, + init: Some(Expr::Integer(800)), + }, + for_loop( + let_mut(2, Some(Expr::Integer(0))), + cmp(get(2), Expr::Integer(800)), + inc(2), + vec![Stmt::Expr(bin(BinaryOp::Div, get(1), Expr::Number(2.0)))], + ), + ]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "write-once const refused: {out:?}"); +} + +/// `benchmarks/suite/14_closure.ts` / `07_object_create.ts`: Perry's calling +/// convention passes NaN-boxed doubles, so an argument position converts. +#[test] +fn call_argument_is_a_cost() { + let stmts = vec![for_loop( + let_mut(1, Some(Expr::Integer(0))), + cmp(get(1), Expr::Integer(100)), + inc(1), + vec![Stmt::Expr(call(vec![get(1)]))], + )]; + let out = run(&stmts, &HashSet::new()); + assert!(out.contains(&1), "call-argument counter kept: {out:?}"); +} + +/// `benchmarks/suite/06_math_intensive.ts`: `result = result + (1.0 / i)`. +/// `/` is floating-point in JS whatever its operands are. +#[test] +fn float_divide_operand_is_a_cost() { + let stmts = vec![ + let_mut(1, Some(Expr::Number(1.0))), + for_loop( + let_mut(2, Some(Expr::Integer(1))), + cmp(get(2), Expr::Integer(50_000_000)), + inc(2), + vec![set( + 1, + bin( + BinaryOp::Add, + get(1), + bin(BinaryOp::Div, Expr::Number(1.0), get(2)), + ), + )], + ), + ]; + let out = run(&stmts, &HashSet::new()); + assert!(out.contains(&2), "fdiv operand kept: {out:?}"); +} + +/// Bitwise operands are ToInt32-coerced by the language, so an i32 slot feeds +/// them with no conversion — `17_loop_data_dependent`'s `x[i & 63]` shape and +/// every FNV/xorshift mixer in the corpus. +#[test] +fn bitwise_use_is_a_benefit() { + let stmts = vec![ + let_mut(1, Some(Expr::Integer(0))), + for_loop( + let_mut(2, Some(Expr::Integer(0))), + cmp(get(2), Expr::Integer(100)), + inc(2), + vec![ + set(1, bin(BinaryOp::BitXor, get(1), get(2))), + Stmt::Expr(call(vec![get(2)])), + ], + ), + ]; + let out = run(&stmts, &HashSet::new()); + assert!( + !out.contains(&2), + "bitwise-consumed counter refused: {out:?}" + ); +} + +/// Writing into a local that already holds parallel-shadow i32 storage is an +/// i32-consuming use: `j = j + i` in `11_prime_sieve` keeps `i` profitable. +#[test] +fn write_into_an_i32_storage_local_is_a_benefit() { + let index_used: HashSet = [2u32].into_iter().collect(); + let stmts = vec![ + let_mut(2, Some(Expr::Integer(0))), + for_loop( + let_mut(1, Some(Expr::Integer(0))), + cmp(get(1), Expr::Integer(100)), + inc(1), + vec![ + set(2, bin(BinaryOp::Add, get(2), get(1))), + Stmt::Expr(call(vec![get(1)])), + ], + ), + ]; + let out = run(&stmts, &index_used); + assert!( + !out.contains(&1), + "i32-store-consumed counter refused: {out:?}" + ); +} + +/// `Math.imul` is the one call form that takes i32 operands directly. +#[test] +fn math_imul_operand_is_a_benefit() { + let stmts = vec![for_loop( + let_mut(1, Some(Expr::Integer(0))), + cmp(get(1), Expr::Integer(100)), + inc(1), + vec![ + Stmt::Expr(Expr::MathImul(Box::new(get(1)), Box::new(Expr::Integer(3)))), + Stmt::Expr(call(vec![get(1)])), + ], + )]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "Math.imul operand refused: {out:?}"); +} + +/// An expression form the model does not understand contributes neither side. +/// A refusal rule must under-approximate the cost: a missed refusal is today's +/// behaviour, a spurious one is a lost promotion. +#[test] +fn unmodelled_forms_are_neutral() { + let stmts = vec![for_loop( + let_mut(1, Some(Expr::Integer(0))), + cmp(get(1), Expr::Integer(100)), + inc(1), + vec![Stmt::Expr(Expr::TypeOf(Box::new(get(1))))], + )]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "typeof-read counter refused: {out:?}"); +} diff --git a/crates/perry-codegen/src/expr/slot_rep.rs b/crates/perry-codegen/src/expr/slot_rep.rs index d5dbe8c7cb..bc49c70fc4 100644 --- a/crates/perry-codegen/src/expr/slot_rep.rs +++ b/crates/perry-codegen/src/expr/slot_rep.rs @@ -306,6 +306,10 @@ pub(crate) struct CanonicalI32Denial { pub closure_referenced: bool, pub array_row_alias: bool, pub not_index_used_or_bounded: bool, + /// #7128: every range proof passed, and the promotion would still emit + /// more work than the box. Distinct from every other field here, which + /// records a *provability* failure. + pub no_i32_consuming_use: bool, pub context: Option<&'static str>, } @@ -388,6 +392,21 @@ impl CanonicalI32Denial { Some(NOT_BOUNDED_ISSUE), )); } + if self.no_i32_consuming_use { + return Some(( + "no_i32_consuming_use", + "provable, but not profitable: the local is written after its \ + declaration, no read of it anywhere is consumed as an i32 (no \ + array index, no bitwise operand, no `Math.imul`), and at least \ + one read inside a loop needs the double back. The i32 slot \ + would emit a `sitofp` per iteration and buy nothing — this is \ + the +14.87% instructions #7128 measured on 15_mandelbrot, \ + where the mixed representation also costs the loop its fused \ + single-block exit test", + Tier::CompilerLimitation, + Some(NO_BENEFIT_ISSUE), + )); + } // Everything value-level passed; only the context gate is left. self.context.map(|rule| { let (reason, issue) = context_rule_text(rule); @@ -423,6 +442,9 @@ pub(crate) fn deny_canonical_i32(ctx: &FnCtx<'_>, id: u32, name: &str, denial: C const MODULE_GLOBAL_ISSUE: &str = "#7109"; /// Tracking issue for the index-use / i32-bound precondition. const NOT_BOUNDED_ISSUE: &str = "#7110"; +/// Tracking issue for the profitability refusal — the one denial in this list +/// that is not a failed proof. +const NO_BENEFIT_ISSUE: &str = "#7128"; /// `(reason, issue)` for a context-level denial rule. fn context_rule_text(rule: &str) -> (&'static str, &'static str) { @@ -905,6 +927,7 @@ mod repsel_denial_tests { closure_referenced: false, array_row_alias: false, not_index_used_or_bounded: false, + no_i32_consuming_use: false, context: None, } } @@ -966,6 +989,7 @@ mod repsel_denial_tests { closure_referenced: true, array_row_alias: true, not_index_used_or_bounded: true, + no_i32_consuming_use: true, context: Some(MODULE_INIT_CONTEXT), }; assert_eq!(all.verdict().map(|v| v.0), Some("declared_bigint")); @@ -977,6 +1001,7 @@ mod repsel_denial_tests { ("module_global", "closure_referenced"), ("closure_referenced", "array_row_alias"), ("array_row_alias", "not_index_used_or_bounded"), + ("not_index_used_or_bounded", "no_i32_consuming_use"), ]; let mut d = all; for (named, next) in order { @@ -988,12 +1013,42 @@ mod repsel_denial_tests { "module_global" => d.module_global = false, "closure_referenced" => d.closure_referenced = false, "array_row_alias" => d.array_row_alias = false, + "not_index_used_or_bounded" => d.not_index_used_or_bounded = false, _ => unreachable!(), } assert_eq!(d.verdict().map(|v| v.0), Some(next)); } } + /// #7128: a local that passed every range proof and lost only to the + /// profitability model names that, and not the context gate — otherwise + /// `15_mandelbrot`'s `iter` would report `module_init_context`, which is + /// exactly the rule #7121 removed, sending the next reader back to a bug + /// that is already fixed. + #[test] + fn the_profitability_refusal_outranks_the_context_rule() { + let d = CanonicalI32Denial { + no_i32_consuming_use: true, + context: Some(MODULE_INIT_CONTEXT), + ..passing() + }; + let (rule, _, _, issue) = d.verdict().expect("a profitability denial"); + assert_eq!(rule, "no_i32_consuming_use"); + assert_eq!(issue, Some("#7128")); + } + + /// …but a failed PROOF still outranks a failed benefit check: "we cannot" + /// is more fundamental, and more actionable, than "we should not". + #[test] + fn a_failed_range_proof_outranks_the_profitability_refusal() { + let d = CanonicalI32Denial { + not_index_used_or_bounded: true, + no_i32_consuming_use: true, + ..passing() + }; + assert_eq!(d.verdict().map(|v| v.0), Some("not_index_used_or_bounded")); + } + /// Body-context reasons map onto stable rule names; a permitting context /// yields `None` so no denial is recorded for an ordinary sync body. #[test] diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index ff6a1ae6ba..2b47108c0c 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -1275,11 +1275,24 @@ pub(crate) fn lower_let( let canonical_safe_local = i32_safe_local || ctx.native_facts.int_valued_ta_locals().contains(&id) || ctx.native_facts.loop_bounded_i32_locals().contains(&id); + // And one PROFITABILITY term, which is not a safety term at all (#7128). + // Every rule above answers "may we?"; this one answers "should we?". A + // local written after its declaration, with no i32-consuming read anywhere + // and at least one double-consuming read inside a loop, pays a + // `sitofp`/`uitofp` per iteration and buys nothing back — measured at + // +14.87% instructions retired on `benchmarks/suite/15_mandelbrot.ts`, + // where the mixed representation additionally costs the loop its + // single-basic-block `fcmp`/`fccmp` exit. See `collectors/repsel_benefit.rs`. + let unprofitable = ctx + .native_facts + .unprofitable_canonical_i32_locals() + .contains(&id); // Split into the VALUE-level proof and the CONTEXT gate so a context-level // exclusion can be reported (#7106). `canonical_i32` is the conjunction, so // selection behaviour is unchanged. let canonical_i32_value_eligible = (ctx.integer_locals.contains(&id) || is_unsigned_i32_local) && canonical_safe_local + && !unprofitable && init_in_i32_range && !matches!(refined_ty, perry_hir::types::Type::BigInt) && !ctx.boxed_vars.contains(&id) @@ -1321,6 +1334,7 @@ pub(crate) fn lower_let( closure_referenced: ctx.repsel_closure_ref_locals.contains(&id), array_row_alias: ctx.array_row_aliases.contains_key(&id), not_index_used_or_bounded: !canonical_safe_local, + no_i32_consuming_use: unprofitable, context: ctx.repsel_context_denial, }, ); From 487b1f6911494ab234430911deb44fa594f31e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 11:00:08 +0200 Subject: [PATCH 02/12] test(repsel): gate the profitability refusal, and give it a fixture case --- .../fixtures/fixture_loop_bounded_i32.ts | 46 +++++++- .../compiler_output_harness/repsel_census.py | 105 ++++++++++++++++++ tests/test_repsel_census.py | 77 +++++++++++++ 3 files changed, 222 insertions(+), 6 deletions(-) diff --git a/benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts b/benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts index ea61a8f77c..b3ffd613dc 100644 --- a/benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts +++ b/benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts @@ -9,9 +9,11 @@ // `collect_loop_bounded_i32_locals` returns the empty set this file's // canonical-i32 count is zero and the census goes red. // -// The two locals it must NOT promote are here on purpose: an unadmitted -// counter and an unbounded accumulator keep the fixture from being satisfied by -// any rule that simply says yes to proven-integer locals. +// The three locals it must NOT promote are here on purpose: an unadmitted +// counter, an unbounded accumulator, and (since #7128) a counter that is +// perfectly provable and not worth promoting. Together they keep the fixture +// from being satisfied by any rule that simply says yes to proven-integer +// locals. // // Requirements shared with the other canonical-slot fixtures: plain synchronous // function bodies (async/generator bodies are context-excluded), and no closure @@ -31,8 +33,11 @@ function countUp(): number { } // PROMOTES. A `while` whose guard is a CONJUNCTION and whose step is a -// `LocalSet` Add rather than `++` — the 15_mandelbrot `iter` shape. -// Interval [0, 100]. +// `LocalSet` Add rather than `++`. Interval [0, 100]. Nothing inside the loop +// reads `iter` at all, so the representation converts nowhere and the counter +// is free to take the i32 slot; the one `return iter` runs once, outside the +// loop. Compare `mixedWithFloat` below, which is the same proof and the +// opposite verdict. function iterate(seed: number): number { let iter = 0; let x = seed; @@ -67,6 +72,33 @@ function accumulate(): number { return sum; } +// DOES NOT PROMOTE — and this is the only entry here whose PROOF succeeds. +// `hit` is admitted by exactly the same interval argument as `iterate`'s +// counter above: single literal init, one guarded step, interval [0, 100]. +// What differs is the consumer. `weight` is an unbounded accumulator, so it +// keeps a boxed double slot, and `weight = weight + hit` reads `hit` back as a +// double once per iteration. An i32 slot for `hit` would emit a `sitofp` per +// iteration and buy nothing — `hit` is never an array index, never a bitwise +// operand, never a `Math.imul` argument. +// +// `benchmarks/suite/15_mandelbrot.ts` is exactly this shape (`totalIter = +// totalIter + iter` around a `while (fp && iter < MAX_ITER)`), and promoting +// it cost **+14.87% instructions retired** — measured on a quiet Raspberry Pi +// 5 at a 0.02% noise floor (#7128). The refusal is gated by REFUSAL_FLOORS in +// scripts/compiler_output_harness/repsel_census.py; deleting +// collectors/repsel_benefit.rs takes that check red. +function mixedWithFloat(seed: number): number { + let weight = 0.0; + let hit = 0; + let x = seed; + while (x < 1000.0 && hit < 100) { + x = x * 1.5; + hit = hit + 1; + weight = weight + hit; + } + return weight; +} + console.log( "loopBounded:" + countUp() + @@ -75,5 +107,7 @@ console.log( ":" + overshoot() + ":" + - accumulate(), + accumulate() + + ":" + + mixedWithFloat(1.0), ); diff --git a/scripts/compiler_output_harness/repsel_census.py b/scripts/compiler_output_harness/repsel_census.py index 2ae84b3db6..33d3005cb6 100644 --- a/scripts/compiler_output_harness/repsel_census.py +++ b/scripts/compiler_output_harness/repsel_census.py @@ -221,6 +221,42 @@ }, } +#: Minimum number of times a deliberate REFUSAL rule must fire, per workload. +#: **Held in code, never in the baseline**, for the same reason as +#: [`LIVENESS_FLOORS`], and gated separately. +#: +#: Every other number in this census is a promotion count, and every gate on it +#: is a floor — which can only catch a promotion that STOPPED happening. #7128 +#: is the opposite failure: `benchmarks/suite/15_mandelbrot.ts` promoted three +#: counters it should not have, and paid **+14.87% instructions retired** for +#: them on a quiet Raspberry Pi 5 at a 0.02% noise floor. No floor anywhere in +#: this file can go red for that, because more promotions always reads as an +#: improvement. +#: +#: So the refusal itself gets a minimum. Reverting +#: `collectors/repsel_benefit.rs` takes `15_mandelbrot` from three +#: `no_i32_consuming_use` denials to zero and this check goes red — which is +#: the only direction in which the census can currently observe an +#: unprofitable promotion at all. +REFUSAL_FLOORS: dict[str, dict[str, int]] = { + # `py`, `px` and `iter`: all three proven by #7110's loop-induction + # interval, all three consumed only as doubles inside a loop + # (`totalIter + iter`, `px - WIDTH / 2.0`). Pinned at the exact count, so + # losing any one goes red rather than degrading to "still nonzero". + "suite_15_mandelbrot": {"no_i32_consuming_use": 3}, + # `i` in `result = result + (1.0 / i)`: the same shape one workload over, + # which is what says the rule generalises rather than pattern-matching + # 15_mandelbrot. + "suite_06_math_intensive": {"no_i32_consuming_use": 1}, + # `mixedWithFloat`'s `hit`, the hand-written minimal case. It sits beside + # `iterate`'s `iter` in the same file, admitted by the same #7110 interval + # proof and differing only in what consumes it — so this floor and that + # fixture's `canonical-i32: 3` liveness floor cannot both be satisfied by a + # rule that is simply always-yes or always-no. + "fixture_loop_bounded_i32": {"no_i32_consuming_use": 1}, +} + + #: Workloads allowed to produce **zero candidates** — no analysis considered any #: value in them. Held in code for the same reason as [`LIVENESS_FLOORS`]: it is #: an assertion about the compiler, and `--update` must not be able to widen it. @@ -350,6 +386,7 @@ def census_from_report(report: dict[str, Any]) -> dict[str, Any]: # population. It is reported separately instead (`consumed_receiver`). consumed_receiver = 0 unconsumed_mechanisms: dict[str, int] = {} + denial_rules: dict[str, int] = {} # Keyed by analysis as well as by rule. `unconsumed_mechanisms` alone is # rule-keyed, and with a second instrumented analysis a gap in analysis A # would be excused by a mechanism recorded for analysis B. @@ -392,6 +429,16 @@ def census_from_report(report: dict[str, Any]) -> dict[str, Any]: unconsumed_by_analysis[str(analysis)] = ( unconsumed_by_analysis.get(str(analysis), 0) + 1 ) + elif outcome == "denied": + # #7128. Denials are context, never a floor — with ONE exception, + # `no_i32_consuming_use`, which is not a failed proof but a + # deliberate REFUSAL of a provable promotion. A refusal cannot be + # gated by a promotion floor, because floors are minimums and the + # regression it prevents is an EXTRA promotion. So the rule is + # counted here and given its own minimum in [`REFUSAL_FLOORS`]. + denial_rules[str(entry.get("rule") or "")] = ( + denial_rules.get(str(entry.get("rule") or ""), 0) + 1 + ) if unknown_sites: raise HarnessError( f"--opt-report recorded consumption at unregistered site(s) " @@ -425,6 +472,7 @@ def census_from_report(report: dict[str, Any]) -> dict[str, Any]: "counts": counts, "candidates": candidates, "unconsumed_mechanisms": unconsumed_mechanisms, + "denial_rules": denial_rules, "unconsumed_by_analysis": unconsumed_by_analysis, "consumed_receiver": consumed_receiver, "consumption_sites": consumption_sites, @@ -657,6 +705,37 @@ def check_liveness_fixtures(observed: dict[str, dict[str, Any]]) -> list[str]: return failures +def check_refusal_floors(observed: dict[str, dict[str, Any]]) -> list[str]: + """Every deliberate refusal must still be firing where it was measured. + + The mirror image of [`check_liveness_fixtures`]. That one asks "is the + representation still being promoted"; this asks "is the promotion still + being refused where refusing it was worth 14.87% of the instructions". + + Independent of the baseline file on purpose — see [`REFUSAL_FLOORS`]. + """ + failures: list[str] = [] + for name, minimums in REFUSAL_FLOORS.items(): + if name not in observed: + failures.append( + f"refusal workload {name!r} did not run; the census cannot claim " + "to observe a refusal it never measured" + ) + continue + rules = observed[name].get("denial_rules", {}) + for rule, minimum in minimums.items(): + seen = int(rules.get(rule, 0)) + if seen < minimum: + failures.append( + f"{name}: rule {rule!r} refused {seen} promotion(s), and must " + f"refuse at least {minimum}. Either the profitability model " + "stopped firing (collectors/repsel_benefit.rs) or the workload " + "changed shape; an unprofitable promotion is invisible to every " + "floor in this census, which is why this check exists." + ) + return failures + + def check_instrument_liveness(observed: dict[str, dict[str, Any]]) -> list[str]: """No census key may read zero across the ENTIRE corpus. @@ -985,6 +1064,7 @@ def census(args: argparse.Namespace) -> int: regressions += reg improvements += imp liveness = check_liveness_fixtures(observed) if not partial else [] + refusals = check_refusal_floors(observed) if not partial else [] dead = check_instrument_liveness(observed) if not partial else [] unreached = check_analysis_reach(observed) if not partial else [] # Always checked, even for a --workload subset: it is an internal @@ -1009,6 +1089,7 @@ def census(args: argparse.Namespace) -> int: for label, problems in ( ("REGRESSION", regressions), ("DEAD INSTRUMENT", liveness + dead), + ("REFUSAL NO LONGER FIRING", refusals), ("UNREACHED BY EVERY ANALYSIS", unreached), ("CONSUMPTION COUNTER IS INCOHERENT", invariant), ("WASTED PROMOTION WITH NO NAMED MECHANISM", unexplained), @@ -1131,6 +1212,30 @@ def self_test(_args: argparse.Namespace) -> int: dead = check_instrument_liveness({"w": {"counts": counts}}) assert any("ptr-shape" in d for d in dead), dead + # #7128: the refusal check must go red when the rule stops firing, and + # green only when it fires at least as often as it was measured to. + target = next(iter(REFUSAL_FLOORS)) + minimums = REFUSAL_FLOORS[target] + rule, minimum = next(iter(minimums.items())) + all_silent = { + name: {"counts": counts, "denial_rules": {}} for name in REFUSAL_FLOORS + } + silent = check_refusal_floors(all_silent) + assert any(rule in f and target in f for f in silent), silent + all_firing = { + name: {"counts": counts, "denial_rules": dict(mins)} + for name, mins in REFUSAL_FLOORS.items() + } + assert not check_refusal_floors(all_firing), all_firing + one_short = dict(all_firing) + one_short[target] = { + "counts": counts, + "denial_rules": {rule: minimum - 1}, + } + assert check_refusal_floors(one_short), "one short of the floor must be red" + absent = check_refusal_floors({}) + assert any(target in f for f in absent), absent + failures = check_liveness_fixtures({"fixture_ptr_shape": {"counts": counts}}) assert any("fixture_ptr_shape" in f for f in failures), failures diff --git a/tests/test_repsel_census.py b/tests/test_repsel_census.py index a1b7ca7069..85611f1084 100644 --- a/tests/test_repsel_census.py +++ b/tests/test_repsel_census.py @@ -91,6 +91,18 @@ def consumed_entry( } +def denied_entry(analysis: str, rule: str, local_id=1) -> dict: + return { + "analysis": analysis, + "outcome": "denied", + "rep": "Boxed", + "position": "local", + "local_id": local_id, + "function": "module_init", + "rule": rule, + } + + def unconsumed_entry(analysis: str, rule: str, local_id=1) -> dict: return { "analysis": analysis, @@ -716,5 +728,70 @@ def test_a_missing_report_is_an_error_not_an_empty_census(self): CENSUS._extract_json("nothing here\n") +class RefusalFloors(unittest.TestCase): + """#7128: the one gate in this census that catches an EXTRA promotion. + + Every other check here is a floor on promotions, and a floor cannot go red + when a compiler promotes more. `15_mandelbrot` promoted three counters it + should not have and paid +14.87% instructions retired for them; these tests + are the ones that turn red if that refusal stops firing. + """ + + def test_denied_entries_are_counted_by_rule(self): + payload = report( + denied={"canonical-slot": 3}, + entries=[ + denied_entry("canonical-slot", "no_i32_consuming_use", local_id=1), + denied_entry("canonical-slot", "no_i32_consuming_use", local_id=2), + denied_entry("canonical-slot", "not_index_used_or_bounded", local_id=3), + ], + ) + rules = CENSUS.census_from_report(payload)["denial_rules"] + self.assertEqual(rules["no_i32_consuming_use"], 2) + self.assertEqual(rules["not_index_used_or_bounded"], 1) + + def test_a_silent_refusal_is_red(self): + observed = { + name: {"counts": {}, "denial_rules": {}} for name in CENSUS.REFUSAL_FLOORS + } + failures = CENSUS.check_refusal_floors(observed) + self.assertTrue(failures, "a refusal that stopped firing must be red") + self.assertIn("suite_15_mandelbrot", " ".join(failures)) + + def test_one_short_of_the_floor_is_red(self): + """Pinned at the exact count, so losing ONE of the three is red. + + `15_mandelbrot` refuses `py`, `px` and `iter`. A rule that kept + refusing only `iter` would leave two per-iteration `sitofp`s behind and + still report a nonzero refusal count. + """ + observed = { + name: {"counts": {}, "denial_rules": dict(mins)} + for name, mins in CENSUS.REFUSAL_FLOORS.items() + } + observed["suite_15_mandelbrot"]["denial_rules"]["no_i32_consuming_use"] = 2 + self.assertTrue(CENSUS.check_refusal_floors(observed)) + + def test_meeting_every_floor_is_green(self): + observed = { + name: {"counts": {}, "denial_rules": dict(mins)} + for name, mins in CENSUS.REFUSAL_FLOORS.items() + } + self.assertEqual(CENSUS.check_refusal_floors(observed), []) + + def test_a_workload_that_did_not_run_is_red(self): + """A refusal the census never measured is not a refusal it observed.""" + self.assertTrue(CENSUS.check_refusal_floors({})) + + def test_the_refusal_generalises_beyond_one_workload(self): + """Two different programs, so the rule cannot be a 15_mandelbrot patch. + + If someone narrows the model until only `15_mandelbrot` is refused, + `06_math_intensive` (`result = result + (1.0 / i)`) goes red. + """ + self.assertGreaterEqual(len(CENSUS.REFUSAL_FLOORS), 2) + self.assertIn("suite_06_math_intensive", CENSUS.REFUSAL_FLOORS) + + if __name__ == "__main__": unittest.main() From 05014c5c3bc4e0f125f8ede446827579af0fdc6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 11:02:11 +0200 Subject: [PATCH 03/12] fix(repsel): a self-write in an i32 position is a benefit, not a suppressed read --- .../src/collectors/repsel_benefit.rs | 23 ++++++--- .../src/collectors/repsel_benefit/tests.rs | 47 +++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/crates/perry-codegen/src/collectors/repsel_benefit.rs b/crates/perry-codegen/src/collectors/repsel_benefit.rs index 6623f429bb..3226857229 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit.rs @@ -78,8 +78,12 @@ //! parameter `n` is the single most common loop in JavaScript; classifying //! its guard as a double consumer would refuse nearly every counter in the //! corpus to buy nothing measurable. -//! * **A write of a local into its own slot is neutral** (`iter = iter + 1`), -//! because the value never leaves the representation it arrived in. +//! * **A write of a local into its own slot never costs** (`iter = iter + 1`), +//! because the value never leaves the representation it arrived in — but it +//! still *counts as a benefit* when it lands in an i32-consuming position. +//! `seed = (Math.imul(seed, K) + C) & 0x7fffffff` is a local whose only read +//! is inside its own assignment and whose whole reason to want an i32 slot +//! is that read. //! //! A missed refusal is today's behaviour. A spurious refusal is a lost //! promotion, so the model is built to err in the first direction. @@ -167,12 +171,19 @@ impl<'a> Model<'a> { } fn read(&mut self, id: u32, ctx: UseCtx, depth: u32) { - if self.self_target == Some(id) { - return; - } + // A read of a local inside its own assignment is representation- + // PRESERVING, so it is never a cost — `iter = iter + 1` costs one + // instruction whichever slot `iter` lives in. It is still a benefit + // when it lands in an i32-consuming position, and suppressing that + // half was wrong: `seed = (Math.imul(seed, K) + C) & 0x7fffffff` is a + // local whose ONLY read is inside its own assignment and whose whole + // reason to want an i32 slot is that read + // (`benchmarks/suite/17_loop_data_dependent.ts`, and every FNV / + // xorshift mixer in the corpus). + let is_self = self.self_target == Some(id); match ctx { UseCtx::Int => self.entry(id).int_reads += 1, - UseCtx::Double if depth >= 1 => self.entry(id).hot_double_reads += 1, + UseCtx::Double if depth >= 1 && !is_self => self.entry(id).hot_double_reads += 1, _ => {} } } diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs index 02ac55ab32..bd706de946 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -403,6 +403,53 @@ fn math_imul_operand_is_a_benefit() { assert!(!out.contains(&1), "Math.imul operand refused: {out:?}"); } +/// `seed = (Math.imul(seed, K) + C) & 0x7fffffff` — a local whose ONLY read is +/// inside its own assignment, in an i32-consuming position. The self-target +/// exemption must suppress the COST half and not the BENEFIT half: +/// `17_loop_data_dependent`'s LCG seed and every FNV/xorshift mixer in the +/// corpus have exactly this shape, and also divide the value out as a double +/// once per iteration. +#[test] +fn a_self_write_in_an_i32_position_is_still_a_benefit() { + // 1 = seed (already holds parallel-shadow i32 storage via its `&` write) + let strictly: HashSet = [1u32].into_iter().collect(); + let stmts = vec![ + let_mut(1, Some(Expr::Integer(42))), + for_loop( + let_mut(2, Some(Expr::Integer(0))), + cmp(get(2), Expr::Integer(64)), + inc(2), + vec![ + set( + 1, + bin( + BinaryOp::BitAnd, + Expr::MathImul(Box::new(get(1)), Box::new(Expr::Integer(1103515245))), + Expr::Integer(0x7fff_ffff), + ), + ), + Stmt::Expr(call(vec![bin( + BinaryOp::Div, + get(1), + Expr::Number(2147483647.0), + )])), + ], + ), + ]; + let (_, _, unsigned, int_valued_ta) = no_i32_storage(); + let empty = HashSet::new(); + let out = collect_unprofitable_canonical_i32_locals( + &stmts, + &I32StorageFacts { + index_used: &empty, + strictly_bounded: &strictly, + unsigned: &unsigned, + int_valued_ta: &int_valued_ta, + }, + ); + assert!(!out.contains(&1), "self-written i32 mixer refused: {out:?}"); +} + /// An expression form the model does not understand contributes neither side. /// A refusal rule must under-approximate the cost: a missed refusal is today's /// behaviour, a spurious one is a lost promotion. From 064f1bc9c567b11939213354ee548ae31b2e6916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 11:12:54 +0200 Subject: [PATCH 04/12] docs(repsel): record profitability as a first-class selection question --- benchmarks/repsel_census/README.md | 23 +++++++++++++++++++++++ docs/representation-selection-rfc.md | 23 +++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/benchmarks/repsel_census/README.md b/benchmarks/repsel_census/README.md index b52fe7e2f6..e9b2bbde7b 100644 --- a/benchmarks/repsel_census/README.md +++ b/benchmarks/repsel_census/README.md @@ -111,6 +111,29 @@ report *no consumption data* rather than a zero (`CONSUMPTION_INSTRUMENTED` in the script), because "uninstrumented" and "never applied" are exactly the pair this census exists to keep apart. +## The one check that catches an EXTRA promotion + +Every number above is a promotion count and every gate on it is a **floor**, so +the census can only ever go red when a representation stops firing. #7128 is the +opposite failure: `benchmarks/suite/15_mandelbrot.ts` promoted three counters it +should not have — all three provably i32-bounded, none ever used as an integer — +and paid **+14.87% instructions retired** for them, measured on a quiet +Raspberry Pi 5 at a 0.02% noise floor. No floor in this file can go red for +that; more promotions always reads as an improvement. + +So the deliberate refusal gets a minimum of its own. `REFUSAL_FLOORS` (in +`scripts/compiler_output_harness/repsel_census.py`, in code and not in the +baseline, for the same reason as `LIVENESS_FLOORS`) says how many times the +`no_i32_consuming_use` rule must fire per workload. Deleting +`crates/perry-codegen/src/collectors/repsel_benefit.rs` takes `15_mandelbrot` +from three refusals to zero and the census red. + +`fixture_loop_bounded_i32.ts` carries the paired case: `iterate()`'s counter and +`mixedWithFloat()`'s counter are admitted by the identical #7110 interval proof +and differ only in what consumes them. One must promote (its `canonical-i32` +liveness floor) and one must be refused (its refusal floor), so neither an +always-yes nor an always-no rule can satisfy the file. + ## How it cannot quietly pass Read CLAUDE.md, "★ Four ways a gate can be unable to fail". The fourth applies diff --git a/docs/representation-selection-rfc.md b/docs/representation-selection-rfc.md index 90ea53c460..6dfddd513a 100644 --- a/docs/representation-selection-rfc.md +++ b/docs/representation-selection-rfc.md @@ -109,6 +109,29 @@ control flow (a value's representation at a `catch` join is the meet over all po paths — in practice `Boxed` unless all throwing paths agree). The inference treats each of these as a hard meet-to-`Boxed` edge; none of them may be "optimized through." +**Profitability is a second, separate question (#7128).** Everything above answers *may we +select this representation*. Nothing in it answers *should we*, and the two have different +failure modes: an unsound selection is a wrong answer, an unprofitable one is a slower answer +that no test can see. #7128 measured the consequence — after #7110 widened the i32 range proof +and #7121 removed the module-init exclusion, `benchmarks/suite/15_mandelbrot.ts` promoted three +provably-bounded loop counters, none of which is ever used as an integer, and paid **+14.87% +instructions retired** for them (invisible in wall time: the workload is FP-latency-bound). + +For an i32-range value, `double` is a lossless and equal-cost representation of `+`, `-` and +comparison. `I32` only *buys* something where the consumer cannot take a double without a +conversion — array/typed-array indexing, bitwise operands, `Math.imul` — which is the same list +§5.3 already gives for where `I32` semantics are exact. It becomes a *cost* the moment any hot +consumer needs the double back. So selection carries one profitability term alongside the +soundness terms: a value written after its declaration, with no i32-consuming read anywhere and +at least one double-consuming read inside a loop, stays `Boxed` +(`collectors/repsel_benefit.rs`). The term only ever refuses, so every uncertainty resolves +toward "not a cost"; a missed refusal is the pre-existing behaviour. + +The same shape exists for the other representations and is not yet modelled: #7128 found every +`__pshape` clone dead-stripped before the object (finding C) and `Ptr` emitting nothing +outside its own fixture (finding D). Both are "selected, and no byte changed" — the cheap end of +the same gap. + ### 5.3 Representation-selected lowering & operation semantics Locals and params get native slots per representation; loop phis are typed; ops stay native end-to-end. **Operation semantics are representation-preserving — lowering may never change an From 69cbf86db858291d92f034f484c50455fdb31487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 11:17:19 +0200 Subject: [PATCH 05/12] fix(repsel): carry the self-write marker through unmodelled expression nodes --- .../src/collectors/repsel_benefit.rs | 16 ++++++---- .../src/collectors/repsel_benefit/tests.rs | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/crates/perry-codegen/src/collectors/repsel_benefit.rs b/crates/perry-codegen/src/collectors/repsel_benefit.rs index 3226857229..f939573d91 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit.rs @@ -300,13 +300,15 @@ impl<'a> Model<'a> { Expr::Closure { .. } => {} // Everything else: recurse neutrally. Under-approximating the cost // is the safe direction for a rule that can only refuse. - _ => { - let saved = self.self_target.take(); - perry_hir::walker::walk_expr_children(e, &mut |child| { - self.expr(child, UseCtx::Neutral, depth) - }); - self.self_target = saved; - } + // + // `self_target` is deliberately carried THROUGH an unmodelled node + // rather than cleared: `t = (t / 2.0)` is still a write + // of `t` into its own slot, and clearing the marker would turn it + // into a cost. A nested `LocalSet` re-targets the marker for its + // own RHS and restores it, so the invariant holds either way. + _ => perry_hir::walker::walk_expr_children(e, &mut |child| { + self.expr(child, UseCtx::Neutral, depth) + }), } } diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs index bd706de946..c53b665054 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -450,6 +450,38 @@ fn a_self_write_in_an_i32_position_is_still_a_benefit() { assert!(!out.contains(&1), "self-written i32 mixer refused: {out:?}"); } +/// The self-write exemption survives an unmodelled wrapper. `t = void (t / 2.0)` +/// is still a write of `t` into its own slot; if the marker were cleared on the +/// way down, an unmodelled node anywhere above a `/` would silently turn a +/// self-write into a refusal. +/// +/// The second local in the same position is the anti-vacuity control: the `/` +/// under the wrapper really does reach the model as a cost, so a green verdict +/// for `t` is the exemption working and not the walk stopping early. +#[test] +fn a_self_write_through_an_unmodelled_node_is_still_not_a_cost() { + // 1 = t (self-written), 2 = other (read in the same expression) + let stmts = vec![ + let_mut(1, Some(Expr::Integer(0))), + let_mut(2, Some(Expr::Integer(0))), + Stmt::Expr(inc(2)), + Stmt::While { + condition: cmp(get(1), Expr::Integer(100)), + body: vec![set( + 1, + Expr::Void(Box::new(bin( + BinaryOp::Div, + bin(BinaryOp::Add, get(1), get(2)), + Expr::Number(2.0), + ))), + )], + }, + ]; + let out = run(&stmts, &HashSet::new()); + assert!(!out.contains(&1), "wrapped self-write refused: {out:?}"); + assert!(out.contains(&2), "control local not refused: {out:?}"); +} + /// An expression form the model does not understand contributes neither side. /// A refusal rule must under-approximate the cost: a missed refusal is today's /// behaviour, a spurious one is a lost promotion. From e6e47aee703f1a6ff811b85adad8176237ff1cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 11:39:36 +0200 Subject: [PATCH 06/12] chore(census): record the #7128 refusal in the baseline floors --- benchmarks/repsel_census/baseline.json | 20 ++++---- changelog.d/7132-repsel-profitability.md | 60 ++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 changelog.d/7132-repsel-profitability.md diff --git a/benchmarks/repsel_census/baseline.json b/benchmarks/repsel_census/baseline.json index c402822320..e1cea6f0f2 100644 --- a/benchmarks/repsel_census/baseline.json +++ b/benchmarks/repsel_census/baseline.json @@ -173,15 +173,15 @@ "canonical-u32": 0, "canonical-str": 0, "int-valued-ta": 0, - "spec-abi-entry": 1, + "spec-abi-entry": 2, "spec-abi-taptr-slot": 0 }, "candidates": { "ptr-shape": 0, "ptr-numarray": 0, - "canonical-slot": 5, + "canonical-slot": 7, "int-valued-ta": 0, - "spec-abi": 4 + "spec-abi": 5 }, "unconsumed_mechanisms": {}, "consumption_sites": {} @@ -375,7 +375,7 @@ "ptr-shape": 0, "ptr-shape-consumed": 0, "ptr-numarray": 0, - "canonical-i32": 2, + "canonical-i32": 1, "canonical-u32": 0, "canonical-str": 0, "int-valued-ta": 0, @@ -400,7 +400,7 @@ "ptr-shape": 1, "ptr-shape-consumed": 0, "ptr-numarray": 0, - "canonical-i32": 2, + "canonical-i32": 1, "canonical-u32": 0, "canonical-str": 0, "int-valued-ta": 0, @@ -533,7 +533,7 @@ "ptr-shape": 1, "ptr-shape-consumed": 0, "ptr-numarray": 0, - "canonical-i32": 2, + "canonical-i32": 1, "canonical-u32": 0, "canonical-str": 0, "int-valued-ta": 0, @@ -560,7 +560,7 @@ "ptr-shape": 0, "ptr-shape-consumed": 0, "ptr-numarray": 0, - "canonical-i32": 2, + "canonical-i32": 1, "canonical-u32": 0, "canonical-str": 0, "int-valued-ta": 0, @@ -585,7 +585,7 @@ "ptr-shape": 0, "ptr-shape-consumed": 0, "ptr-numarray": 0, - "canonical-i32": 2, + "canonical-i32": 1, "canonical-u32": 0, "canonical-str": 0, "int-valued-ta": 0, @@ -610,7 +610,7 @@ "ptr-shape": 0, "ptr-shape-consumed": 0, "ptr-numarray": 0, - "canonical-i32": 6, + "canonical-i32": 3, "canonical-u32": 0, "canonical-str": 0, "int-valued-ta": 0, @@ -678,5 +678,5 @@ "consumption_sites": {} } ], - "generated_at": "2026-07-31T06:45:01.246943Z" + "generated_at": "2026-07-31T09:26:30.260742Z" } diff --git a/changelog.d/7132-repsel-profitability.md b/changelog.d/7132-repsel-profitability.md new file mode 100644 index 0000000000..84172466e7 --- /dev/null +++ b/changelog.d/7132-repsel-profitability.md @@ -0,0 +1,60 @@ +### Fixed + +- **repsel: canonical i32 is now chosen on benefit, not only on provability + (#7128).** `benchmarks/suite/15_mandelbrot.ts` regressed **+14.87% + instructions retired** at #7121, measured on a quiet Raspberry Pi 5 with + `perf stat` at a 0.02% noise floor and bisected by binary hash. Wall time did + not move (48 ms vs 49 ms) because the workload is FP-latency-bound, which is + why nothing caught it. + + **Root cause, read out of the emitted AArch64 rather than inferred.** The + innermost loop is `while (x*x + y*y <= 4.0 && iter < MAX_ITER)`. With `iter` + a boxed double, both exit tests are FP and LLVM fuses them into `fcmp` + + `fccmp`: **one basic block, 12 instructions, one branch**. #7122's monotone + loop-induction interval proves `iter ∈ [0, 100]` and #7121 let that proof + reach the module-init body, so `iter` took a canonical i32 slot — and an + integer compare cannot fuse with an FP compare. The loop splits into **two + blocks totalling 14 instructions**, plus a `ucvtf` at `totalIter = totalIter + + iter`. 2 instructions × 8,011,148 innermost iterations ≈ 16.0M, against a + measured +15.63M. `px` and `py` are the same shape one level out + (`px - WIDTH / 2.0`). + + **The defect was not the proof.** The proof is correct. The Let-site + eligibility gate in `stmt/let_stmt.rs` was a conjunction of "may we?" terms + with no "should we?" term anywhere in it, so widening the proof + automatically widened the emission. + + **The fix is a profitability model** (`collectors/repsel_benefit.rs`), + consulted by the selection gate as one more conjunct. For an i32-range value + a `double` is a lossless, equal-cost representation of `+`, `-` and + comparison; canonical i32 only *buys* something where the consumer cannot + take a double without a conversion — array/typed-array indexing, bitwise + operands, `Math.imul` — and becomes a *cost* the moment a hot consumer needs + the double back. So a local that is written after its declaration, has no + i32-consuming read anywhere, and has at least one double-consuming read + inside a loop stays boxed. The model only ever refuses, so every uncertainty + resolves toward "not a cost" (comparison is neutral on both sides — + `for (let i = 0; i < n; i++)` with a `number` parameter must keep + promoting). + + Measured on the Pi, 11 repeats, `perf stat -e instructions:u`: + + | workload | before | after | Δ | + |---|---|---|---| + | `15_mandelbrot` | 120,738,726 | 105,110,137 | **−12.94%** | + | `11_prime_sieve` (the #7121 win) | 2,597,174,368 | 2,597,174,368 | 0.00% | + | `08_string_concat` (the #7121 `Str` win) | 30,281,732 | 30,281,732 | 0.00% | + + Over the whole 26-workload census corpus the emitted object changes on + **exactly one** program, and there its disassembly is byte-identical to the + pre-#7121 compiler's. The other 9 refused promotions were already emitting + byte-identical code, so the census counts fall (canonical-i32 64 → 55) + without a single emitted byte moving. + + **Gated.** Every other number in the promotion census is a floor, and a floor + cannot go red when a compiler promotes *more*. `REFUSAL_FLOORS` in + `scripts/compiler_output_harness/repsel_census.py` gives the refusal its own + minimum; `benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts` now + carries `iterate()` and `mixedWithFloat()` side by side — same #7110 interval + proof, opposite verdict, differing only in what consumes the counter — so + neither an always-yes nor an always-no rule can satisfy the file. From 0ed73fe1674ae65fb710197fa8e51f49fdcf85d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 11:50:55 +0200 Subject: [PATCH 07/12] test(repsel): give the mandelbrot reduction its outer-loop f64 use --- .../src/collectors/repsel_benefit/tests.rs | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs index c53b665054..890e26554f 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -106,6 +106,7 @@ fn run(stmts: &[Stmt], index_used: &HashSet) -> HashSet { /// ```text /// let totalIter = 0; /// for (let py = 0; py < 800; py++) { +/// const cy = (py - 400.0) * 4.0; /// for (let px = 0; px < 800; px++) { /// const cx = (px - 400.0) * 4.0; /// let iter = 0; @@ -120,7 +121,7 @@ fn run(stmts: &[Stmt], index_used: &HashSet) -> HashSet { /// consumed only as doubles inside a loop. This is the +14.87% regression. #[test] fn mandelbrot_iter_is_refused() { - // 1 = totalIter, 2 = py, 3 = px, 4 = cx, 5 = iter + // 1 = totalIter, 2 = py, 3 = px, 4 = cx, 5 = iter, 6 = cy let inner_while = Stmt::While { condition: Expr::Logical { op: LogicalOp::And, @@ -147,18 +148,36 @@ fn mandelbrot_iter_is_refused() { // totalIter = totalIter + iter → iter read into a boxed accumulator set(1, bin(BinaryOp::Add, get(1), get(5))), ]; + // The py loop carries its own `const cy = (py - 400.0) * 4.0`, exactly as + // the source does. Leaving it out is how the first version of this test + // passed for `px` and `iter` while `py` stayed promoted — the reduction + // was wrong, not the model. + let py_body = vec![ + Stmt::Let { + id: 6, + name: "cy".into(), + ty: HirType::Number, + mutable: false, + init: Some(bin( + BinaryOp::Mul, + bin(BinaryOp::Sub, get(2), Expr::Number(400.0)), + Expr::Number(4.0), + )), + }, + for_loop( + let_mut(3, Some(Expr::Integer(0))), + cmp(get(3), Expr::Integer(800)), + inc(3), + px_body, + ), + ]; let stmts = vec![ let_mut(1, Some(Expr::Integer(0))), for_loop( let_mut(2, Some(Expr::Integer(0))), cmp(get(2), Expr::Integer(800)), inc(2), - vec![for_loop( - let_mut(3, Some(Expr::Integer(0))), - cmp(get(3), Expr::Integer(800)), - inc(3), - px_body, - )], + py_body, ), ]; From aa41722042032dda2a4d6ddd65f87086aa4216e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 12:11:21 +0200 Subject: [PATCH 08/12] fix(repsel): the self-write exemption follows the representation, not the shape CodeRabbit on #7132: `x = x / 2` and `x = f(x)` are self-writes whose VALUE is a double. Keying the exemption on the syntactic shape scored them as free, which would promote a local whose hot consumer wants a double -- the exact failure this module refuses, re-entering through the fix. --- .../src/collectors/repsel_benefit.rs | 70 ++++++++++---- .../src/collectors/repsel_benefit/tests.rs | 92 ++++++++++++++++--- 2 files changed, 131 insertions(+), 31 deletions(-) diff --git a/crates/perry-codegen/src/collectors/repsel_benefit.rs b/crates/perry-codegen/src/collectors/repsel_benefit.rs index f939573d91..17bd605381 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit.rs @@ -85,6 +85,14 @@ //! is inside its own assignment and whose whole reason to want an i32 slot //! is that read. //! +//! The exemption is about the *representation flow*, not the syntactic shape +//! `t = … t …`: it is cleared on the way through any operation that forces a +//! materialization (a `/` or `**` operand, a call or `new` argument), so +//! `x = x / 2` and `x = f(x)` are still costs. Those genuinely convert out +//! and back on every iteration, which is the failure mode this module +//! exists to refuse — re-entering through the exemption instead of through +//! the original path. +//! //! A missed refusal is today's behaviour. A spurious refusal is a lost //! promotion, so the model is built to err in the first direction. //! @@ -161,7 +169,10 @@ struct Model<'a> { storage: &'a I32StorageFacts<'a>, tallies: std::collections::HashMap, /// The local currently being written, if the walk is inside the RHS of - /// `LocalSet(t, …)`. A read of `t` there is representation-preserving. + /// `LocalSet(t, …)` **and** has not yet passed through an operation that + /// forces a representation change. See [`Model::forced_double`]: the + /// exemption is for values that flow back into their own slot without + /// leaving their representation, not for the syntactic shape `t = … t …`. self_target: Option, } @@ -171,14 +182,18 @@ impl<'a> Model<'a> { } fn read(&mut self, id: u32, ctx: UseCtx, depth: u32) { - // A read of a local inside its own assignment is representation- - // PRESERVING, so it is never a cost — `iter = iter + 1` costs one - // instruction whichever slot `iter` lives in. It is still a benefit - // when it lands in an i32-consuming position, and suppressing that - // half was wrong: `seed = (Math.imul(seed, K) + C) & 0x7fffffff` is a - // local whose ONLY read is inside its own assignment and whose whole - // reason to want an i32 slot is that read - // (`benchmarks/suite/17_loop_data_dependent.ts`, and every FNV / + // A read of a local inside its own assignment is not a cost WHEN the + // value flows back into the same slot without leaving its + // representation — `iter = iter + 1` costs one instruction whichever + // slot `iter` lives in. `self_target` is cleared on the way through any + // operation that forces a materialization ([`Model::forced_double`]), + // so `x = x / 2` and `x = f(x)` are still costs: those really do + // `sitofp` out and convert back on every iteration. + // + // The exemption never suppresses the BENEFIT half: + // `seed = (Math.imul(seed, K) + C) & 0x7fffffff` is a local whose ONLY + // read is inside its own assignment and whose whole reason to want an + // i32 slot is that read (`17_loop_data_dependent`, and every FNV / // xorshift mixer in the corpus). let is_self = self.self_target == Some(id); match ctx { @@ -188,6 +203,19 @@ impl<'a> Model<'a> { } } + /// Descend into a position whose `Double` context is **forced by the + /// operation** rather than inherited from a slot: a `/` or `**` operand, or + /// a call / `new` argument. The value is genuinely materialized as an f64 + /// there, so the self-write exemption must not survive the descent — + /// otherwise `x = x / 2` and `x = f(x)` would read as free, and an i32 slot + /// for `x` would pay a conversion per iteration and buy nothing, which is + /// the exact failure this whole module exists to refuse. + fn forced_double(&mut self, e: &Expr, depth: u32) { + let saved = self.self_target.take(); + self.expr(e, UseCtx::Double, depth); + self.self_target = saved; + } + /// The context a value acquires by being stored into `target`. fn target_ctx(&self, target: u32) -> UseCtx { if self.storage.holds_i32(target) { @@ -232,6 +260,18 @@ impl<'a> Model<'a> { self.expr(object, UseCtx::Neutral, depth); self.expr(index, UseCtx::Int, depth); } + // `/` and `**` are floating-point in JS regardless of their + // operands, so an i32 operand is materialized here whatever the + // surrounding context says — including inside the local's own + // assignment. + Expr::Binary { + op: BinaryOp::Div | BinaryOp::Pow, + left, + right, + } => { + self.forced_double(left, depth); + self.forced_double(right, depth); + } Expr::Binary { op, left, right } => { let child = match op { // Bitwise operands are ToInt32-coerced by the language, so @@ -242,13 +282,10 @@ impl<'a> Model<'a> { | BinaryOp::Shl | BinaryOp::Shr | BinaryOp::UShr => UseCtx::Int, - // `/` and `**` are floating-point in JS regardless of the - // operands, so an i32 operand must be converted. - BinaryOp::Div | BinaryOp::Pow => UseCtx::Double, // Additive / multiplicative chains inherit their root: // `a[i * 4 + k]` keeps `i` and `k` integral, `sum + i * 2` // makes them doubles. - BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul | BinaryOp::Mod => ctx, + _ => ctx, }; self.expr(left, child, depth); self.expr(right, child, depth); @@ -272,16 +309,17 @@ impl<'a> Model<'a> { self.expr(b, UseCtx::Int, depth); } // Perry's calling convention passes NaN-boxed doubles, so every - // argument position converts an i32 operand back. + // argument position converts an i32 operand back — `x = f(x)` + // included, which is why these go through `forced_double`. Expr::Call { callee, args, .. } => { self.expr(callee, UseCtx::Neutral, depth); for a in args { - self.expr(a, UseCtx::Double, depth); + self.forced_double(a, depth); } } Expr::New { args, .. } => { for a in args { - self.expr(a, UseCtx::Double, depth); + self.forced_double(a, depth); } } Expr::Conditional { diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs index 890e26554f..3532f09468 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -469,16 +469,79 @@ fn a_self_write_in_an_i32_position_is_still_a_benefit() { assert!(!out.contains(&1), "self-written i32 mixer refused: {out:?}"); } -/// The self-write exemption survives an unmodelled wrapper. `t = void (t / 2.0)` -/// is still a write of `t` into its own slot; if the marker were cleared on the -/// way down, an unmodelled node anywhere above a `/` would silently turn a -/// self-write into a refusal. -/// -/// The second local in the same position is the anti-vacuity control: the `/` -/// under the wrapper really does reach the model as a cost, so a green verdict -/// for `t` is the exemption working and not the walk stopping early. +/// `x = x / 2` — a self-write whose VALUE is a double. The exemption is about +/// the representation flow, not the syntactic shape `t = … t …`: `/` is +/// floating-point in JS whatever its operands, so an i32 slot for `x` would +/// `sitofp` out and `fptosi` back on every iteration and buy nothing. Raised +/// by CodeRabbit on #7132; it fails against the first version of the +/// exemption, which keyed on the shape. +#[test] +fn self_divide_is_still_a_cost() { + let stmts = vec![ + let_mut(1, Some(Expr::Integer(64))), + Stmt::While { + condition: cmp(get(1), Expr::Integer(1)), + body: vec![set(1, bin(BinaryOp::Div, get(1), Expr::Number(2.0)))], + }, + ]; + let out = run(&stmts, &HashSet::new()); + assert!( + out.contains(&1), + "self-divided counter must still be refused: {out:?}" + ); +} + +/// `x = f(x)` — the same point through the other code path. Perry's calling +/// convention passes NaN-boxed doubles, so the argument position materializes +/// even though the value comes straight back into `x`'s own slot. #[test] -fn a_self_write_through_an_unmodelled_node_is_still_not_a_cost() { +fn self_referencing_call_argument_is_still_a_cost() { + let stmts = vec![ + let_mut(1, Some(Expr::Integer(0))), + Stmt::While { + condition: cmp(get(1), Expr::Integer(100)), + body: vec![set(1, call(vec![get(1)]))], + }, + ]; + let out = run(&stmts, &HashSet::new()); + assert!( + out.contains(&1), + "self-referencing call argument must still be refused: {out:?}" + ); +} + +/// …and `x = new C(x)` likewise, since `new` has its own arm. +#[test] +fn self_referencing_new_argument_is_still_a_cost() { + let stmts = vec![ + let_mut(1, Some(Expr::Integer(0))), + Stmt::While { + condition: cmp(get(1), Expr::Integer(100)), + body: vec![set( + 1, + Expr::New { + class_name: "C".into(), + args: vec![get(1)], + type_args: vec![], + byte_offset: 0, + }, + )], + }, + ]; + let out = run(&stmts, &HashSet::new()); + assert!( + out.contains(&1), + "self-referencing new argument must still be refused: {out:?}" + ); +} + +/// The exemption still holds through a representation-PRESERVING chain that +/// the model does not otherwise model: `t = -(t + other)` keeps `t` in its own +/// slot. The second local is the anti-vacuity control — it is read in the same +/// expression and is NOT a self-write, so it must be refused; a green verdict +/// for `t` therefore cannot come from the walk stopping early. +#[test] +fn a_self_write_through_a_preserving_chain_is_still_not_a_cost() { // 1 = t (self-written), 2 = other (read in the same expression) let stmts = vec![ let_mut(1, Some(Expr::Integer(0))), @@ -488,16 +551,15 @@ fn a_self_write_through_an_unmodelled_node_is_still_not_a_cost() { condition: cmp(get(1), Expr::Integer(100)), body: vec![set( 1, - Expr::Void(Box::new(bin( - BinaryOp::Div, - bin(BinaryOp::Add, get(1), get(2)), - Expr::Number(2.0), - ))), + Expr::Unary { + op: perry_hir::UnaryOp::Neg, + operand: Box::new(bin(BinaryOp::Add, get(1), get(2))), + }, )], }, ]; let out = run(&stmts, &HashSet::new()); - assert!(!out.contains(&1), "wrapped self-write refused: {out:?}"); + assert!(!out.contains(&1), "preserving self-write refused: {out:?}"); assert!(out.contains(&2), "control local not refused: {out:?}"); } From 76435456392da22cb36b3ec26a5111ddc50a0c67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 12:13:11 +0200 Subject: [PATCH 09/12] test(repsel): fix the New literal in the self-argument case --- crates/perry-codegen/src/collectors/repsel_benefit/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs index 3532f09468..8cc9975a1d 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -524,6 +524,7 @@ fn self_referencing_new_argument_is_still_a_cost() { args: vec![get(1)], type_args: vec![], byte_offset: 0, + cap_args_appended: 0, }, )], }, From 7d1c85b8ef84b0defd8efc615ce8160e2258826b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 12:35:45 +0200 Subject: [PATCH 10/12] docs: final measured numbers in the changelog fragment --- changelog.d/7132-repsel-profitability.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/changelog.d/7132-repsel-profitability.md b/changelog.d/7132-repsel-profitability.md index 84172466e7..121e5f8f81 100644 --- a/changelog.d/7132-repsel-profitability.md +++ b/changelog.d/7132-repsel-profitability.md @@ -41,9 +41,13 @@ | workload | before | after | Δ | |---|---|---|---| - | `15_mandelbrot` | 120,738,726 | 105,110,137 | **−12.94%** | - | `11_prime_sieve` (the #7121 win) | 2,597,174,368 | 2,597,174,368 | 0.00% | - | `08_string_concat` (the #7121 `Str` win) | 30,281,732 | 30,281,732 | 0.00% | + | `15_mandelbrot` | 120,738,701 | 105,110,087 | **−12.94%** | + | `11_prime_sieve` (the #7121 win) | 2,597,182,143 | 2,597,177,676 | −0.00% | + | `08_string_concat` (the #7121 `Str` win) | 30,281,798 | 30,281,711 | −0.00% | + + Both #7121 wins re-measured against the `at7122` arm with this compiler: + canonical `Str` **−4.12%**, canonical i32 **−1.05%**. They hold by + construction — the linked binary for each is byte-identical to `main`'s. Over the whole 26-workload census corpus the emitted object changes on **exactly one** program, and there its disassembly is byte-identical to the From 66a57daeec1416a0c8b6cd8064e827dcc9e7dd52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 12:49:11 +0200 Subject: [PATCH 11/12] test(census): pair every lowered canonical-i32 floor with a refusal minimum CodeRabbit on #7132: a floor that fell because a promotion was deliberately refused must be paired with the assertion that it is still being refused, otherwise the lower floor silently accommodates a DIFFERENT promotion going missing. Counts verified per workload with --opt-report, not inferred. --- changelog.d/7132-repsel-profitability.md | 7 +++---- .../compiler_output_harness/repsel_census.py | 20 ++++++++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/changelog.d/7132-repsel-profitability.md b/changelog.d/7132-repsel-profitability.md index 121e5f8f81..ab2dd466c0 100644 --- a/changelog.d/7132-repsel-profitability.md +++ b/changelog.d/7132-repsel-profitability.md @@ -14,10 +14,9 @@ loop-induction interval proves `iter ∈ [0, 100]` and #7121 let that proof reach the module-init body, so `iter` took a canonical i32 slot — and an integer compare cannot fuse with an FP compare. The loop splits into **two - blocks totalling 14 instructions**, plus a `ucvtf` at `totalIter = totalIter - + iter`. 2 instructions × 8,011,148 innermost iterations ≈ 16.0M, against a - measured +15.63M. `px` and `py` are the same shape one level out - (`px - WIDTH / 2.0`). + blocks totalling 14 instructions**, plus a `ucvtf` where the accumulator + joins. 2 instructions × 8,011,148 innermost iterations ≈ 16.0M, against a + measured +15.63M. `px` and `py` are the same shape one level out. **The defect was not the proof.** The proof is correct. The Let-site eligibility gate in `stmt/let_stmt.rs` was a conjunction of "may we?" terms diff --git a/scripts/compiler_output_harness/repsel_census.py b/scripts/compiler_output_harness/repsel_census.py index 33d3005cb6..61875730e1 100644 --- a/scripts/compiler_output_harness/repsel_census.py +++ b/scripts/compiler_output_harness/repsel_census.py @@ -244,10 +244,24 @@ # (`totalIter + iter`, `px - WIDTH / 2.0`). Pinned at the exact count, so # losing any one goes red rather than degrading to "still nonzero". "suite_15_mandelbrot": {"no_i32_consuming_use": 3}, - # `i` in `result = result + (1.0 / i)`: the same shape one workload over, - # which is what says the rule generalises rather than pattern-matching - # 15_mandelbrot. + # Every OTHER workload whose canonical-i32 floor this change lowered, so + # that no lowered floor rests on observation alone (CodeRabbit on #7132). A + # floor that fell because a promotion was deliberately refused must be + # paired with the assertion that it is still being refused; otherwise the + # lower floor silently accommodates a DIFFERENT promotion going missing. + # These are also what says the rule generalises rather than pattern-matching + # `15_mandelbrot`: six programs, four distinct syntactic shapes. + # + # 06 `result = result + (1.0 / i)` — f64 divide operand + # 07 `new Point(i, i + 1)` — constructor argument + # 12 `new Point3D(i, i + 1, i + 2)` — constructor argument + # 13 `sum = sum + (i % 1000)` — boxed accumulator join + # 14 `sum = sum + compute(i)` — call argument "suite_06_math_intensive": {"no_i32_consuming_use": 1}, + "suite_07_object_create": {"no_i32_consuming_use": 1}, + "suite_12_binary_trees": {"no_i32_consuming_use": 1}, + "suite_13_factorial": {"no_i32_consuming_use": 1}, + "suite_14_closure": {"no_i32_consuming_use": 1}, # `mixedWithFloat`'s `hit`, the hand-written minimal case. It sits beside # `iterate`'s `iter` in the same file, admitted by the same #7110 interval # proof and differing only in what consumes it — so this floor and that From 4ff84340886f2d9899cb0db29bae285c3b3c52eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 12:53:58 +0200 Subject: [PATCH 12/12] docs: correct the refusal accounting in the changelog fragment --- changelog.d/7132-repsel-profitability.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/changelog.d/7132-repsel-profitability.md b/changelog.d/7132-repsel-profitability.md index ab2dd466c0..24d29bec58 100644 --- a/changelog.d/7132-repsel-profitability.md +++ b/changelog.d/7132-repsel-profitability.md @@ -49,10 +49,12 @@ construction — the linked binary for each is byte-identical to `main`'s. Over the whole 26-workload census corpus the emitted object changes on - **exactly one** program, and there its disassembly is byte-identical to the - pre-#7121 compiler's. The other 9 refused promotions were already emitting + **exactly one** benchmark, and there its disassembly is byte-identical to the + pre-#7121 compiler's. The other refused promotions were already emitting byte-identical code, so the census counts fall (canonical-i32 64 → 55) - without a single emitted byte moving. + without a single emitted byte moving. Every lowered floor is paired with a + `no_i32_consuming_use` minimum, so a floor that fell because a promotion was + refused cannot silently accommodate a different promotion going missing. **Gated.** Every other number in the promotion census is a floor, and a floor cannot go red when a compiler promotes *more*. `REFUSAL_FLOORS` in