[Kernel] Fix the RDNA3 GEMM grid swizzle and migrate it to the tiled copy/MMA API - #979
Merged
Merged
Conversation
Replaces the hand-rolled index arithmetic in rdna3_f16_gemm with the layout-based API, and fixes a latent grid-swizzle fault found on the way. The epilogue now partitions C through a tiled_mma and make_tiled_copy_C instead of an explicit ``g_row = base + 2*si + klane`` store loop. The tiled_mma carries a permutation that reproduces the kernel's wave banding: the default stamping interleaves the repeats instead, which measured 62% slower at 3072x3072x1024. The GMEM to LDS path now goes through flat_divide plus a tiled copy with BufferCopy128b, replacing the precomputed offset tables and the flat _v8_store. The thread geometry is the same assignment the tables computed. The grid swizzle moves out of the kernel body into _group_width and _swizzle_tile_id. It derived bid_m from a group width that need not divide grid_m, so the last group addressed tiles past the end of the grid and the kernel wrote past C. Every shape in use happened to divide evenly, which is why it survived; it becomes reachable as soon as a caller asks for a tile narrower than 128x128. The block tile stays a parameter defaulting to 128x128x32, so this is a refactor: the generated ISA is byte-identical for rn and rs output across 128x128x32, 64x64x64 and 128x64x32 on gfx1100. Adds test_rdna3_wmma_atom for the gfx11 tiled copy atoms, which have to handle lanes 16-31 mirroring lanes 0-15 under the v16 operand ABI, and test_rdna3_grid_swizzle for the swizzle bijection. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
Follow-up #980 stacks on this branch and adds shape-driven tile selection on top. It targets |
Benchmarking the migration against the unmodified kernel turned up that the swizzle bug is reachable at the default 128x128 tile, not only at narrower ones as the docstrings claimed, and that it has two symptoms rather than one. Measured on gfx1100 at 128x128x32: M of 1152, 1280 and 1664 return a wrong C, off by roughly 400x the bf16 rounding floor, while 1536 and 2560 fault the GPU. Which one you get depends on whether the address past the grid happens to be mapped, so the silent wrong answer is the common case and the fault is the lucky one. No behaviour change -- the fix itself already landed with the swizzle rewrite. This corrects the three places that understated it and adds grid_m of 9, 10 and 13 to the bijection test so the wrong-answer grids are covered alongside the two that faulted. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
Looks good. But style check failed. @vlluvia |
The repo formats at 120 columns; these files were wrapped nearer 100, so black wanted to rejoin several call arguments and expand the parametrize lists to one tuple per line. Formatting only, no behaviour change. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
Sorry, that’s on me. I’ll get the style check fixed. |
coderfeli
approved these changes
Aug 8, 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.
The part that matters most
rdna3_f16_gemmcomputes a wrong C, or faults the GPU, at ordinary shapes on today'smain.The grid swizzle derives
bid_mfrom a fixed group width of 8 that need not dividegrid_m, so the trailing partial group addresses tiles past the end of the grid. At the default 128x128x32 tile on gfx1100, bf16 in and out:grid_mmainK is 1024 throughout; each shape ran in its own process so a fault would not take the sweep with it. A correct bf16 result lands at 0.0039, so the failures are ~400x the rounding floor rather than a precision artifact. The damage is always confined to output tile rows at index 8 and above — the remainder group — and never touches the first eight, which is the swizzle's signature. Whether a given shape produces a wrong answer or a hard fault depends on whether the out-of-bounds address happens to be mapped: 1536 faulted in one harness and returned wrong numbers in another. The silent wrong answer is the common case.
_group_widthnow snaps the width down to the largest divisor ofgrid_mwithin the cap, which restores the bijection. Every shape above is correct after this PR.The rest: migration to the layout API
Replaces the hand-rolled index arithmetic, which is how the swizzle bug surfaced.
Epilogue. C is now partitioned through a
tiled_mmaandmake_tiled_copy_Cinstead of an explicitg_row = base + 2*si + klanestore loop. Thetiled_mmacarries a permutation that reproduces the kernel's wave banding — the default stamping interleaves the repeats instead, which measured 62% slower at 3072x3072x1024, so the permutation is load-bearing rather than cosmetic.GMEM to LDS. Now goes through
flat_divideplus a tiled copy withBufferCopy128b, replacing the precomputed offset tables and the flat_v8_storehelper. The thread geometry is the same assignment the tables computed.Grid swizzle. Lifted out of the kernel body into
_group_widthand_swizzle_tile_id, which is what makes it unit-testable without a GPU.Performance: unchanged, by construction
The block tile stays a parameter defaulting to 128x128x32, and the generated ISA is byte-identical for
rnandrsoutput across 128x128x32, 64x64x64 and 128x64x32 on gfx1100.Measured anyway, against the unmodified kernel over 16 shapes where
mainis correct enough to time: geomean 1.002x, spread 0.978x to 1.034x. Since both sides run identical machine code, that spread is the harness noise floor rather than an effect.Tests
test_rdna_gemm— device-level regression atgrid_mof 3, 9 and 20, with anassert grid_m % 8guard so the shapes cannot silently stop covering the bug.test_rdna3_grid_swizzle— the bijection, over the grids that faulted (12, 20) and the grids that returned wrong answers (9, 10, 13). No GPU needed.test_rdna3_wmma_atom— the gfx11 tiled copy atoms, which have to handle lanes 16-31 mirroring lanes 0-15 under the v16 operand ABI.Full suite on gfx1100 (Radeon Pro W7900): 1609 passed, 0 failed.