perf(codegen): representation-selection Phase 1 — canonical unboxed i32 locals - #6903
Conversation
…32 locals
For a local proven I32/U32 (integer_locals incl. int-valued-TA locals,
unsigned_i32_locals, gated by the existing needs_i32_slot proof), the i32
slot becomes the CANONICAL AND ONLY storage: no double slot, no dual
writes, no shadow-stack GC binding. A boxed double is materialized
(sitofp/uitofp) only at genuinely-boxed use sites. Implements Phase 1 of
docs/representation-selection-rfc.md.
Mechanism (Phase 2 builds on this):
- SlotRep lattice seed {Boxed, I32, U32} + FnCtx.local_slot_reps rep map
(absent = Boxed); i32_counter_slots stays the single slot registry.
- Rep-aware access helpers in expr/slot_rep.rs: canonical_local_i32_slot,
load_canonical_local_boxed (materialize at boxed use sites),
store_canonical_local_from_double (NaN-safe toint32_wrap entry, keeps
the #6898 OOB-undefined trap closed).
- All local access routed: LocalGet/LocalSet/Update (literals_vars),
WithSet fallback (instance_misc1), SIMD channel writeback, class-capture
writeback, loop matcher gates + counter-slot lifecycle (loops.rs; the
arr.length-hoist path now reuses a pre-existing slot and only removes
what it inserted), guarded dynamic bounds, static i32 bounds.
- Eligibility exclusions (stay Boxed): closure-referenced locals, params,
async/generator/was_plain_async bodies, module init, boxed/TDZ locals.
- Strictness widening (flag-gated): a proven in-window const int-TA view
load (let l = P[0] with literal-length Int32Array) now counts as a
strict i32 write, the same judgment that already seeds integer_locals.
- Env flag PERRY_CANONICAL_I32_LOCALS (default on; 0/off/false restores
the parallel-shadow model), keyed into the object cache + key test.
PERRY_REPSEL_DEBUG=1 prints each canonical local at compile time.
Claude-Session: https://claude.ai/code/session_01UGDwjukzhowLDJYsMPFwMv
…y eligibility term The box slot.ts mix shape (let l = P[0] from an Int32Array PARAM, bitwise updates, observations only in ToInt32-coercing contexts) is admitted via int_valued_ta_locals (#6898) but is neither index-used nor strictly bounded, so the base i32-safe gate missed it. Retain the int-valued-TA subset as its own fact on RepresentationFacts and accept it as a canonical-only safety term: its whole-function observation proof makes canonical-i32 storage output-invariant under the NaN-safe entry conversion. The parallel-shadow gate (needs_i32_slot) is deliberately NOT widened — flag-off stays exactly the pre-phase model. Plus cargo fmt. Claude-Session: https://claude.ai/code/session_01UGDwjukzhowLDJYsMPFwMv
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughAdds Phase 1 canonical i32 storage for eligible locals, typed-array proof plumbing, representation-aware expression and loop lowering, closure handling, cache-key invalidation, debug controls, and a comprehensive gap test. ChangesCanonical i32 representation selection
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
Quiet-box measurements (M1 arm64, Structural acceptance —
enc.ts (typed
On the current toolchain the canonical model is wall-clock neutral (−1.8%) on this bench: #6898's parallel shadows + #6794's region privatization already kept enc_real.ts (real untyped bcryptjs, Node reference on this workload family: 133 ms ( |
…t undefined (#6961) inline_typed_array_fast_path_matches_slow_path asserted g(t,1.5) === g(t,1) — a deliberate encoding of the truncate-a-fractional-index quirk, as its own comment documented. Per 10.4.5.2 a non-integral canonical numeric index is not an own property, so [[Get]] yields undefined; #6903 (representation- selection Phase 1) made both the inline and slow paths spec-correct, so the pinned string stopped matching and cargo-test has been red on main since the Jul 28 nightly. The invariant the test protects (inline == slow) still holds — both paths return undefined for 1.5 / -0.5 / 2.7, on an inline-eligible receiver and under PERRY_TA_VIEW_GUARD, byte-identical to node. Correct the expectation, rewrite the comment to cite the spec + the commit that fixed the quirk, and add a B2 line asserting the values are === undefined: the old form only asserted 'differs from t[1]', which a regression returning some other wrong value would still pass. Test-only; no runtime or codegen changes. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Phase 1 of the representation-selection RFC (
docs/representation-selection-rfc.md, #6901): canonical unboxed i32 locals — kill the parallel-shadow model.What changes
Today a proven-integer local has a canonical NaN-boxed
doubleslot plus a parallel i32 shadow slot kept in sync by dual writes (needs_i32_slot,stmt/let_stmt.rs). Post--O3the loop-carried value stays aphi doublewith per-iterationfptosi/sitofpthat LLVM cannot remove — the canonical representation is still the box.This PR flips it: for a local whose representation is proven
I32/U32, the i32 slot is the canonical (and only) storage. No double slot, no dual writes, no shadow-stack GC binding (a number is never a pointer — the RFC's structural root-set reduction). A boxed double is materialized (sitofp/uitofp) only at genuinely-boxed use sites.Structural acceptance (quiet-box
slot.ts, post-opt -O3)phi doublephi i32fptosiin hot loopsitofpin hot loopThe mechanism (what Phase 2 builds on)
SlotRep { Boxed, I32, U32 }(expr/slot_rep.rs) — the seed of the RFC's representation lattice.FnCtx.local_slot_reps: HashMap<u32, SlotRep>maps LocalId → selected representation; absent =Boxed(exactly the pre-phase behavior).ctx.i32_counter_slotsremains the single slot registry for canonical and parallel-shadow slots — one source of truth.canonical_local_i32_slot(rep+slot query),load_canonical_local_boxed(materialize at a boxed use site),store_canonical_local_from_double(NaN-safetoint32_wrapentry conversion — the perf(codegen): native-i32 residency for int-typed-array-seeded locals (bcrypt _encipher) #6898 OOB-undefinedtrap stays closed).LocalGet/LocalSet/Update(literals_vars.rs),WithSetfallback (instance_misc1.rs), SIMD channel-reduction writeback (channel.rs), class-capture writeback (lower_call/capture_writeback.rs), loop matcher gates + the counter-slot lifecycle inloops.rs(thearr.length-hoist path now reuses a pre-existing slot and removes only what it inserted — previously it unconditionally dropped the Let-site slot at loop exit, which would have stranded a canonical local with no storage), guarded dynamic bounds, static i32 bounds,var-redeclaration reuse guard (let_stmt.rs), and the non-pointer-bits dataflow predicate (expr/helpers.rs).Eligibility (reuses the proven sets — no new analysis)
Decided at the
Stmt::Letsite: the existingneeds_i32_slotproof (integer_locals∪unsigned_i32_locals, restricted to index-used / strictly-i32-bounded / unsigned, init in range, not boxed, not module-global), plus one canonical-only term:int_valued_ta_localsmembers (#6898) qualify even when neither index-used nor strictly bounded — their whole-function proof (every write i32-producing or an int-kind TA read, every observation ToInt32-coercing) makes canonical storage output-invariant. Two flag-gated widenings, both off when the flag is off:let l = P[0]with a literal-lengthInt32Array) counts as a strict i32 write — the same judgment that already seedsinteger_locals;Excluded (stay Boxed), under-approximating freely: closure-referenced locals (capture machinery stays on the boxed protocol), params (ABI is boxed until Phase 2), async / generator /
was_plain_asyncbodies (shared-cell locals), module init, boxed/TDZ locals, flat-const row aliases.Range-soundness audit (the "verify before trusting" step)
strictly_i32_bounded_locals: greatest-fixpoint proof,++/--disqualifies — sound.unsigned_i32_locals: all writes top-level>>> 0,++disqualifies; reads materializeuitofp— sound.int_valued_ta_locals: writes i32-or-undefinedwith all observations ToInt32-coercing; NaN-safe seed — sound.integer_locals ∩ index_used_locals: admission acceptsAdd/Sub/Mulchains that could in principle exceed i32 — but under the shipped shadow model everyLocalGetof such a local already reads the i32 slot, so canonical storage preserves the byte-exact-validated semantics for exactly the same locals. No new overflow surface; no eligibility tightening needed (tightening to bitwise-only would drop the loop counters and index chains this phase exists for).Flag / cache
PERRY_CANONICAL_I32_LOCALS(default on;0/off/falserestores the parallel-shadow model), keyed into the object cache (object_cache.rs) + key test.PERRY_REPSEL_DEBUG=1prints one line per canonical local at compile time.Validation
slot.tspost-opt -O3— see table above (measured, not inferred; cache-busted between env arms).test_gap_int_valued_ta_locals/test_gap_ta_param_numeric_read/test_gap_typedarray_param_readbyte-exact under both flag arms.test_gap_repsel_canonical_i32.ts: boxed-consumer materialization after a mix loop, loop-counter shapes (hoistedarr.length, static bound, decrement, counter read after the loop), unsigned>>> 0local above 2^31 (uitofp), const Int32Array view mix, possibly-OOB int-TA seed (ToInt32(undefined)=0), hoistedvarredeclaration, postfix/prefix update values, closure-captured exclusion. Byte-exact flag on/off and underPERRY_GC_FORCE_EVACUATE=1.cargo test: perry-codegen lib green;native_proof_buffer_views::reassigned_typed_array_store_records_runtime_fallbackfails identically on pristinemain(pre-existing, nightly-only suite).https://claude.ai/code/session_01UGDwjukzhowLDJYsMPFwMv
Summary by CodeRabbit
New Features
Bug Fixes
Tests