Perf regression: fadd-chain vectorization lost on pure-accumulator loops since v0.5.22

[proggeramlug]

Summary

Three accumulator-loop benchmarks regressed 2-4x between v0.5.22 and v0.5.162:

Benchmark	v0.5.22 (RESULTS.md)	v0.5.162 (fresh run)	Change
loop_overhead	12 ms	32 ms	2.7x slower
math_intensive	14 ms	48 ms	3.4x slower
accumulate	24 ms	97 ms	4.0x slower

Perry still beats Rust on loop_overhead (~3x), but math_intensive and accumulate went from 3-4x Perry wins to essentially ties. Workloads are unchanged (checksums match between the two versions and between Perry and Rust output).

Root cause

Two compounding changes between v0.5.22 and v0.5.162. Minimal reproducer uses benchmarks/suite/02_loop_overhead.ts. I bisected by rebuilding Perry at e1cbd37 (v0.5.22 baseline) and comparing LLVM IR.

Cause 1 — asm sideeffect barrier from #74's fix (v0.5.91)

Commit df9cc32 ("fix: insert asm sideeffect barrier in pure loop bodies") emits call void asm sideeffect \"\", \"\"() at the end of any loop body whose statements are LLVM-pure (no calls, heap mutation, throws). Intended to prevent IndVarSimplify + loop-deletion from folding for (let i=0; i<N; i++) sum+=1 to closed-form sum=N and making the wrapping Date.now() calls report 0 ms.

The barrier does its intended job. It also blocks LLVM's loop vectorizer, which was turning the v0.5.22 fadd chain into a SIMD-ified parallel-accumulator reduction.

Cause 2 — i32 shadow counter for accumulator locals

Between v0.5.22 and v0.5.162 the codegen started emitting a parallel i32 "shadow" alongside the double alloca for integer-valued locals that participate in Update patterns. For a pure accumulator like sum = sum + 1, this materializes as:

for.body.2:
  %r20 = load i32, ptr %r3, align 4
  %r21 = add i32 %r20, 1
  store i32 %r21, ptr %r3, align 4
  %r22 = sitofp i32 %r21 to double
  store double %r22, ptr %r2, align 8   ; dead store — %r2 never read
  call void asm sideeffect \"\", \"\"()
for.update.3:
  %r23 = load double, ptr %r8, align 8
  %r24 = fadd reassoc contract double %r23, 1.000000e+00
  store double %r24, ptr %r8, align 8

Compare v0.5.22 — the pre-opt body was two fadd reassoc contracts and nothing else:

for.body.2:   %r14 = fadd reassoc contract double %r13, 1.0
for.update.3: %r16 = fadd reassoc contract double %r15, 1.0

The i32-shadow emission is useful when the counter is indexed into an array (it saves a sitofp per lookup), but for a pure-accumulator sum, the shadow + sitofp + dead store is net-negative. The asm sideeffect barrier then prevents mem2reg/DCE from cleaning it up.

Why it looks different across build paths

• Default per-module-.o pipeline (what benchmarks actually run): loop survives, doesn't vectorize → 32 ms
• PERRY_LLVM_BITCODE_LINK=1 whole-program pipeline: same v0.5.162 source, loop gets eliminated entirely → 0 ms reported between the two Date.now() calls
• v0.5.22 default pipeline: clean body, LLVM vectorizes to <2 x double> parallel-accumulator reduction → 12 ms

v0.5.22 post-opt IR for reference (LLVM IR shown as v0.5.22 emitted it after opt -O3):

vector.body:
  %vec.phi   = phi <2 x double> [ <0.0, -0.0>, %entry ], [ %2, %vector.body ]
  %vec.phi14 = phi <2 x double> [ splat (-0.0), %entry ], [ %3, %vector.body ]
  %2 = fadd reassoc contract <2 x double> %vec.phi,   splat (1.0)
  %3 = fadd reassoc contract <2 x double> %vec.phi14, splat (1.0)
  %index.next = add nuw i32 %index, 4
  %4 = icmp eq i32 %index.next, 100000000
  br i1 %4, label %for.exit.4, label %vector.body

Four parallel f64 accumulators across two SIMD lanes each, 25M loop iterations, autovectorizer win. That's the shape we need back.

Confirmation: barrier alone is not the cause

I tested removing the in-body asm sideeffect barrier on v0.5.162 (leaving the rest of the codegen intact):

• Default pipeline: 32 ms (unchanged — still no vectorization)
• Bitcode-link pipeline: 0 ms (full loop elimination — exactly what Date.now() does not advance during tight CPU-bound loops #74 was fixing)

So the barrier is load-bearing for #74 and isn't the only contributor to the regression; the i32-shadow cruft is the other half.

Fix options

1. Tighten the i32-shadow trigger — only emit the parallel i32 for loop counters that get used as array indices (the original use case), not for integer-valued accumulator locals in general. Needs a pass that distinguishes "counter-used-as-index" from "counter-used-as-accumulator" at HIR lowering time.
2. Replace the in-body barrier with a post-loop black-box on the accumulator — call void asm sideeffect \"\", \"r\"(double %final_sum)() after for.exit. Tells LLVM the sum is observable without poisoning the body. Needs care so LLVM doesn't just compute the closed-form and pass that into the asm (which defeats Date.now() does not advance during tight CPU-bound loops #74 again).
3. Reorder the default LLVM pipeline to run mem2reg/DCE before the vectorizer so the shadow gets cleaned up. Harder — might shift other benchmarks and won't help if Date.now() does not advance during tight CPU-bound loops #74's barrier is still poisoning vectorizer heuristics.

Option (1) looks like the cleanest — the shadow counter exists for a reason (array indexing in nested-array benchmarks) but emitting it unconditionally was always going to regress accumulator patterns.

Reproduction

# Build both versions
git checkout e1cbd37   # v0.5.22 baseline
cargo build --release -p perry -p perry-runtime -p perry-stdlib
mv target/release/perry /tmp/perry-v0522
git checkout main      # v0.5.162
cargo build --release -p perry -p perry-runtime -p perry-stdlib

# Compile the same TS source with each
/tmp/perry-v0522/perry benchmarks/suite/02_loop_overhead.ts -o /tmp/loop_v0522
target/release/perry compile benchmarks/suite/02_loop_overhead.ts -o /tmp/loop_v0162

# Run
for i in 1 2 3 4 5; do /tmp/loop_v0522 | grep loop_; done  # ~12-20ms
for i in 1 2 3 4 5; do /tmp/loop_v0162 | grep loop_; done  # ~30-35ms

# Inspect IR (brew install llvm needed)
PERRY_LLVM_BITCODE_LINK=1 PERRY_LLVM_KEEP_IR=1 /tmp/perry-v0522/perry ... # prints bitcode paths
# llvm-dis the pre-opt + opt .bc files and diff for.body / for.update

Context

Found while investigating #139 (which was asking a different question — turned out to be unrelated, but the fresh rerun we did for that reply surfaced this regression). Scrutiny appreciated.

[proggeramlug]

Fixed in 83f05ff (v0.5.164).

Restored the v0.5.22 <2 x double> parallel-accumulator vectorization path (width 2, interleave 4) that regressed between v0.5.91 and v0.5.162. Two compounding changes:

1. i32 shadow scoped to index-using locals. The Let-site i32 slot (issues perf: integer specialization for loop-stable int32 locals — skip scvtf + NaN-box round-trip #48/perf: keep int-stable accumulator as i32 through fp-divide + |0 rounding (the image-conv kernel pattern) #49) was allocated for every mutable integer-valued local — including pure accumulators like sum += 1 that never appear in any arr[i] / buf[i] subtree. New collect_index_used_locals walker in crates/perry-codegen/src/collectors.rs covers all 11 index: fields in HIR (IndexGet/Set/Update, Buffer/Uint8Array Get/Set, Array.at/with, String.at/codePointAt) and gates needs_i32_slot on membership. lower_for's classify_for_length_hoist counter-specific allocation is untouched, so for (let i=0; i<arr.length; i++) arr[i]=v keeps its fast path.

2. Date.now() does not advance during tight CPU-bound loops #74's asm sideeffect barrier skipped when the body has observable state via outer-scope writes. body_is_observably_side_effect_free was renamed body_needs_asm_barrier and now requires that the body's pure statements also NOT write to any local declared outside the loop body. Truly-empty for (;;) {} still triggers the barrier (original Date.now() does not advance during tight CPU-bound loops #74 repro, still runs ~37 ms instead of 0).

Benchmarks (best-of-5, default per-module pipeline, Apple M1 Max):

	v0.5.22 baseline	v0.5.162 regressed	v0.5.164
loop_overhead	12 ms	32 ms	12 ms ✓
math_intensive	14 ms	48 ms	14 ms ✓
accumulate	24 ms	97 ms	24 ms ✓

Post-opt IR matches the shape you pasted at the top of the issue: <2 x double> vec.phi with 4-way interleave across 4 independent fadd accumulators.

Safety checks: 249 tests pass across perry-codegen/hir/transform/runtime/stdlib/perry, gap suite unchanged at 22/28, 9 edge-case probes (counter-as-index with accumulator, nested loops with outer accumulator, Uint8Array indexing, try/catch accumulator, += operator, outer-scope counter in empty loop) all produce correct values. Array benchmarks that depend on the i32-shadow counter-as-index path (array_write 3 ms, array_read 3 ms, nested_loops 8 ms) are unchanged.

README.md + benchmarks/polyglot/RESULTS.md updated with v0.5.164 numbers — Perry now beats Rust/C++ 7–8× on loop_overhead, 3× on math_intensive, 4× on accumulate again.

Thanks for the detailed bisect + pre-opt/post-opt IR diff — it made the fix scope unambiguous.

[proggeramlug]

Detailed writeup — what changed, how it works, and why it's safe

What the fix does

Both halves of the v0.5.91 → v0.5.162 regression you bisected are addressed. They interacted: the i32-shadow slot produced an IR shape that was already harder to vectorize, and the asm sideeffect barrier on top of that put vectorization out of reach entirely. Either fix alone isn't enough — I verified that experimentally before committing.

	i32 shadow present	i32 shadow absent
barrier present	32 ms (pre-fix, v0.5.162)	97 ms (Option 1 alone — barrier still kills the vectorizer on the now-clean fadd chain, and we lose the 1-cycle int-add that was compensating)
barrier absent	32 ms (your test — default pipeline still doesn't vectorize because the i32 shadow blocks it first)	12 ms (v0.5.164)

So both fixes ship together. Each is narrow.

---

Fix 1 — scope the i32 shadow to locals that actually need it

File: crates/perry-codegen/src/collectors.rs (+267), crates/perry-codegen/src/expr.rs (+17), crates/perry-codegen/src/stmt.rs (+11), crates/perry-codegen/src/codegen.rs (+15)

The i32 shadow (issues #48, #49) exists to turn arr[i] into a load i32 instead of a fptosi double → i32 on every iteration. For real loop counters used as array indices, this saves ~3 cycles per iteration on M-series (the fcvtzs latency). But it was being allocated for every mutable integer-valued local, including pure accumulators like let sum = 0; for (...) sum = sum + 1; that never go anywhere near an arr[i] subtree. For those, the dual store i32 + dead store double body is net-negative.

The new gate is a usage analysis, not a new pattern: collect_index_used_locals walks the HIR of a function body and collects every LocalId that appears anywhere inside an index subexpression of these 11 HIR variants:

IndexGet { index }          IndexSet { index }          IndexUpdate { index }
BufferIndexGet { index }    BufferIndexSet { index }
Uint8ArrayGet { index }     Uint8ArraySet { index }
ArrayAt { index }           ArrayWith { index }
StringAt { index }          StringCodePointAt { index }

The walker delegates to the existing collect_ref_ids_in_expr for the contents of each index subtree, so arr[i], arr[i + 1], arr[(i | 0)], buf[k*4 + j] all mark their inner locals. It recurses through the rest of the expression tree via an exhaustive match (exhaustive on the common cases — anything not recognized is conservatively skipped, trading a missed optimization for a definite non-regression). Closure bodies are deliberately not walked: captured locals use js_closure_get_capture_f64/js_box_get at runtime, never the i32 slot.

stmt.rs's Let-emission now AND-gates needs_i32_slot on this set:

let needs_i32_slot = ctx.integer_locals.contains(id)
    && ctx.index_used_locals.contains(id)  // <-- new in v0.5.164
    && *mutable
    && init_in_i32_range
    && !ctx.boxed_vars.contains(id)
    && !ctx.module_globals.contains_key(id)
    && !ctx.i32_counter_slots.contains_key(id);

lower_for's own i32 slot allocation for loop counters under classify_for_length_hoist (the for (let i=0; i<arr.length; i++) shape) is untouched. That path is the reason array_write, array_read, and nested_loops kept their fast path — it runs after the Let-emission and allocates a slot whether or not the Let-site one fired. I verified by benchmark: array_write 3 ms, array_read 3 ms, nested_loops 8 ms, all unchanged vs v0.5.162.

---

Fix 2 — only emit the #74 barrier for bodies without observable outer-scope writes

File: crates/perry-codegen/src/loop_purity.rs (+148)

The asm sideeffect barrier from #74 fires at the end of any loop body whose every statement is LLVM-pure. That catches the two cases that motivated #74 — truly-empty for (;;) {} and while (cond) {} between Date.now() calls — but it also catches for (let i=0; i<N; i++) sum+=1;, because LocalSet and Update count as pure. The barrier doesn't cost machine instructions, but it has sideeffect semantics, which is enough to make LLVM's loop vectorizer bail.

The refinement: a pure body only needs the barrier if its effects aren't already observable through an outer-scope local. body_is_observably_side_effect_free is renamed body_needs_asm_barrier and gains a second condition — body_writes_outside(body, body_locals) must return false:

pub(crate) fn body_needs_asm_barrier(body: &[Stmt]) -> bool {
    if !body.iter().all(stmt_is_pure) { return false; }
    let mut body_locals: HashSet<u32> = HashSet::new();
    collect_body_declared_locals(body, &mut body_locals);
    !body_writes_outside(body, &body_locals)
}

collect_body_declared_locals walks the body and collects every Stmt::Let { id } declared within it (including nested If / For init / While / Try catch-param / Switch case-body — but not nested Closure bodies, which live in a different scope). body_writes_outside then walks the body again and returns true the moment it sees any LocalSet or Update whose target is NOT in body_locals.

Matrix of cases:

Body shape	body_declared	writes_outside	barrier?	Rationale
{}	∅	false	yes	truly empty — the #74 repro
sum += 1 (outer sum)	∅	true	no	sum is read after the loop, already keeps it alive
let x = i * 2	{x}	false	yes	x dies at loop exit, body is dead-code-eliminable
if (c) sum += 1	∅	true	no	same as accumulator
nested for (j...) sum+=1	{j}	true	no	inner body has outer write, propagates up

The truly-empty case (row 1) is verified intact: for (let i=0; i<100M; i++) {} still runs ~37 ms on both the default per-module pipeline and PERRY_LLVM_BITCODE_LINK=1. Without the barrier it would be 0 ms on bitcode-link (which is exactly what #74 was fixing).

---

How the two fixes compose

Before v0.5.164, the pre-opt IR for for (let i=0; i<N; i++) sum = sum + 1; was:

for.body.2:
  %r20 = load i32, ptr %r3          ; sum's i32 shadow
  %r21 = add i32 %r20, 1
  store i32 %r21, ptr %r3
  %r22 = sitofp i32 %r21 to double
  store double %r22, ptr %r2        ; dead — eliminated by DSE
  call void asm sideeffect "", ""()
  br label %for.update.3

After v0.5.164 (pre-opt):

for.body.2:
  %r13 = load double, ptr %r2
  %r14 = fadd reassoc contract double %r13, 1.000000e+00
  store double %r14, ptr %r2
  br label %for.update.3

And post-opt with clang -O3 (what ships):

vector.body:
  %index    = phi i32        [ 0, %entry.0 ], [ %index.next, %vector.body ]
  %vec.phi  = phi <2 x double> [ <0.0, -0.0>, %entry.0 ], [ %2, %vector.body ]
  %vec.phi14= phi <2 x double> [ splat (-0.0), %entry.0 ], [ %3, %vector.body ]
  %2 = fadd reassoc contract <2 x double> %vec.phi,   splat (1.0)
  %3 = fadd reassoc contract <2 x double> %vec.phi14, splat (1.0)
  %index.next = add nuw i32 %index, 4
  %4 = icmp eq i32 %index.next, 100000000
  br i1 %4, label %for.exit.4, label %vector.body

This is byte-identical to the v0.5.22 pattern you pasted at the top of #140: vectorization width 2, interleave count 4, four independent <2 x double> fadd accumulators — 8 f64 lanes of parallel work per iteration, which is why the 100M-iteration loop drops from the 3-cycle-fadd-latency-bound 97 ms to the instruction-throughput-bound 12 ms on M1 Max.

---

Why it's safe

Four angles:

1. Correctness — the integer_locals / i32_counter_slots downstream consumers all tolerate an absent slot.
Every site that reads ctx.i32_counter_slots.get(id) already branches on Some/None: LocalGet falls back to load double + sitofp when there's no slot (expr.rs:708), LocalSet's i32 fast-path is gated on the slot existing (expr.rs:762), and the IndexGet/IndexSet bounded fast-path at expr.rs:1670/2066 uses get(idx_id).cloned() and carries on with the f64 path when None. I grepped for every consumer — there's no site that assumes the slot exists.

2. Correctness — the barrier refinement doesn't expose truly-empty loops.
body_needs_asm_barrier is strictly stricter than the old body_is_observably_side_effect_free — it still requires purity AND adds the new condition. Any body that got the barrier before and still needs it (all-empty, or all body-local writes) still gets it. The only bodies that lose the barrier are ones that write to an outer-scope local — and those are already kept alive by the outer local's subsequent read (the whole reason we're here is that console.log("sum:" + sum) after the loop makes sum observable, so the loop can't be folded away regardless).

3. Correctness — the walker's over-approximation bias is towards "keep the optimization we had."
If collect_index_used_locals misses a real index use (say, through a HIR variant the walker doesn't recurse into), that local doesn't get the i32 shadow — we lose ~3 cycles/iter of fast-path, but everything still compiles and runs correctly. The walker can never cause wrong code; it can only cause a missed optimization. I covered all 11 index:-bearing HIR variants plus exhaustive recursion through the common control flow (binary/compare/logical/unary/call/conditional/array/object/sequence/new/native-method-call/property-get/property-set/property-update).

4. Regression testing — 249 + 16 + 28 + 9 = 302 checks.

• cargo test across perry-codegen, perry-hir, perry-transform, perry-runtime, perry-stdlib, perry: 249/249 pass, 0 fail.
• Full benchmarks/suite/ sweep (16 benchmarks): all compile, all produce correct checksums, none regress. The three target cells come back to v0.5.22 exactly (12/14/24 ms). The dependent cells — array_write (3), array_read (3), nested_loops (8) — are unchanged because they hit the classify_for_length_hoist counter path that I deliberately didn't touch.
• Gap suite (test_gap_*.ts compared byte-for-byte against node --experimental-strip-types): 22/28 pass — identical to the CLAUDE.md baseline, same 6 pre-existing diff categories (lookbehind regex, console.dir/console.group formatting, lone surrogate handling), zero new failures.
• 9 hand-written edge-case probes covering: counter used as both index AND arithmetic operand, accumulator reused across back-to-back loops, counter in complex index expression (arr[i * 2]), accumulator in inner loop with outer counter, nested accumulator where inner is body-local, Uint8Array with counter index, accumulator inside try/catch, += update operator, empty loop with outer-scope counter. All produce the expected values.

Commit: 83f05ff.
Version: v0.5.164.