Repository navigation
packed-f64: an arr.length-bounded loop loses the fast path entirely if the body has any a[k ± c] access (9x, 8ms -> 72ms) #9259
Description
Activity
Root-caused the second defect — the one that caps this issue's payoff — down to a single line. Not taking the fix; handing it over with the evidence, because I burned three inert attempts finding it and the next person should not repeat them.
The literal-bound row is slow for a different reason than the length-bound row
Reproduced #9259 independently, with guard-call counts from the full function disassembly beside every timing:
fixture guard calls perry node k < a.length,s += a[k]1 8 ms 5 k < a.length,s += a[k] + a[k-1]0 70 ms 6 k < 4096,s += a[k]1 8 ms 5 k < 4096,s += a[k] + a[k-1]1 40 ms 8 The last row has its guard and is still 5× slower than the plain loop. Two reads account for roughly 2×; about 2.5× was unexplained. So fixing the bound gap alone lands this shape on a second defect rather than clearing it — worth knowing before sizing the win.
It is not the array reads. It is the add.
The offset loop's hot body is 47 instructions against 11 for the plain loop. Disassembling it, the array reads are already hoisted — what fills those instructions is the numeric-add guard diamond: three
is_numbertag tests, a branch, twofadds on the hot arm, andjs_dynamic_string_or_number_addtwice on the cold arm.and x8, x19, #0xffff000000000000 ; tag test cmp x19, x21 ccmp x8, x25, #0x2, hs cset w8, hi ... ; two more of these b.hi <slow> / cbz <slow> / tbz <slow> fadd d0, d1, d0 ; hot arm fadd d0, d9, d0 b <merge> bl _js_dynamic_string_or_number_add ; cold arm bl _js_dynamic_string_or_number_add
The line
accumulator_rhs_is_numeric,crates/perry-codegen/src/stmt/stable_packed_accumulator.rs:45:Expr::IndexGet { object, index } => matches!( (object.as_ref(), index.as_ref()), (Expr::LocalGet(a), Expr::LocalGet(i)) if *a == array_id && *i == counter_id ),
The index must be the bare counter.
a[k-1]fails, so the RHS is not numeric, sosis never admitted as a numeric accumulator, so every+in the enclosing expression lowers dynamically. Note alsoarray_idsingular — the same line is whys += a[k] * b[k]pays this cost, which matches a 2.5× two-array penalty I measured separately.What does NOT work, so nobody repeats it
I fixed the analogous bare-counter requirement in
has_numeric_index_fact(stable_packed_loop.rs:1379) by routing throughpacked_f64_loop_fact_for_index, which understands offsets soundly. Correct, builds clean — and completely inert:lit_offset41 ms against 40 before.The reason is ordering, and it is the thing that makes this non-trivial: accumulator admission runs in the preheader, before any fact is published — the admitted ids then ride the fact so
is_numeric_exprcan see them. So teaching the later expression-lowering predicate about offsets cannot help; the accumulator was already classified.Why I am not fixing it
The admission walk has no tier information, and it is shared:
lower_packed_f64_versioned_forpublisheswindow_validated: false, so admitting offsets unconditionally there would be unsound — an unvalidated window can yield a hole, the value is not a number, and the accumulator's proof breaks. The range tier validates the offset window and is safe. So the fix needs the tier threaded into admission, or the walk split. That is a design decision on code @ECS1 is actively changing for the bound half, and it deserves better than my fourth attempt at the end of a long session.Trace from #9258 (
PERRY_PACKED_LOOP_TRACE=1,[range-loop]prefix) makes the admission side legible if it helps.Correction to my comment above — I had the two tiers backwards, and the mistake points at the wrong fix. Thanks to @ECS1 for catching it.
I wrote that admitting offset reads in the versioned tier would be unsound because "an unvalidated window can yield a hole." That is false, and I verified the correction rather than taking it:
packed_f64_array_loop_guard(typed_feedback.rs:1386) ends injs_array_is_numeric_f64_layout(header).- That predicate (
array/header.rs:1838) testsarray_numeric_layout(arr) == Some(NumericArrayLayout::RawF64)— a whole-array property — and explicitly answers 0 for a holes-flagged array.
So when the versioned guard passes, every in-bounds slot is a raw f64. There is no hole to hit at any index, offset or not.
What
window_validated: falseactually means is narrower than the name suggests: the length bound proveskis in range, notk ± c. It is a statement about bounds, not about holes — and bounds are exactly what an inlineicmp ult idx, lenagainst the fact's existing side exit re-establishes.The hole concern is real, but it belongs to the range tier, which sets
allow_holes: !matched.dense; its own guard doc says it "normalizes the slots hole-tolerantly" and relies on the inline loads hole-checking and side-exiting.So the conclusion survives but the reasoning inverts. It is not "the versioned tier is unsafe and the range tier is safe." It is that the two tiers carry different guarantees and
collect_numeric_accumulatorscurrently assumes the weaker one for both. The fix threads the caller's guarantee into the walk — and the complication @ECS1 identified is thatPackedAccumulatorScope(loops.rs:869) is shared by the versioned admission and three range admissions (1052 / 2518 / 2567 / 2626), so the flag has to come from each admission site rather than be set once in the helper.Anyone picking this up from my earlier comment would have guarded the wrong tier. The line (
stable_packed_accumulator.rs:45), the ordering trap (admission runs in the preheader before any fact is published, so fixing the later predicate is inert), and the measurements all stand unchanged.- added a commit that references this issue
on Aug 31, 2026 - added 4 commits that reference this issue
on Aug 31, 2026
A packed-f64 loop bounded by
arr.lengthgets a fast clone. Add one non-zero-offset access to the body and it gets no packed fast path at all — not a degraded one, none. Same work, 9× slower, and it flips from beating node to 5.5× behind it.Measurement
Four fixtures, identical output (
16769025000),--no-cache --no-auto-optimize, min of 5–9 runs, self-timed:packed_f64.*blocks in IRk < a.lengths += a[k]k < a.lengths += a[k] + a[k-1]k < 4096s += a[k] + a[k-1]A beats node. B is the same loop plus one
a[k-1]and is 9× slower than A. C is B with the bound written as a literal and is 2.06× faster than B.Spread is tight and well clear of the effect (B: 72 73 74 74 74 75 75 81; C: 35 35 36 36 36 36 38 44).
Mechanism
Two matchers each cover half the shape, and the combination falls between them:
lower_packed_f64_versioned_for(stmt/loops.rs:5823) handles thei < arr.lengthbound and publishes its fact withwindow_validated: false— its guard provesiitself in bounds, not an offset window.packed_f64_loop_fact_for_index(expr/index_get/foreign_counter.rs:76) declines every non-zero offset unlessallow_holes || window_validated:lower_packed_f64_range_versioned_for(stmt/loops.rs:5830) is the tier that does validate the whole offset window and publisheswindow_validated: true— but it accepts only a literal or loop-invariant local/global bound, and per its own call-site comment runs "only after thei < arr.lengthmatcher above declined."So
arr.lengthbound + offset access is covered by neither: the tier that understands the bound rejects the offset, and the tier that understands the offset rejects the bound.arr.lengthis the more natural way to write the loop, so the idiomatic form is the slow one.Fixture B's IR confirms it — the loop is claimed by
plen.fast/spec_public.fast, with no packed clone anywhere.Fix direction
The versioned matcher already hoists the length and has constant offsets in hand, so the offset window is provable where it stands: for constant
c,a[i+c]is in bounds wheni+c < lenandi+c >= 0. Either narrow the fast clone's iteration range to the intersected window and publishwindow_validated: true, or emit the window check in the preheader alongside the existing guard. Letting the range matcher accept anarr.lengthbound would also work but duplicates what the versioned tier already proves.Worth checking whether this is the whole of the
a[k ± c]gap tracked in #9258 — that investigation found the range matcher does admita[k + 8]on a literal-bound single-loop fixture, which is consistent with this: the shape only falls off the tier when the bound isarr.length.Method note
My first two attempts at this reproduced nothing, both because the fixture never reached the tier — a union-typed parameter pushed it onto
aidx.dynamic.fast, and the correct-looking timings meant nothing. Every number above is paired with a block-label count from the emitted IR (PERRY_LLVM_KEEP_IR=1, path on stderr) so "the tier fired" is checked rather than assumed.https://claude.ai/code/session_01Pcq6j6y57TdKSR2Zx2D187