Repository navigation
feat(unroll): recognize range for loops - #1346
Conversation
77ff0d4 to
77ed9ed
Compare
…r) — 87 counter `while` loops with an unroll marker in linear/{delta,conv,norm_gate}, flash_gqa, flash_gqa_prefill, flash and rope_neox become `for i in 0..N` with the same marker (ledger #31 closed by NVIDIA/cuda-rust#1346 in the pin); the prefill-256 body adds the warp-half offset into its K/V bases once before the loop, so the per-step ldmatrix offsets stay immediates (ptxas 250 regs, predicted 190–200), and its exchange add is one add (IEEE add commutes); gpu-deepseek41 takes `expf_ik` from bloomery_gpu::linear
`needless_range_loop` is allowed per fn with its reason: an iterator loop is not unrolled and keeps its array in a local depot. sihyung: check rc 0, clippy 48, fmt ok, gpu-gates lib 43 passed. Box-ready: ptx-scan of generate_ds41, gate_e2e, gate_linear and gate_qwen35moe_attn predicted identical instructions everywhere except gqa_prefill_flash_256; gate-gpu-linear, gate-gpu-qwen35moe-attn and gate-ptx-spill predicted green (not yet run).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T9xdyin8HGq4ZLiEreJzey
`#[unroll]` on `for i in a..b` did not unroll: the loop exits through a match on the `Option` from the inlined `Range::next`, so the counter is not an induction variable. Rewrite that match into a header test before the unroll analysis. No block is copied, and loops whose header already exits are unchanged. Signed-off-by: midagedev <midagedev@gmail.com>
77ed9ed to
5a59773
Compare
|
I tested this PR on the real-use question from #559 and found one gap: range loops with signed counters, including the default Setup: PR head Real use: a Q4_K prefill MMA tileThe kernel is a 128×128 int8
Timing covers the tile plus its activation-quantization kernel: 40 launches per CUDA graph after warmup, 8 samples per shape, with builds interleaved. Medians:
*In those runs the paired native CUDA kernel was also about 10% slower. Against it, the ratio is 0.79 versus 0.76, so the difference is about +3.5%. On Gap: signed counters (
|
|
Thank you for testing this on a real kernel, and for finding the cause. I can reproduce the shape from your dump: signed |
core steps a signed counter through checked_add (checked_sub for .rev()), with the tuple in a stack slot and a branch on the overflow flag before the arm's goto. When the header tests lo < hi and the step is one, the step cannot overflow and the overflow edge leaves the loop, so fold the step to a plain add or sub and the branch to a goto. The next round sees the unsigned shape. An inferred-i32 counter (used only through `as`) now unrolls: the tester's range_i32 kernel goes from 2 warnings, 18 ld/st.local and 3 branches to 0, 0 and 0, and its PTX equals the usize form. unroll_smoke's existing kernels keep their PTX. Signed-off-by: midagedev <midagedev@gmail.com>
|
Pushed 20e30b6. Thank you for the clear report and the reproducer. The cause was as you described: core's signed step goes through Your The missing source span in the warning is a separate change; I left it out of this PR. |
|
Thanks, confirmed. Setup: both PR commits cherry-picked cleanly onto
Both range kernels now have the same instruction counts as the counted- Prepared with help from an AI assistant (Claude Code). |
|
Thanks for the PR, it looks good: no soundness problems but three things before it lands:
Also merge main in (conflict free): the branch pins nightly-2026-08-28 and CI runs on the head sha, so today it would test the wrong toolchain. In short, to land this: we'll need the importer no-op cast fix with |
nihalpasham
left a comment
There was a problem hiding this comment.
Reviewed 20e30b60: changes requested. The details and the list to land this are in my comment above: #1346 (comment)
Signed-off-by: midagedev <midagedev@gmail.com>
A cast whose source and target lower to the same dialect type returns the operand and emits nothing, as the reborrow path already does. core steps isize through i64 and back; those casts were hiding the checked step from the unroller. unroll_smoke adds a 16x16 nest over a 16-lane accumulator. A runtime-bounded mix in the inner range is what this LLVM leaves looped: 17 ld.local, 17 st.local, and a 64-byte local depot without #[unroll], and none with it. The same example covers isize a..b, .rev(), and #[unroll(4)]. verify-code-shape.sh rejects .local in those kernels. Docs list the same forms, isize included. An unrecognized range step is no longer described as not a range loop. Signed-off-by: midagedev <midagedev@gmail.com>
|
Thanks for the review. All four are done on
Ran |
|
/ok to test 4d9b829 |
The Python regex used a POSIX character class, which re does not support, so the .local depot alternative never matched and every run printed a FutureWarning. Use \s with the multiline flag so a `.local .align` depot line fails the gate as the comment promises. Signed-off-by: nihalpasham <nihalp@nvidia.com>
|
/ok to test 660873c |
|
Checked at
#1494 (const-generic bounds) and #1478 (clone budget) stay open; neither path is touched here. |
What this adds
#[unroll]onfor i in a..bandfor i in (a..b).rev()now unrolls. After inlining, such a loop exits through a match on theOptionreturned byRange::next, so the unroll pass found no header exit test and no induction variable; arrays indexed byistayed in local memory. A new dialect-mir canonicalization threads that match into a header exit test, and the signed-counterchecked_add/checked_substep core emits is folded to a plain add/sub where the header test proves it cannot overflow. Casts between identical dialect types (isize/i64,usize/u64) are elided in the importer soisizeloops look likei64loops.What we fixed
forloops over any integer counter, includingisize, with full#[unroll]and partial#[unroll(N)], and(a..b).rev()with full unroll.checked_addoverflow branch is dead under the header's strictlo < hitest and is folded away; the fold refuses any other step or an overflow edge that re-enters the loop.a..=b,.step_by(k)and custom iterators still warn and stay loops; the warning text now distinguishes an unrecognized step from a loop that is not ana..brange.Implementation
canonicalize.rs:thread_iterator_exit_testruns only for a hinted loop whose headercond_brhas both successors inside the loop. It requires single-entry arms ending ingototo a two-predecessor join that reads the discriminant of one argument, each arm building its value withmir.construct_enum, and evaluates the match blocks with a small evaluator that accepts only storage markers, constants, the discriminant read,IntToIntcasts andeq/ne. Replacements are planned before any mutation; a match value used where both targets reach is refused. Dead blocks are removed by the nextsimplify_cfground inunroll.rs.canonicalize.rs:fold_checked_stepmatches core'schecked_add/checked_subby constant 1 on the counter the header orders strictly, with the overflow edge leaving the loop.analyses/induction.rs:match_cmp,unwrap_not,const_i128andCmpPred::negatemadepub(crate)for reuse.mir-importertranslator/rvalue/expr.rs: a cast whose operand type equals the result type returns the operand and emits nothing, beside the existing reborrow shortcut. Pointer-kind, signedness and width changes still emitmir.cast.unroll_smoke: new kernelsrange_for_loops,signed_range_for_loops,signed_partial_range_for,isize_range_for,nested_lane_range;verify-code-shape.shfails the build whennested_lane_rangeorisize_range_foruses local memory.nested_lane_rangekeeps a runtime innerwhileso LLVM cannot unroll it alone; it is the discriminating gate.isize_range_foris a control, since LLVM already unrolls it on main.#[unroll]macro docs,thread.rs, supported-features and the three book pages list the same supported forms.Verification
Checked at
4d9b8295:cargo test -p mir-transforms: 91 passed. New tests fail on main; i8/u8 sweeps over every boundary pair, interpreter over every end value for factors 2, 3 and 4, negative cases for a re-entering overflow edge, a step of 2 and a value read after both exits.just test: 3986 passed;just bookbuilds; fmt, clippy, rustdoc and all 18check-*.shguards pass;sync-example-locks.sh --checkOK after merging current main.cargo oxide run unroll_smokeon RTX 5090 (sm_120), LLVM and libNVVM routes: PASS; memcheck and synccheck 0 errors.nested_lane_rangewith main's backend: 17ld.local, 17st.local, 64-byte depot; here 0/0/none. The 17 pre-existing kernels are instruction-identical to main.isizebounds,rev(), partial remainders for severaln,breakwith value,continue, runtime bounds, mixed casts; all correct, 0 local memory, memcheck 0 errors.Caveats
#[unroll(N)]on(a..b).rev()or on a loop with a runtime start still warns, as it does for countedwhileloops.a..=band.step_by(k)are not recognized.nested_lane_rangein the compile-only CI lane would surface.#[unroll(N)]still emits no loop metadata ([BUG]: #[unroll(N)] emits no loop metadata, so the NVVM backend unrolls the loops a second time #1375), so NVVM may unroll the remainder again.#[unroll]rejects a loop bound computed from a const generic (N / 8) #1494 (const-generic bounds) and #[unroll] over the clone budget leaves an array-indexing loop rolled and moves the array to .local, with no way to raise the budget #1478 (clone budget) are not addressed.