Repository navigation
Split-K combine: correctness and wave parallelism - #534
Open
RichardChamberlain1 wants to merge 1 commit into
Open
RichardChamberlain1 wants to merge 1 commit into
RichardChamberlain1 wants to merge 1 commit into
Conversation
RichardChamberlain1
marked this pull request as ready for review
September 30, 2026 13:39
|
@q10 has imported this pull request. If you are a Meta employee, you can view this in D122658585. |
This branch has not been deployed
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.
Split out of #531.
What changes
Split-K combine, across both kernels that implement it. Everything later in the stack raises
partition counts, and both halves of this PR are defects or limits that only appear once those
counts are high.
Reduce kernel correctness
Three independent defects in the paged decode reduce kernel:
WARP_SIZElanes andonly masked afterwards. The
pm/psbuffers are builtmax_size=True, so nothing bounds-checksthem and the inactive lanes walk raw memory past the end — faulting only when the overrun happens
to clear the allocator's slab. The load index is now clamped to the last valid partition; the
existing
activemask still discards the value, so this costs nothing.partial_outholds the un-normalised numeratorsum(p*v), so the numerator weight iswalone andpart_sumbelongs only in the denominator.The slow path multiplied by
w * ls_v. The fast path was already correct.-inf, so the merge computesexp(-inf - -inf). The global max is now floored at a large finite negative sentinel, whichyields weight 0 and can never win the max against real data.
The slow path also staged stats in LDS behind a
p < _MAX_PARTSbranch and sized the LDS arrays tothe exact partition count. They are now padded to a whole number of waves so every lane has a slot
and the staging loop needs no bound branch; the padded slots are never read back, since the
accumulation loops are
range_constexprover_MAX_PARTS.AOT_CONFIGSis widened tomax_parts128 and 256 to match
pa_decode_dense._SPLIT_KS.Combine parallelism
The split-K combine in the shared
DualwaveSplitKCombinehelper — used by both the generic and thegfx950 dualwave kernels — became the bottleneck as split counts grew. Three changes:
combine_chunk(HEAD_DIM)head-dim values, so a row needs onlyHEAD_DIM/chunklanes. Giving the row a whole wave leaves64/lanes_per_rowgroups to divide thesplit dimension between — the only axis with work left, since every lane used to walk all
NUM_KV_SPLITSpartials itself and that walk is where the time goes. The groups are contiguouslane ranges, so the cross-group merge is an in-wave XOR butterfly with no LDS and no barrier. Head
dims that cannot form power-of-two lane groups (96, for instance) keep the ungrouped layout —
correct, just without the extra parallelism.
mandlup front and keeping them livecosts
2 * NUM_KV_SPLITSregisters and spills hard past roughly a hundred splits, with combinecost growing superlinearly while the attention kernel feeding it kept getting faster. Streaming
the max and re-reading
m/lduring accumulation keeps the live setO(chunk), at one extra4-byte read per split.
outside any
scf.if. The per-split descriptor setup put a 4-SGPR sequence between consecutiveloads, so they could not be in flight together and the combine paid a full memory latency per
split — linear in split count, and enough to exceed the attention kernel itself on a
low-concurrency decode.
FLYDSL_COMBINE_BLOCKandFLYDSL_COMBINE_PREFETCHexpose the block size and the loads-in-flightdepth. The block is clamped to whole waves when split groups are in use, since a partial wave would
silently drop split groups.
A note on the sentinel: an unowned split uses a large finite negative value rather than
-inf,because a real
-infmakes an all-empty group's merge compute(-inf) - (-inf) = NaNand poisonevery group it then merges with.
Global→LDS copy wrappers
The direct
rocdl.s_waitcnt/rocdl.raw_ptr_buffer_load_ldscalls in the generic kernel arereplaced by two thin wrappers,
wait_lds_copiesand_buffer_load_to_lds. Both keep the existingbehaviour on the default path and add an opt-in async variant behind
FLYDSL_FLASH_ATTN_ASYNC_LDS, which selectsbuffer_load ... ldson the asynccnt counter insteadof the vmcnt-tracked form. Same operands, different completion counter, so the swap is mechanical —
opt-in until it is proven on this kernel.
This sits here because the wrappers are in the same two files as the combine rework and everything
later in the stack calls
wait_lds_copiesunconditionally.Why it is separable
The reduce fixes and the combine rework touch disjoint files —
decode/pa_decode_reduce.pyagainstflash_attn_{utils,generic,gfx950}.py— and no other PR in the stack touches either set. Thecombine rework changes only how partials are merged, not what is produced, and the copy wrappers
preserve the existing default path exactly. All three are prerequisites for everything downstream
that raises split counts.
Merge this before PR
5, which drives the reduce slow path fixed here.Verification
The project's lint checks pass.
No dedicated test for either half; both are covered incidentally by the split-K tests. On a
supported device, run
test/attention/flydsl/flash_attn_paged_splitk_test.pyin full at the top ofthe stack. A direct unit test for the reduce kernel would be a worthwhile addition.