perf: the % fast path gates on the i32 set, but only needs integer-valued-in-i64 (20.4× on bench_bitwise)

[proggeramlug]

bench_bitwise is 20.4× slower than Node (56956 vs 2787, checksums identical after #7403 made it measure anything at all). The cause is one instruction choice.

Evidence chain

The hot loop contains no runtime helper calls at all — it is pure LLVM arithmetic, so the usual "native-able primitive became a runtime call" story does not apply here. The op mix per iteration:

4 frem     ← the problem
3 fsub  3 fadd  2 fmul  2 sitofp  2 fcmp  1 icmp  1 add

frem is not an aarch64 instruction. It lowers to a libm call:

$ otool -tvV … | grep 'bl.*fmod'
4 × bl  _fmod        # in runArithmeticBenchmark

And the profile says it dominates:

1754 samples  fmod

Four fmod calls per iteration × 10M iterations. The source is ordinary integer arithmetic:

result = result + (a % 1000);
result = result - (b % 1000);
result = result + ((a * 3) % 10000);
result = result - ((b * 2) % 10000);

V8 keeps these as SMIs and uses integer modulo.

The mechanism already exists — and knows this is the problem

stmt/loops.rs's emit_object_array_write_index_i64 emits srem, and its doc comment names the exact issue:

"the counters' native i32 registers drive i64 add/sub/mul/srem directly: no float round-trip and, critically, no frem (which lowers to an fmod LIBRARY CALL on AArch64 — ~10ns per element). srem equals JS % on the proven domain (nonnegative dividend, positive divisor)."

So the correctness argument is already worked out and the emitter is already written.

Why it does not fire here

The integer path is reachable only through ObjectArrayWriteNumber, whose variants are OuterCounter | InnerCounter | Constant | Add | Sub | Mul | Mod — the #6812 object-array-write index pattern. a and b in bench_bitwise are loop-carried locals (a = a + 1, reset by a conditional), not loop counters, so they never enter that domain and fall to the f64 emitter's frem.

Every other BinaryOp::Mod site emits frem unconditionally: expr/mod.rs:2240, expr/binary.rs:650, codegen/typed_abi/lower.rs:56 and :433, lower_call/scalar_method.rs:174.

Suggested direction

Widen the integer-domain proof from loop counters to loop-carried locals with a provable integer range. The correctness side-condition is already stated (nonnegative dividend, positive divisor) and the emitter already exists; what is missing is the analysis that admits more values into the proven domain.

This is the same lever #7296 proved worth 9.9× on matmul — see the engine plan's sequencing item 2, where reducing what must be treated as an opaque f64 pulls size and speed together.

Caveat

Timings taken at host load ~4–25. The 20.4× ratio and the 1754/fmod profile share are far outside that noise; exact figures want a quiet machine.

Related: #7396 (array-store guard siting), #7403 (this benchmark measured nothing until today), #7294 (engine plan, Part 2).

[proggeramlug]

Specification note for whoever implements this — the naive fptosi/srem/sitofp fast path is wrong in four distinct ways. Measured against Node:

case	JS result	naive srem path
-0 % 5	-0	+0 — wrong
-2147483648 % -1	-0	UB in LLVM srem, and wrong zero even if it survived
5 % 0	NaN	UB (division by zero)
1e19 % 4	0	out of i64 range; loops.rs already notes this sremmed to 3
5.5 % 2	1.5	non-integral — must reach the slow path

The -0 cases are the nastiest: -0.0 == 0.0 compares true under fcmp oeq, so an "is this integral?" check written as sitofp(fptosi(x)) == x admits -0 and then silently produces +0. That difference is observable (Object.is(-0 % 5, -0) is true in JS) and would not show up in any test that compares with ===.

So the guard needs, at minimum:

• divisor != 0
• not (dividend == INT_MIN && divisor == -1)
• both operands integral and -0-free on the dividend
• both within the integer range the chosen width can represent

The existing emit_object_array_write_index_i64 sidesteps all of this by only admitting values from a proven domain (nonnegative dividend, positive divisor) — which is why widening the domain analysis is the sound route, and a general guarded fast path at every BinaryOp::Mod site is not the shortcut it looks like.

Any implementation should carry a differential test over exactly this table plus NaN/Infinity dividends, asserting with Object.is rather than === so the -0 cases cannot pass silently.

[proggeramlug]

Correcting this issue: the optimization already exists. The gap is the analysis that gates it.

I set out to implement a guarded srem fast path, built it, verified it against Node on the whole edge-case table above (all passing, including both -0 cases) — and then found Perry already has one. I reverted mine.

What exists

expr/binary.rs:588:

if matches!(op, BinaryOp::Mod)
    && crate::type_analysis::is_integer_valued_expr(ctx, left)
    && crate::type_analysis::is_integer_valued_expr(ctx, right)
    && !right_is_known_zero
{
    let li = blk.fptosi(DOUBLE, &l_raw, I64);
    let ri = blk.fptosi(DOUBLE, &r_raw, I64);
    let m  = blk.srem(I64, &li, &ri);
    // IEEE 754: when the integer remainder is 0 and the dividend was
    // negative, the result must be -0.0.
    …
}

It already handles the -0 case the table above warns about, and it is general — not restricted to loop counters the way stmt/loops.rs's emit_object_array_write_index_i64 is. My earlier framing in this issue was wrong on that point: there are two integer-mod mechanisms, not one, and the general one is analysis-gated rather than shape-gated.

Why it does not fire for bench_bitwise

is_integer_valued_expr (type_analysis/numeric.rs) resolves a LocalGet to:

Expr::LocalGet(id) => ctx.integer_locals.contains(id),

a % 1000 has right = Expr::Integer(1000) → passes. left = LocalGet(a) → a is not in ctx.integer_locals, so the whole condition fails and the expression falls to frem.

a is declared let a = 12345678 and only ever assigned a = a + 1 or the literal reset. It is integer-valued by inspection; the analysis simply does not admit loop-carried locals of that shape.

The actual work

Widen integer_locals to admit a local whose declaration and every assignment are integer-valued expressions. Nothing in the emitter needs to change — the fast path, its -0 correction and its zero-divisor guard are all already there and already correct.

That is a dataflow question (a fixpoint over assignments), not a new lowering, which makes it considerably smaller and safer than what this issue originally proposed.

Still true from the original report

The measurement stands: 4 × bl _fmod per iteration, 1754 profile samples in fmod, 20.4× slower than Node. Only the prescription changes.

[proggeramlug]

Reduced to a one-line reproduction. The disqualifier is reassignment, not shape:

// srem — fast path fires
function f(){ let a=12345678; let r=0; for(let i=0;i<3;i++){ r=r+(a%1000); } return r; }

// frem — fast path lost, ONLY difference is the increment
function f(){ let a=12345678; let r=0; for(let i=0;i<3;i++){ r=r+(a%1000); a=a+1; } return r; }

Measured by counting srem/frem in the emitted IR (--trace llvm):

shape	srem	frem
let a = …; a % 1000	3	0
+ a = a + 1	18	15
+ a = a + 1 and conditional reset	18	15
(a*3) % 10000, reassigned	18	15
a = a - 1	18	15

Every reassigning shape loses it; the non-reassigning one keeps it. a + 1 where a is already an integer candidate is trivially integer-valued, so this looks like the write-disqualifier running against a candidate set that does not yet contain a — i.e. an ordering problem between seeding and disqualification, rather than a missing rule.

collectors/integer_locals.rs already runs a forward-closure fixpoint (collect_extra_integer_let_ids) for exactly this kind of transitive case, and its doc comment describes the sibling failure it was written to fix:

"without hi in the int-stable set the disqualifier marks the assignment as non-int-producing and removes xx/yy from the set"

That is the same symptom one level up: the fixpoint covers Let inits, but a plain LocalSet reassignment appears not to participate.

This is the whole of bench_bitwise's 20.4× gap — a and b are exactly this shape (a = a + 1 with a conditional reset) — and it is now a self-contained repro anyone can run in seconds.

[proggeramlug]

Narrowed to the exact call, and a warning for whoever takes it.

The disqualification runs in ProvenanceJudge::walk_expr (collectors/integer_locals.rs:632):

Expr::LocalSet(id, value) => {
    if self.candidates.contains(id) {
        self.judge_obligation(*id, value);   // ← `a = a + 1` lands here
    }
    self.walk_expr(value);
}

So the boundary from the repro above is this obligation: a = a + 1 is judged against the optimistic candidate set, and the verdict's dependency edge points at a itself. Start there, not at the fixpoint.

Two hypotheses I had were wrong — recording them so they are not re-tried:

1. "The fixpoint is least, so a is not in the set when its write is judged." No. The module comment is explicit that judgment runs against the optimistic set and prunes transitively, with a stated exactness argument: "the judgment is monotone in the candidate set (it only relies positively on membership), so judging once against the optimistic set and pruning transitively is exact."
2. "There is no rule for reassignment." No. judge_obligation handles every LocalSet rhs, and the doc notes it is deliberately stricter than the admission-side is_int32_producing_expr — e.g. Expr::Update is int-producing only when the updated local is itself a candidate, where admission says yes unconditionally.

That asymmetry between the strict disqualification judgment and the optimistic admission judgment is where I would look first, along with whether a self-referential dependency edge (a's verdict depending on a) prunes itself.

This is not a small change. The design carries a documented exactness proof, and modifying it risks invalidating that proof rather than merely admitting one more local. A widening that is wrong admits a local which can hold a non-integer, and fptosi/srem then returns silently wrong arithmetic that a ===-based test will not catch — the same class as the -0 traps tabled earlier.

Worth pairing any attempt with the differential test from the earlier comment plus the bench_bitwise checksum (CHECKSUM:525000000), which is now a valid oracle after #7403.

[proggeramlug]

Diagnosis complete — and it rules out the explanation I gave two comments ago.

Isolating which write shape disqualifies:

write to a	emitted
none	srem
a = 99 (integer literal)	srem
a = a + 1	frem
a = i + 1 (i is a loop counter)	frem
a++	frem

The fourth row is the one that matters. i is a loop counter and unambiguously integer-valued, so a value-based integer analysis should accept i + 1. It does not. Neither does a++, which Expr::Update is explicitly documented to handle.

So the disqualification is not "this write's value might not be an integer". It is coarser than that — triggered by the presence of any non-literal write at all. That means my previous framing ("widen is_integer_valued_expr to admit loop-carried locals") describes the symptom, not the mechanism, and anyone implementing that is likely to patch the wrong predicate.

Two possibilities worth separating before any change:

1. ProvenanceJudge::judge_obligation rejects Binary{Add, LocalGet, Integer} outright, for a reason the module comment does not spell out; or
2. the non-literal write moves the whole function off a specialised numeric lowering, so integer_locals is not the deciding predicate at all. The srem/frem counts jump together (3/0 → 18/15) across all three failing shapes, which is more consistent with a different code path being selected than with one local losing a property.

Possibility 2 would mean this issue is misfiled against integer_locals entirely.

Running tally of wrong hypotheses about this subsystem, all mine, all recorded so they are not retried:

1. the mechanism lives only in loops.rs, scoped to counters — no, binary.rs:588 is general
2. a guarded srem needs building — no, one exists with the IEEE -0 correction
3. the fixpoint is least-ordered — no, it is optimistic with a written exactness proof
4. there is no rule for reassignment — no, judge_obligation handles every LocalSet rhs
5. it is an integer-valued judgment on the write — no, a = i + 1 fails with i a proven counter

Five in one evening. That rate is why I am not attempting the change itself: the next plausible-looking edit has a poor prior of being right, and a wrong one returns silently incorrect % results that === tests cannot see.

Whoever picks this up: start by distinguishing possibility 1 from possibility 2 — dump the HIR/IR for the a = 99 and a = a + 1 variants side by side and find where the two diverge. Do not start from the tests' headers or from this issue's original title.

[proggeramlug]

Root cause found, and it inverts the prescription. Do not widen integer_locals — that would be a miscompile.

The mismatch

judge_obligation delegates to int32_producing_deps. The property integer_locals computes is int32-range, not integer-valued. Confirmed by its other consumers:

// stmt/let_stmt.rs
let needs_i32_slot            = (ctx.integer_locals.contains(&id) || …)
let canonical_i32_value_eligible = (ctx.integer_locals.contains(&id) || …)

It gates i32 shadow slots. So the exclusions are all correct for what it computes:

write	in i32 set?	why
a = 99	yes	literal, fits i32
a = a + 1	no	unbounded increment chain can exceed i32
a = i + 1	no	same
a++	no	same

Nothing is broken here. Had I widened this set — which is what my earlier comments proposed and what the issue title asked for — a value that overflows i32 would land in an i32 shadow slot. That is a miscompile, and a far worse one than a slow %.

The actual bug

The % fast path at expr/binary.rs:588 does fptosi(DOUBLE → I64) and srem I64. It needs values to be integer-valued within i64, up to 2^63. It does not need i32 at all.

But it gates on is_integer_valued_expr, whose LocalGet arm resolves to integer_locals — the i32 set. So the fast path asks a question it does not need answered, and refuses every local that is integer-valued but not provably i32-ranged.

bench_bitwise's a (12345678, incremented, reset below 1e8) is exactly that: comfortably integer-valued, comfortably inside i64, not provably inside i32.

Correct direction

Introduce a weaker predicate — integer-valued within i64 range — and gate the % fast path on that, leaving integer_locals untouched for its i32-slot consumers. a = a + 1 from an integer literal seed qualifies for the weaker property (an increment chain cannot leave i64 in finite time) while correctly failing the stronger one.

That is additive rather than proof-modifying: no existing consumer changes, and the new set is only read by the % gate.

Corrected tally

Six wrong hypotheses about this subsystem, now including the one this issue was titled after:

1. mechanism lives only in loops.rs — no, binary.rs:588 is general
2. a guarded srem needs building — no, one exists with the IEEE -0 correction
3. the fixpoint is least-ordered — no, optimistic with a written exactness proof
4. no rule exists for reassignment — no, judge_obligation handles every LocalSet rhs
5. it is an integer-valued judgment — no, it is int32
6. widening integer_locals is the fix — no, it would corrupt i32 shadow slots

The subsystem was right the whole way through; the % gate is reading the wrong set.