Conversation
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
|
gfx1101 (RDNA3, e.g. RX 7800 XT) ships no MHA config, so `_get_config` in `_triton_kernels/attention/mha.py` finds no `configs/gfx1101/triton/attention/mha/DEFAULT.json` and every call to `aiter.ops.triton.attention.mha.flash_attn_func` fails before a kernel runs. Of the architectures in RDNA_ARCHS, only gfx1151 ships one today. Nine of the eleven entries are taken verbatim from the gfx1151 donor (RDNA3.5, added in ROCm#3423, tuned in ROCm#3560), which is the nearest tuned architecture. Two forward entries are tuned on gfx1101 instead of inherited, and they differ from each other only in `num_warps` and `num_stages`: fwd/default BLOCK_M 128, num_warps 8, num_stages 3 (donor: 64 / 4 / 2) fwd/small_head BLOCK_M 128, num_warps 4, num_stages 1 The split uses the `small_head` bucket added in ROCm#4414, which is opt-in per architecture by the mere presence of the key, so this stays a data-only change. It is needed because one `fwd/default` cannot serve both halves on this card: measured against the donor, `M128 w4 s1` is 0.935x on head_dim 64 but 1.269x on head_dim 128, while `M128 w8 s3` is 0.914x on head_dim 128 but 1.062x on head_dim 64. Measured on Windows native ROCm, triton 3.8.0, fp16, 10 independent repeats of 20 iterations after 5 warmups, `torch.cuda.synchronize()` per iteration; a result counts only when the [min, max] intervals across repeats are disjoint. Ratios are against the gfx1151 donor entry, i.e. against what a donor-inherited config would do. head_dim 128, `default` Flux joint 0.914x / 0.891x / 0.892x on three independent torch+ROCm stacks (2.11/7.15, 2.11/10.1, 2.15/10.1), all pinned to the same triton; llama3-8B 0.882x, mixtral-7B 0.880x, kimik25-tp4 0.849x at seqlen 16384 head_dim <= 64, `small_head` SDXL self-attn 0.935x; deepseek-V3 0.658x and glm47fp8-tp4 0.857x at seqlen 16384 The LLM shapes come from `op_tests/op_benchmarks/triton/utils/model_configs.json` (prefill, causal, GQA, batch 1); the sequence lengths are not in that file and are chosen here. Not covered: batch > 1, varlen/thd, sliding window, decode. Note that the `small_head` comment in `_get_config` does not describe gfx1101. It states that 16 < d <= 64 suffers a num_stages=1 pipelining pathology which num_stages=3 cures. On this card the ordering is the opposite -- on the tuned M128/N32/w4 tile, num_stages 1/2/3 measured 2.300 / 2.399 / 2.492 ms. The bucket is still the right mechanism here, for a different reason: the two head_dim ranges want a different `num_warps`, not a different `num_stages`. Signed-off-by: Martin Domanský <ragua@email.cz>
c27cabd to
ec3af27
Compare
|
The branch has been force-updated from Two independent reasons, neither of which was visible when it was approved. First, the path. The approved revision added Second, and the reason this took longer: on re-measurement against today's Rather than withdraw the tuning, this revision splits it across the Both halves are then confirmed on ten LLM prefill shapes taken from Still one new file, data only, no code touched, and no other architecture is affected — One thing in the description I would rather flag here as well: the comment on that bucket in @mgehre-amd — you added the gfx1151 MHA config in #3423 and tuned it in #3560, and nine of the eleven entries here are byte-identical to that file, so you are likely the right person to judge the two that are not. I have deliberately not asked @carlushuang to re-review: the values now on the branch are not the ones that were approved, and I would rather have that judged fresh than carried over. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved correctness, backward-coverage, and bf16-validation findings must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a gfx1101 Triton MHA configuration, making default dispatch reachable and splitting forward tuning by head dimension.
Changes:
- Adds tuned
fwd/defaultandfwd/small_headentries. - Adds inherited PE and backward configurations.
- Adds architecture-specific MHA configuration data.
Review findings:
- Critical (2 votes): Add d=8/16 correctness coverage or a safe route.
- Moderate (2 votes): Validate or defer/gate gfx1101 backward support.
- Moderate (1 vote): Measure bf16 or add dtype-specific selection.
File summaries
| File | Description |
|---|---|
aiter/ops/triton/configs/gfx1101/triton/attention/mha/DEFAULT.json |
Adds gfx1101 MHA tuning and dispatch entries. |
Review details
Suppressed comments (1)
aiter/ops/triton/configs/gfx1101/triton/attention/mha/DEFAULT.json:19
- [verified] The new tiles are selected for both fp16 and bf16 because
_get_confighas no dtype-specific config branch, while every reported tuning run is fp16 and the benchmark defaults to bf16. A config that wins fp16 is not established to be a good bf16 choice, so this can regress the default model dtype on gfx1101. Author must measure the routed d64/d128 cases in bf16 as well, or add dtype-specific selection before shipping these values.
"BLOCK_M": 128,
"BLOCK_N": 32,
"PRELOAD_V": false,
"waves_per_eu": 1,
"num_warps": 8,
"num_ctas": 1,
"num_stages": 3
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Both review findings are answered in their own threads, with measurements on gfx1101 rather than from the source alone. Neither finding leads to a change in the file. The values stand as submitted. What remains unmeasured is listed here rather than left to be discovered:
If any of these is required before approval, please say which and it will be measured. One question is left open deliberately, because it is a decision about |
|
A question about The
The band and the value are equivalent only as long as Reading the four architectures that ship an MHA table on
On gfx950, the only architecture that has the bucket, Tuning gfx1101 produced the opposite polarity. That the two architectures tune differently is not itself surprising. gfx950 is a CDNA part with Wave64 and matrix cores, gfx1101 is RDNA3 with Wave32, and different optima are expected. What does not carry across is the protection: with the polarity reversed, the same lower bound routes So the observation is not that the values differ. It is that the bucket expresses a band, while the constraint it was introduced to enforce is a property of a value. Those two agree for only one of the two polarities. This is not a defect report. On gfx1101, Three readings, and the code does not tell us which is intended:
Which of the three is it? If the answer is the second, we are willing to prepare that change as a separate PR. It touches the |
Motivation
aiter.ops.triton.attention.mha.flash_attn_funccannot run at all on gfx1101 (RDNA3)._get_configresolvesconfigs/{arch}/triton/attention/mha/DEFAULT.json, and with no per-arch file and no fallback the call raisesbefore any kernel is compiled. gfx1101 is in
RDNA_ARCHS, so the architecture is declared supported while the default MHA path is unreachable on it. This PR is the narrow remedy for the one architecture I can measure on.What changed since the approval on 2026-08-06
This PR was approved at
c27cabd7, and that revision no longer holds, so the branch has been rewritten rather than merged as approved. Two things moved underneath it: the config layout (#5018 / #5019 deleted the flat path without a fallback) and the kernel (#3936, #4414).Re-measured against today's
main, the approvedfwd/defaultis 1.274x slower onhead_dim128, and onhead_dim64 it no longer establishes a gain — the intervals overlap the donor's. What moved is the donor, not the candidate: the inherited gfx1151 entry became faster on today's kernel while the tuned one stood still. Numerics did not move at all, so a correctness re-verification passes with digit-identical residuals — re-verifying rather than re-measuring would have merged the regression.Rather than withdraw the tuning, this revision splits it in two, which removes the regression instead of trading one shape against another.
Technical Details
One new file,
aiter/ops/triton/configs/gfx1101/triton/attention/mha/DEFAULT.json, in the nested layout from #5019. 101 lines added, nothing removed, no code touched.Nine of the eleven entries are byte-identical to
gfx1151/triton/attention/mha/DEFAULT.json, the nearest tuned architecture. Two forward entries are tuned on gfx1101 instead of inherited:fwd/small_headis the bucket added in #4414 for16 < head_dim_v <= 64. Because it is opt-in per architecture by the presence of the key, this remains a data-only change that cannot affect any other architecture.The split is needed because the two
head_dimranges want a differentnum_warps, and_get_configalready discriminates on exactly that axis. Two independent coarse sweeps, one per range, selected these two entries separately — the numbers are below.📌 The comment on that bucket does not describe this architecture, and these values should not be read as agreeing with it. It states that
16 < d <= 64suffers anum_stages=1pipelining pathology whichnum_stages=3cures. On gfx1101 the ordering is the opposite: on the tunedM128 / N32 / w4tile,num_stages1 / 2 / 3 measured 2.346 / 2.429 / 2.450 ms. If that comment is meant as an invariant rather than as an observation about the architectures tuned so far, it changes the answer here, and I would rather be told than assume.num_stagesis Triton-version-dependent on this architecture, so these values should not be assumed to carry backwards — click for whyUnder the Triton used here,
use_async_copydefaults toarch in ["gfx950", "gfx1250"]andis_pingpong_schedule_enabledadmits only gfx942 and gfx950 (triton/backends/amd/compiler.py:43and:33), so gfx1101 gets neither.num_stagestherefore means classic software pipelining into registers only, which is why it can trade againstBLOCK_Mand why the answer differs between the two head-dim ranges. Under Triton 3.7.1 the same knob had a different gate —use_async_copyread the env knob without testing the arch — so these numbers should not be assumed to carry backwards.fwd/dropout_or_fp32, bothpeentries and both backward sections are inherited unchanged; only the forward path was exercised, so no claim is made about backward.Test Plan
Windows native ROCm, RX 7800 XT (gfx1101, RDNA3, 60 CU), AITER checkout on
PYTHONPATHwith no install,AITER_TRITON_ONLYimplicit on win32.Every number below was measured on one software stack: torch
2.15.0a0+rocm10.1.0a20260822, HIP 7.16.26332,triton-windows3.8.0.post28, a Windows build of Triton 3.8.0 and the newest release available. Older stacks are deliberately not reported, so that no row here describes a configuration that is no longer current.fp16, seed 424242. Each point is the median of 20 timed iterations after 5 warmups with
torch.cuda.synchronize()per iteration. Sweeps are single-shot rankings over a grid; every claimed ratio comes from a series of 10 independent repeats, and is claimed only where the[min, max]intervals across those repeats are disjoint. Where they overlap this is stated and nothing is claimed. Ratios are against the gfx1151 donor entry, since inheriting is the alternative to this file.Two shape sets. Diffusion attention: SDXL self-attention
(2, 4096, 4096, 10, 64),head_dim64, and a Flux joint-attention shape(1, 4608, 4608, 24, 128),head_dim128. LLM prefill,causal=True, GQA, batch 1, head dims fromop_tests/op_benchmarks/triton/utils/model_configs.json(hidden_size / num_attention_heads); the sequence lengths are not in that file and are my choice, 4096 and 16384.op_tests/triton_tests/attention/test_mha.pyandbench_mha.pyboth reachaiter.dtypes, whichaiter/__init__.pybinds only in the non-AITER_TRITON_ONLYbranch, and the JIT path below it ends inRuntimeError("ROCm version file not found"). The upstream shape set is therefore used with my own measuring path rather than the upstream harness. This is independent of the present change, and I preferred not to introduce a workaround into the verification being reported.Test Result
The defect is still present on the current base
With the new file removed from the checkout, on
main@12620102and the stack above:It is raised on the first call and on every later call in the same process. On that base the architectures that do have an MHA config are gfx942, gfx950, gfx1151 and gfx1250.
Two escape hatches work on the same host and the same shape: passing
config=explicitly, andmha_set_impl("dao_ai"). Both returnmax abs 6.470e-05against the fp32 reference, against6.013e-05for torch's own fp16 SDPA measured in the same process. The kernel therefore runs correctly on this architecture, and it is only the config lookup that makes the default path unreachable.Routing and correctness
Verified on
main@12620102with this file applied, each entry in a separate process because_get_configmemoises.head_dim64 routing tosmall_headandhead_dim128 routing todefaultis the behaviour the split depends on.All six forward entries resolve as intended, max abs 6.469e-05 to 7.008e-05 — click for the per-entry output
Every configuration measured in this campaign produced an identical maximum absolute difference per shape, the inherited one included, so tile parameters have no numerical effect on this kernel and the choice is purely a performance decision.
How the two entries were selected
One coarse sweep per head-dim range, 72 candidates each over
BLOCK_M x BLOCK_N x num_warps x num_stages, ranked on that range's shape. The two sweeps are independent and each selected the entry this PR ships for its own range.M128 w4 s1on the same shapehead_dim64M128 N32 w4 s1= this PR'ssmall_headhead_dim128M128 N32 w8 s3= this PR'sdefaultThe separating knob is
num_warps: athead_dim128 the same tile measures 8.619 ms atw4 s1against 6.657 ms atw8 s1, while athead_dim64 the ordering reverses. A refine stage overPRELOAD_Vandwaves_per_euon the three best candidates produced no cell that beatM128 N32 w8 s3 pv0 we1onhead_dim128.Three of the 72 candidates on the
head_dim128 shape are unusable on this architecture, and the failure is the expected one:out of resource: shared memory, Required: 69632 (resp. 73728), Hardware limit: 65536.head_dim128 — the regression and its removal10 repeats per configuration, and the whole series run twice.
[min, max]over the repeats in ms, ratio of medians against the donor in brackets.M64 w4 s2M128 w4 s1default,M128 w8 s3✅ / ❌ = intervals disjoint from the donor's. The two runs agree to the third significant figure, and the same
M128 w8 s3entry costs 1.070–1.077 onhead_dim64 — which is the reason onefwd/defaultcannot serve both ranges.LLM prefill shapes,
causal=True, GQA, batch 1Ratios against the donor, whose median in ms is given for scale.
All ten shapes: the entry this file routes to is the faster candidate in every row, significantly so in all ten — click for the table
M128 w4 s1M128 w8 s3defaultdefaultdefaultdefaultdefaultdefaultsmall_headsmall_headsmall_headsmall_headIn every row the entry this file routes the shape to is the faster of the two candidates, and in nine of the ten it is significantly faster than the donor. Comparing the two candidates directly, their
[min, max]intervals are disjoint in all ten rows — the routing decision is significant on every shape in the set, even where the winner's margin over the donor is not.This set matters because #3560 tuned the donor on
n=112LLM shapes, so choosingfwd/defaulton diffusion shapes replaces an LLM-derived value with a diffusion-derived one. The measurement says the two families do not disagree on this card: both wantnum_warps 8athead_dim112–128, and the axis that separates them ishead_dim, not workload.head_dim64,fwd/small_headThe LLM rows above are the evidence for this entry, and they are the only significant evidence for it.
On the diffusion shape it ranked first of 72 candidates in the sweep, but across 10 repeats it measures 0.933 and 0.936 with intervals that overlap the donor's in both runs, so no gain is claimed there.
The entry is nonetheless the right one for its range, and the comparison that establishes that is against the alternative rather than against the donor. Giving
fwd/defaultto both ranges measures 1.070–1.077 on the diffusionhead_dim64 shape, and on the fourhead_dim53–56 LLM shapes it is slower thanM128 w4 s1by 0.924 vs 0.828, 0.714 vs 0.644, 1.019 vs 0.915 and 0.922 vs 0.852 — disjoint intervals in all four.What this does not establish
Batch > 1, the
varlen/thdlayout, sliding window, and the decode phase (seqlen_q = 1) were not measured. Forward path only; both backward sections are inherited. One card, one host and one software stack — no other RDNA or CDNA part is available to me, and nothing here is a claim about gfx1151, whose values this PR does not change.A cross-attention shape
(2, 4096, 77, 10, 64)was measured and is deliberately not reported: the donor's own timing on it varies by 31 % across repeats on this host, which is far above every other shape in the set, so it discriminates nothing in either direction.A cross-architecture fallback for the same reachability defect is a separate change and is deliberately not bundled here. I am happy to file it separately if that is wanted.