perf(codegen): offset array reads earn the packed clone's numeric proof (41 → 16 ms)

[proggeramlug]

The second of the two defects behind #9259. @ECS1's #9274 fixes the bound gap; this fixes the cap that sits under it. Independent changes, different files, additive.

The defect

accumulator_rhs_is_numeric's IndexGet arm (stable_packed_accumulator.rs:45) required a bare Expr::LocalGet index:

Expr::IndexGet { object, index } => matches!(
    (object.as_ref(), index.as_ref()),
    (Expr::LocalGet(a), Expr::LocalGet(i)) if *a == array_id && *i == counter_id
),

So a[k - 1] is not numeric → the accumulator never earns its number proof → every + in the enclosing expression lowers to a tag-test diamond over js_dynamic_string_or_number_add. The hot loop is 47 instructions where the plain form is 11, and the reads are already hoisted — it is the add, not the array access:

and  x8, x19, #0xffff000000000000   ; three is_number tag tests
cset w8, hi
...
fadd d0, d1, d0                      ; hot arm
b    <merge>
bl   _js_dynamic_string_or_number_add ; cold arm, twice

This is the cost #9060 and #9091 already removed for the bare-counter form.

Numbers

Quiet host, k < 4096, s = s + a[k] + a[k-1], 4096-element number[], 2000 reps:

	before	after	node
s += a[k] + a[k-1]	41 ms	16 ms	8

Nothing else moves — plain-index rows stay at 7–8 ms, and the arr.length-bounded row stays at ~100 ms because it falls off both tiers for the separate reason #9274 fixes. Every timing was taken with guard-call counts from the full function disassembly beside it, so "the code under test ran" is evidence rather than assumption.

Threaded per tier, not widened

collect_numeric_accumulators takes offset_reads_inlined from each admission site:

• range tier → true. Publishes window_validated, so its guard proved the whole window; its hole-tolerant loads side-exit before producing a value. An offset read is lowered inline and yields a Number.
• versioned and stable-packed → false. Their offset reads take the generic path, which can produce undefined. Admitting that as numeric is a wrong answer, not a missed optimisation.

@ECS1 corrected my original reasoning here — I had the two tiers' guarantees inverted, and on my own analysis I would have set the flag on the tier that cannot inline offset reads.

What the tests guard, and what they do not

They cover the correctness of the shape this admits: an index running off either end, a hole inside the window, a non-numeric element, and a leading string that must concatenate rather than add — which is precisely what a wrongly granted numeric proof turns into a native fadd. All six node-identical.

They do not guard the per-tier flag, and I checked rather than assumed. I flipped the versioned tier to true deliberately; all six still passed. No loop reaches that tier with an offset read today — such a loop falls off both tiers (#9259) — so the flag is currently unobservable in either position. It becomes live when #9274 merges, and flipping it then requires an arr.length-bounded fixture to be testable at all. I have left that in a comment at the call site rather than as folklore.

Validation

• perry-codegen 1843/0, perry-hir 596/0, -D warnings 0, local-binding-type audit OK
• 34 packed-loop integration tests across 5 files (packed_loop_abrupt_statements, loop_property_array_hoist, issue_8690_loop_versioned_arraylike, issue_8897_field_push_writeback, plus the 6 added)
• 7 adversarial differentials against node before writing any test

Refs #9259, #9274, #9060, #9091.

Summary by CodeRabbit

• Bug Fixes

  • Improved numeric handling for offset array reads in packed loops.
  • Preserved correct behavior for out-of-range indexes, sparse arrays, non-numeric values, and string concatenation.
  • Prevented invalid fast-path numeric assumptions that could produce incorrect results.

• Tests

  • Added coverage for offset reads, numeric accumulation, and edge cases producing NaN or string results.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1e583ce8-83fb-4198-98ce-193353d82fcf

📥 Commits

Reviewing files that changed from the base of the PR and between 6c880be and 35d29de.

📒 Files selected for processing (4)

• crates/perry-codegen/src/stmt/loops.rs
• crates/perry-codegen/src/stmt/stable_packed_accumulator.rs
• crates/perry-codegen/src/stmt/stable_packed_loop.rs
• crates/perry/tests/packed_loop_offset_read_accumulator.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The compiler now passes an offset_reads_inlined flag through packed accumulator analysis. Range loops enable offset-read numeric admission; versioned and stable-packed loops keep it disabled. New tests validate sums and non-numeric offset-read behavior.

Changes

Packed offset-read accumulator admission

Layer / File(s)	Summary
Offset-read numeric analysis crates/perry-codegen/src/stmt/stable_packed_accumulator.rs	Accumulator analysis admits array[counter ± constant] only when inline lowering is guaranteed. Recursive expression checks preserve this condition.
Loop-tier admission wiring crates/perry-codegen/src/stmt/loops.rs, crates/perry-codegen/src/stmt/stable_packed_loop.rs	The range tier passes true. Versioned and stable-packed tiers pass false.
Offset-read runtime validation crates/perry/tests/packed_loop_offset_read_accumulator.rs	Tests cover valid sums, bounds errors, holes, non-numeric elements, and string concatenation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 35d29

This optimization admits offset array reads into the numeric fast path only for validated range loops, improving performance while retaining fallback behavior elsewhere. It is mergeable with owner awareness that future loop-tier changes must preserve the same bounds and hole-handling guarantees, since breaking that contract could produce incorrect numeric results.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main code-generation optimization for offset array reads and includes the measured performance improvement.
Description check	✅ Passed	The description provides a detailed summary, technical rationale, related issue references, test coverage, validation results, performance data, and test limitations. It does not use the template head…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description provides a detailed summary, technical rationale, related issue references, test coverage, validation results, performance data, and test limitations. It does not use the template headings or checklist, but it includes the required substantive information and is mostly complete.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Merged.

Validated on a shared branch with #9228, #9257, #9263, #9271, #9272, #9274, #9277, #9279 and #9280 — one build, one validation pass, then split back out and merged individually.

Results across the batch:

• perry-runtime 2885 passed / 0 failed at RUST_TEST_THREADS=1
• perry-codegen 30 suites green (the one failure was a doc-test reporting a missing libperry_codegen-*.rlib — an artifact of my own cleanup of stale build dirs, confirmed by a clean re-run at 31/31, not a code defect)
• all 60 lint gates plus the check_thread_locals / tls_budget checkers
• a nine-row differential probe byte-identical to node 26.5.1, covering every changed area: RegExp \w/\b/. ASCII and LineTerminator semantics, offset (a[i±1]) and length-bounded array reads, the assert RegExp matcher, and closure identity across a 40k-allocation GC churn
• seven earlier regression probes re-run at zero diff lines: tagged and scalar array stores, pointer↔scalar transition churn, growth-forwarding receivers, BigInt negation, iterator protocols, field shadowing

One probe (protorepl) moved from 2 to 4 diff lines and I ran it down rather than waving it through: both divergences are accepted trades already on main — the fresh-instance case (#9239) and #9247's deliberate change of a custom-chain miss from Some(undefined) to None, which it made because swallowing the miss left everything Perry synthesizes unreachable (Object(true).valueOf(), plain-function .prototype, iterator helpers). Neither is anything in this batch.