fix(codegen): an integer literal past 2^31 must not seed the i32 fast path (#6319)#6340
Merged
Merged
Conversation
… path (#6319) Third face of #6072 (`i32 fast-path locals silently wrap at 2^31`), reached through the *copy* path rather than `x++` (#6258) or a dynamic loop bound (#6318): let x = 3000000000; let y = 0; y = x; console.log(y); // node: 3000000000 perry: -1294967296 `Expr::Integer` carries an `i64`, but every judgment in the i32 chain accepted `Expr::Integer(_)` regardless of magnitude. `x` itself stayed correct — its own `Let`-site slot is gated on `init_in_i32_range` — yet it was still admitted to `integer_locals`. `is_strictly_i32_bounded_expr`'s `LocalGet` arm then answered `known_int_locals.contains(id)`, so `y = x` counted as a strictly-i32-bounded write, `y` got an i32 shadow at its `Let` site, and the store truncated the 3e9 double. Reads prefer the shadow (issue #48), so that is what `console.log` saw. Denying the shadow is the only cure: the f64 fallback in `LocalSet` mirrors into the i32 slot with `fptosi`+`trunc` anyway (literals_vars.rs:634), so rejecting the value in `can_lower_expr_as_i32` would not have helped. Narrow all four judgments through one shared predicate, `integer_literal_fits_i32`: * `collect_integer_let_ids` — the syntactic seed * `is_int32_producing_expr` — the forward-closure admission * `int32_producing_deps` — the disqualification judgment * `is_strictly_i32_bounded_expr` — the write-is-i32-bounded proof All four are load-bearing. Narrowing only the seed does not hold: the forward closure re-admits from the `Let` init and the judgment re-admits from every `LocalSet` rhs. And `is_strictly_i32_bounded_expr` is an independent hole — `let y = 0; y = 3000000000;` wrapped without any copy at all. Two latent poison `fptosi`s go with it: the `arr.length`-hoist (loops.rs:2454) and the static `i < n` bound (loops.rs:2516) both install a counter's i32 shadow from `integer_locals` membership alone, seeding it with a bare `fptosi` of the counter's double — poison in LLVM for a counter past 2^31. Bitwise-coerced constants are untouched, so the imul32/FNV i32 chain that `compiler-output-regression` watches is unaffected: image_conv's two out-of-range constants are written `0x9E3779B9 >>> 0` and `0x811c9dc5 | 0`, which reach `integer_locals` through the `>>> 0` / `| 0` arms, not the bare `Expr::Integer` arm. `| 0`, `>>> 0`, bitwise ops and `Math.imul` genuinely produce int32 by spec; a bare literal does not.
|
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 (3)
📝 WalkthroughWalkthroughThe i32 fast path now admits integer literals only when they fit in signed 32-bit range. Integer-local classification, strict boundedness checks, and regression scenarios for large literal magnitudes were updated. Changesi32 literal range validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
This was referenced Jul 13, 2026
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6319 — the third face of #6072 (
i32 fast-path locals silently wrap at 2^31), after #6258 (barex++accumulator) and #6318 (dynamic-bound loop counter).Root cause
Expr::Integercarries ani64— every integral literal that fits in i64 lowers into it, including3000000000,4294967296andNumber.MAX_SAFE_INTEGER(perry-hir/src/lower_patterns.rs:143). But every judgment in the i32 fast-path chain acceptedExpr::Integer(_)regardless of magnitude.xitself stayed correct — its ownLet-site slot is gated oninit_in_i32_range— yet it was still admitted tointeger_locals.is_strictly_i32_bounded_expr'sLocalGetarm then answersknown_int_locals.contains(id), soy = xcounted as a strictly-i32-bounded write,ygot an i32 shadow at itsLetsite, and the store truncated the 3e9 double. Reads prefer the shadow (issue #48), so the wrapped value is whatconsole.logsees.Denying the shadow is the only cure. The f64 fallback in
LocalSetmirrors into the i32 slot withfptosi+truncanyway (literals_vars.rs:634), so rejecting the value incan_lower_expr_as_i32would not have helped — the truncation happens on both paths.The fix
One shared predicate,
integer_literal_fits_i32, applied at all four judgments that make up the seeding chain:collect_integer_let_idsis_int32_producing_exprint32_producing_depsis_strictly_i32_bounded_exprAll four are load-bearing. Narrowing only the seed (the issue's sketch) does not hold: the forward-closure pass re-admits from the
Letinit, and the disqualification judgment re-admits from everyLocalSetrhs. Andis_strictly_i32_bounded_expris an independent hole —let y = 0; y = 3000000000;wrapped with no copy at all.Two latent poison
fptosis go with it: thearr.length-hoist (loops.rs:2454) and the statici < nbound (loops.rs:2516) both install a counter's i32 shadow frominteger_localsmembership alone, seeding it with a barefptosiof the counter's double — poison in LLVM for a counter past 2^31.Why the imul32 / FNV chain is safe
The
compiler-output-regressiongate watches image_convolution's i32 chain, whose two out-of-i32-range constants are written— they reach
integer_localsthrough the>>> 0/| 0arms, not the bareExpr::Integerarm this PR narrows.| 0,>>> 0, bitwise ops andMath.imulgenuinely produce int32 by spec; a bare literal does not. A sweep of everysourceinbenchmarks/compiler_output/workloads.tomlfound no other >i32 literal.Magnitude matrix (vs
node --experimental-strip-types)Each value is copied through a fresh local whose own
Letinit is in range, so theinit_in_i32_rangegate cannot mask the bug.2^31 - 12^312^31 + 12^32-2^31-2^31 - 1MAX_SAFE_INTEGERy = x)d = 3000000000)a → b → c)n = lim; n > 2^31-1)The
comparerow is worth calling out:const lim = 3000000000; let n = 0; n = lim; n > 2147483647evaluated tofalseonmain.Fast paths preserved (IR evidence)
--trace llvmon the new fixture, before vs after:js_typed_feedback_numeric_array_index_get_guard(array fast path, #6299/#6312)mul i32(FNV / imul32 chain)alloca i32(i32 shadows)store i32 2147483647(2^31-1 keeps its shadow)store i32 -2147483648(-2^31 keeps its shadow)Exactly the intended precision: the boundary literals that do fit keep the i32 fast path; only the ones that don't drop off it.
Validation
compiler-output-regression: all five CI gate steps green locally (harness unit tests,native-region-proof×11 workloads,native-abi-proof×12,vectorized_buffer_transform,hir_fact_rewrite,fma_contract) —failed_workloads: [].test_gap_6072_i32_update_overflowandtest_gap_6072_dynamic_bound_counterstay byte-identical to node.origin/mainbuild: zero regressions. The only output changes are the new fixture andtest_gap_console_methods(console.timejitter, e.g.0.001ms→0.003ms).test-files/test_gap_6319_i32_literal_magnitude.ts, byte-identical to node.cargo fmt --all -- --checkandscripts/check_file_size.shclean.Known adjacent face — not this PR
Copying a local whose value came from an arithmetic chain past 2^31 still wraps (
let big2 = big1 + big1; let t = 0; t = big2;→-294967296). That is a different root cause:is_strictly_i32_bounded_expr'sLocalGetarm readsinteger_locals, which by design admits overflowingAdd/Sub/Mul— it is not a literal-magnitude problem, and closing it means turning the strict set into a greatest fixpoint. Filed separately as #6339 with the repro and a fix sketch.Summary by CodeRabbit
Bug Fixes
Tests