[Perf] Enable the XCD swizzle and pin the softmax basis for gfx950 bf16 attention - #1009
[Perf] Enable the XCD swizzle and pin the softmax basis for gfx950 bf16 attention#1009JohnQinAMD wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR wires up the existing gfx950 “XCD head-slow remap” (XCD swizzle) to the bf16/f16 dense DUALWAVE_SWP flash-attention path so large-sequence throughput doesn’t degrade due to poor L2 residency across XCDs.
Changes:
- Plumbs an
xcd_swizzlebuild flag through the dense dualwave builder and exposes it viaflydsl_flash_attn_func(..., dualwave_swp_xcd_swizzle=...)(auto/force on/off). - Adds auto-dispatch logic for enabling the remap on eligible dense shapes (mirroring the existing fp8 behavior).
- Adds gfx950 tests to assert bit-identical output when swizzle is toggled and to validate correctness when heads don’t divide the XCD count.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/kernels/test_flash_attn_fwd.py | Adds tests for bit-identical output and correctness around the XCD swizzle option. |
| kernels/attention/flash_attn_interface.py | Exposes dualwave_swp_xcd_swizzle, computes auto-dispatch condition, and passes the flag into the cached dualwave builder. |
| kernels/attention/flash_attn_gfx950.py | Adds a private _xcd_swizzle build parameter and forwards it into traits creation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| outs = [ | ||
| flydsl_flash_attn_func(q, k, v, causal=False, dualwave_swp_xcd_swizzle=flag).clone() for flag in (False, True) | ||
| ] |
There was a problem hiding this comment.
The single-line form was Black's own output at line-length 120 — it collapsed the multi-line comprehension I originally wrote, and black --check passed on it. So the formatting gate was not at risk.
The readability point stands, though. Replaced the comprehension with an explicit local run(flag) helper, which reads better and is stable under Black.
| """H % 8 != 0 must fall back rather than mis-map. | ||
|
|
||
| The remap divides the linear workgroup id by the q-block count to recover | ||
| the head, which only lands each head on one XCD when the head count divides | ||
| evenly into the 8 XCDs. The guard is in the dispatch condition, so this | ||
| checks the kernel still produces a correct answer for such a shape. | ||
| """ |
There was a problem hiding this comment.
Good catch — the docstring was narrower than the actual guarantee. There are two guards: the dispatch condition auto-selects against H % NUM_XCD_GFX950 != 0, and _init_dualwave_thread_mapping re-checks it independently, which is what makes forcing the flag safe. Docstring now states both.
| out = flydsl_flash_attn_func(q, k, v, causal=False) | ||
| torch.cuda.synchronize() | ||
| ref = F.scaled_dot_product_attention( | ||
| q.transpose(1, 2).float(), k.transpose(1, 2).float(), v.transpose(1, 2).float() | ||
| ).transpose(1, 2) | ||
| torch.testing.assert_close(out.float(), ref, atol=_ATOL_BF16, rtol=0) |
There was a problem hiding this comment.
Agreed, and it is exactly the claim the PR description makes, so it should be pinned. The test is now parametrized over dualwave_swp_xcd_swizzle in [None, True], so the forced-True case at H=12 is covered alongside the auto-selected one. Both must still match SDPA.
6c5dd25 to
3d0e4b9
Compare
3d0e4b9 to
6162271
Compare
The bf16 DUALWAVE_SWP dense kernel loses throughput as the sequence
grows, while aiter stays flat. The fix already existed:
_init_dualwave_thread_mapping re-derives (head, q_block) with head as the
slow axis, and _make_dualwave_swp_traits already accepted xcd_swizzle;
only the fp8 builder ever passed it, gated on dtype_str == "fp8". The
string xcd did not appear in flash_attn_gfx950.py at all.
This plumbs the flag through the bf16 builder and reuses the fp8 dispatch
condition (non-causal, H % NUM_XCD_GFX950 == 0, num_q_blocks >=
MIN_Q_BLOCKS_XCD_SWIZZLE). Split-K is excluded structurally: it takes the
_build_splitk branch above this dispatch site.
Mechanism: workgroups map to XCDs as linear_id % 8, and linear_id is
bx + by*H + bz*H*nqb, so with H % 8 == 0 a head-fast grid pins head h to
XCD h % 8. The ~256 resident workgroups span all H heads within one
batch, so each XCD juggles H/8 independent K/V streams against its L2
slice; the remap puts the resident window inside a single head, leaving
one. Measured across eight (B,H) combinations at S=180,180, the penalty
for leaving the remap off tracks H/8 -- -4.8/-6.1% at 8 streams, -3.3 to
-5.5% at 4, and nil at 2 or 1 -- while tracking neither the L2 hit rate
(H=64 has a better hit rate than H=32 and a worse penalty) nor traffic
volume (H=16/8 see the largest traffic cuts in the sweep, 5.8x and 2.4x,
for no speedup at all). The model does predict independence from B, which
holds at H=32 for B=1/2 but not B=4; that 1-2 point residual is
unexplained.
At B=2 H=32 S=180,180 the L2 hit rate goes 30.0% -> 89.3% and HBM reads
1360 GB -> 245 GB against a compulsory 8.9 GB. Throughput goes flat at
1264-1272 TFLOP/s across S=27k..240k, 2.7-3.1% above aiter at every
point, best case +8.5% at S=239,580 H=64. Since the remap costs nothing
measurable below H=32, the dispatch does not gate on head count.
The remap is bijective, so output is bit-identical -- pinned by a new
test over H=8/16/32/64. A second test covers H=12, where the head count
does not divide the XCD count, both auto and forced-on.
NB: the comment on _init_dualwave_thread_mapping states the opposite
rationale ("one head's q-blocks scatter across all XCDs") and is wrong.
Left in place as pre-existing, but it is worth fixing separately.
dualwave_swp_xcd_swizzle joins the existing dualwave_swp_* build options:
None auto-selects, True/False force it, which is what lets the tests drive
both paths through the public API.
Counting q-blocks at the dispatch site needs BLOCK_M before any traits
object exists, so the literal in _make_dualwave_swp_traits is promoted to
DUALWAVE_SWP_BLOCK_M and used from both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: John Qin <yanyuan.qin@amd.com>
6162271 to
db4143c
Compare
Builds on the XCD swizzle: with L2 residency fixed, the kernel is no longer
memory-starved, which is what makes both of these pay.
Softmax is shift invariant, and sweeping the basis against the true row max
shows fp32 tolerates -100 to +120 exp2 units; the prologue tile max, which
is already computed, is within 5.4. Pinning it makes the per-tile
reduce_max and the online rescale dead code -- which removes the only
reason Q was pre-scaled. That pre-scale costs an extra bf16 narrowing of Q
and is the whole of the accuracy gap against aiter, so the scale moves into
sub_m as one FMA where a subtract already stood. Alone that costs 6%; with
the basis pinned, 0.66%, because four of its five scale multiplies live in
the rescale path. One trait drives both, since raw logits without a pinned
basis is not a configuration anyone wants.
Measured on MI355X, bf16, D=128, non-causal, B=2 H=32, median of three,
aiter in the same process on the same tensors. main means every option off,
including the swizzle, which otherwise auto-dispatches on these shapes:
S aiter main with this
61,380 1225 / 2.29e-03 1258 / 2.87e-03 1290 / 2.35e-03
180,180 1227 / 2.30e-03 1221 / 2.88e-03 1302 / 2.35e-03
239,580 1232 / 2.29e-03 1196 / 2.87e-03 1303 / 2.35e-03
5.4-6.1% above aiter with the accuracy gap closed from 25% to 2%, and +2.5%
to +8.9% over main, growing with S since main decays where this does not.
Causal gains 17% on the same terms. LSE improves three orders of magnitude,
1.5e-03 -> 1.9e-06, because it reads m directly rather than through a
normalisation that divides the error out.
Not bit-identical, so dualwave_swp_fixed_basis defaults off. One case is
uncovered: a later tile whose scores exceed the basis by more than ~109
exp2 units would overflow. Benign data cannot reach that, the measured
worst case being 5.4, but massive-activation data could and there is no
trip test for it yet.
Also tightens the comments the previous commit added, which ran three to
ten times the density of the code around them, and drops a bespoke
list-form helper in favour of the existing _scale_sub_score_pair.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: John Qin <yanyuan.qin@amd.com>
Summary
Two changes to the bf16
DUALWAVE_SWPdense path on gfx950: enable the XCD workgroup remap that already existed but was reachable only from the fp8 builder, and pin the softmax basis to the prologue tile max, which in turn lets the softmax scale move out of Q. Together they put FlyDSL 5.4-6.1% above aiter with the accuracy gap against it closed from 25% to 2%.Motivation
Throughput decayed with sequence length where aiter's stayed flat, and accuracy sat 25% worse than aiter at every length. Each had a single cause, and the two fixes turn out to depend on one another.
Workgroups map to XCDs as
linear_id % 8and the grid is head-fast, so withH % 8 == 0every workgroup of a head lands on the same XCD and each XCD jugglesH/8concurrent K/V streams against its L2 slice. At B=2 H=32 S=180,180 that is a 30.0% L2 hit rate and 1360 GB of HBM reads against a compulsory 8.9 GB.Separately, Q is pre-scaled by
sm*log2eand narrowed back to bf16 before the QK MFMA. That extra narrowing is the whole of the accuracy deficit: modelled, P and O narrowing alone give 2.350e-03 against aiter's measured 2.296e-03, and adding the Q narrowing gives 2.896e-03 against FlyDSL's measured 2.869e-03.Changes
xcd_swizzlethrough the bf16 builder and reuse the fp8 dispatch condition (non-causal,H % NUM_XCD_GFX950 == 0,num_q_blocks >= MIN_Q_BLOCKS_XCD_SWIZZLE). Split-K is unreachable here, taking_build_splitkabove this site. Auto-dispatched; the remap is bijective so output is bit-identical.dualwave_swp_fixed_basis, off by default. It pins the basis, which makes the per-tilereduce_maxand the online rescale dead code, and moves the scale intosub_mas one FMA where a subtract already stood. One trait drives both: raw logits without a pinned basis costs 6% rather than 0.66%, since four of its five scale multiplies live in the rescale path.block_mliteral toDUALWAVE_SWP_BLOCK_Mso the dispatch can count q-blocks before traits exist.Performance
MI355X, bf16, D=128, non-causal, B=2 H=32, median of three, aiter measured in the same process on the same tensors. Before means every option off, including the swizzle, which otherwise auto-dispatches on these shapes. Cells are TFLOP/s and relative L2 against float32.
Causal gains 17% accuracy on the same terms. LSE improves three orders of magnitude, 1.5e-03 → 1.9e-06, since it reads
mdirectly rather than through a normalisation that divides the error out; that matters for the backward pass.Testing
Disabling the cache is not optional here. This change touches helper code outside
the traced closure, so the JIT cache key does not move with it and a warm cache
serves the previous kernel —
CLAUDE.mdcalls this case out. Four tests fail thatway and pass cold.
tests/kernels/test_flash_attn_fwd.py: bit-identity of the remap over H=8/16/32/64, fallback whenH % 8 != 0both forced and auto, accuracy improvement for causal and non-causal, and LSE improvement. Full file passes 45/45.@_requires_gfx950.Dependencies
Breaking Changes
None. The remap is bit-identical, so enabling it by default changes timing only. The pinned basis is not bit-identical and therefore defaults off.
One case is uncovered by it: a later tile whose scores exceed the basis by more than ~109 exp2 units would overflow. Benign data cannot reach that — the measured worst case for the prologue max against the true row max is 5.4 exp2 units — but massive-activation data could, and there is no trip test for it. Adding one was measured and rejected: a trip test needs the per-tile row max, which is exactly what pinning the basis removes, so it gives back more than the option gains (1298 -> 1240 TFLOP/s). Raising the lazy-rescale threshold instead buys 0.15%, since the cost is per-tile rather than on the rare path.