Skip to content

[ROCm] Rank TRITON_ATTN above unified attention on gfx950 until the aiter pin has the #62606 fix - #62813

Closed
zhentaocc wants to merge 6 commits into
vllm-project:mainfrom
zhentaocc:rocm-triton-attn-spec-hybrid-priority
Closed

zhentaocc wants to merge 6 commits into
vllm-project:mainfrom
zhentaocc:rocm-triton-attn-spec-hybrid-priority

Conversation

@zhentaocc

@zhentaocc zhentaocc commented Oct 21, 2026 •

Copy link
Copy Markdown

Overview

On gfx950, TRITON_ATTN now ranks right above ROCM_AITER_UNIFIED_ATTN in ROCm attention auto-selection, for every model. This is a temporary workaround for #62606: unified attention gives wrong output on gfx950 for some KV cache sizes, and vLLM's aiter pin (v0.1.26) does not have the fix (ROCm/aiter#6661, released in aiter v0.1.27). Other GPUs are unchanged.

Related:

Claims

Resulting order of the three backends on gfx950:

main this PR
default ROCM_ATTN > unified > TRITON_ATTN ROCM_ATTN > TRITON_ATTN > unified
with a KV connector (ROCM_ATTN excluded) unified > TRITON_ATTN TRITON_ATTN > unified

Validation

Accuracy. One MI355X (gfx950), temperature 0. GSM8K is 5-shot over all 1319 questions. MRCR is tests/evals/mrcr/mrcr_eval.py with 30 samples of 16k–33k tokens. The setup is in #62606. The Qwen3-32B rows come from #62606. The Qwen3-8B rows were retested on 2026-10-27 on main 00b6c4c565 with aiter v0.1.26 and v0.1.27.

model, KV per layer benchmark TRITON_ATTN unified attention, pinned aiter (no fix) unified attention, aiter with the fix
Qwen3-32B, default settings, 2.7 GiB GSM8K, first run (low blocks only) 0.744 0.749 0.751
MRCR 0.478 0.327 0.502
GSM8K, second run 0.738 0.633 0.733
Qwen3-8B, 3.97 GiB, after 525k filler tokens GSM8K 0.860 0.004 0.864 (v0.1.27)
MRCR 0.497 0.157 0.486 (v0.1.27)

With spec decode (Qwen3-32B + EAGLE3, 2.5 GiB per layer), unified attention has a max error of 0.49 against an fp32 reference; TRITON_ATTN has 0.009. End to end, EAGLE3 acceptance length with unified attention and aiter v0.1.26 is 1.70, both at 1k input / concurrency 64 and at 8k / 16. With TRITON_ATTN, or with aiter v0.1.27, it is 2.31 and 2.11.

Tests. In tests/v1/attention/test_rocm_attention_backends_selection.py, test_auto_selection_for_kv_connector now also covers gfx950. With aiter there, a KV connector run selects TRITON_ATTN; without aiter it selects TRITON_ATTN on both architectures. The test runs in the attention CI step.

With a source build of this branch, run pytest tests/v1/attention/test_rocm_attention_backends_selection.py. Against the image vllm/vllm-openai-rocm:nightly-rocm100-00b6c4c565a8732c131e1f452d4a17803bb0842b, apply this PR's vllm/ diff to the installed package. Then run pytest from outside the checkout, so that the uncompiled source tree is not imported. On gfx950 at 68853bdcbf this gives 65 passed and 17 deselected. All 82 tests in the file pass when run against the source tree.

cd /tmp && python3 -m pytest -q --noconftest -p no:cacheprovider --import-mode=importlib --rootdir /tmp \
  /work/vllm/tests/v1/attention/test_rocm_attention_backends_selection.py -k "not segmented and not deepseek"

Details

Root cause. On gfx950, unified attention gives wrong output when a layer's KV cache is 2–4 GiB (#62606). Requests whose KV sits in blocks past 2 GiB get attention output unrelated to their context. With prefix caching on (the default), a server reaches those blocks once about 2 GiB per layer has been allocated in total. The fix is in aiter (ROCm/aiter#6661), so vLLM gets it only with a pin bump.

Change. A self-contained step at the end of _get_backend_priorities, so that it is easy to revert:

# TODO(#62606): remove once the aiter pin includes ROCm/aiter#6661.
# ROCM_AITER_UNIFIED_ATTN gives wrong output on gfx950 for some KV cache
# sizes, so rank TRITON_ATTN right above it there.
unified = AttentionBackendEnum.ROCM_AITER_UNIFIED_ATTN
if on_gfx950() and unified in backends:
    backends.remove(AttentionBackendEnum.TRITON_ATTN)
    backends.insert(backends.index(unified), AttentionBackendEnum.TRITON_ATTN)

Tradeoff. Where this applies, unified attention with aiter v0.1.27 is faster than TRITON_ATTN, mostly at long context:

  • Qwen3-32B + EAGLE3: 3928 vs 3553 tok/s at 1k c64, and 856 vs 557 at 8k c16.
  • Qwen3-Next-80B-A3B + MTP: 403 vs 207 tok/s at 64k c8.

With the current v0.1.26 pin, unified attention is not correct for every KV cache size, so TRITON_ATTN is the safe choice until the pin bump. #61433 restores the order after it.

History. An earlier version of this PR also ranked ROCM_ATTN last for spec decode and hybrid models, because its Triton fallback had two accuracy bugs. Both are now fixed on main:

What remained was a performance change, which belongs in #61433, so it was dropped from this PR.

Author review. I have reviewed the code changes and this description by hand.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

…and hybrid models

With spec decode or a hybrid model, ROCM_ATTN runs its slower Triton path.
For these cases, rank TRITON_ATTN first, then ROCM_AITER_UNIFIED_ATTN, then
ROCM_ATTN. Other models keep the current order.

Unified attention is faster than TRITON_ATTN at long context, but it gives
wrong output on gfx950 when a layer's KV cache is 2-4 GiB until the aiter
pin includes ROCm/aiter#6661 (vllm-project#62606). vllm-project#61433 moves it above TRITON_ATTN
after that.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Chen, Todd <zhenchen@amd.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

…ll models

The unified attention bug (vllm-project#62606) is specific to gfx950 and does not
depend on spec decode or hybrid models. On gfx950, rank TRITON_ATTN above
ROCM_AITER_UNIFIED_ATTN for every model, which also covers KV connector
runs that auto-selected unified attention. Other GPUs keep unified
attention above TRITON_ATTN.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Chen, Todd <zhenchen@amd.com>
@zhentaocc zhentaocc changed the title [ROCm][Spec Decode] Rank TRITON_ATTN above ROCM_ATTN for spec decode and hybrid models [ROCm][Spec Decode] Rank ROCM_ATTN last for spec decode and hybrid models, and TRITON_ATTN above unified attention on gfx950 Oct 21, 2026

@simondanielsson simondanielsson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix!

Question: Do I understand the selection logic correctly. If hybrid and spec decode on:

  1. gfx942: rocm unified -> ... then same as before
    1. net diff: put rocm_attn last
  2. gfx950: triton -> rocm_attn -> ... then same as before
    1. net diff: put triton first

Suggestion: Can you please clarify in the descriptoin what benchmarks were run on what hardware? The evidence supporting the above change of defaults should ideally be crystal clear and make it as concise as possible. :)

Comment thread vllm/platforms/rocm.py Resolved
Build the attention backend list exactly as on main, then move ROCM_ATTN
to the end for spec decode and hybrid models, and, on gfx950, move
TRITON_ATTN right above unified attention. The gfx950 step is a
self-contained block marked TODO(vllm-project#62606), so it can be dropped once the
aiter pin includes ROCm/aiter#6661. The resulting order is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Chen, Todd <zhenchen@amd.com>
@zhentaocc

Copy link
Copy Markdown
Author

@simondanielsson Thanks for the review!

Selection logic. Close, with two corrections. The condition is spec decode or a hybrid model, not both. And on gfx950 ROCM_ATTN is last, not second. For the three backends:

gfx950 gfx942 and others
spec decode or hybrid TRITON_ATTN > unified > ROCM_ATTN unified > TRITON_ATTN > ROCM_ATTN
other models ROCM_ATTN > TRITON_ATTN > unified unchanged: ROCM_ATTN > unified > TRITON_ATTN

So the net diffs are:

  1. Spec decode or hybrid: ROCM_ATTN moves to the end (all GPUs).
  2. gfx950 only: TRITON_ATTN moves right above unified attention (all models). For a plain dense model ROCM_ATTN is still first, so this only changes runs where ROCM_ATTN is excluded or last: KV connector runs, spec decode and hybrid models.

ROCM_AITER_FA stays first when VLLM_ROCM_USE_AITER=1, as on main.

Special case. Done in d83b437, see the inline reply.

Description. The "Evidence" section is now one table: hardware, model, spec decode, workload, backend before and after, and the result. Everything was measured on MI355X (gfx950). I have not measured gfx942; there the PR moves spec decode and hybrid models from ROCM_ATTN to unified attention, which matches the gfx950 results with the aiter fix, but it is not measured on gfx942.

assert backend_path == expected_backend.get_path()


@pytest.mark.parametrize(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Is this test adding much compared to the one above? if not, i'd say we can remove it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in bb8c9a9.

@simondanielsson simondanielsson added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 22, 2026
@github-actions

Copy link
Copy Markdown

✅ @zhentaocc, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@simondanielsson

Copy link
Copy Markdown
Contributor

@tjtanaa would be great to have your view on this. Basically, changing default backends in some cases:

image

@tpopp tpopp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I'm getting a bit confused, but this might be a relevant PR to include in this PR's description: #63268

The auto-selection tests already check the backend that gets picked for
spec decode, hybrid models, KV connectors and gfx950. This test only
checked the runner-up order and rebuilt it with the same logic as
_get_backend_priorities.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Chen, Todd <zhenchen@amd.com>
Both accuracy bugs in ROCM_ATTN's Triton fallback that this avoided are
fixed on main: the sliding-window NaN (vllm-project#62828) no longer occurs since
vllm-project#60767 nulls below-window block-table entries, and the gfx942 unwritten
output heads were fixed by vllm-project#63268. What remains is a performance change,
which belongs in vllm-project#61433. Keep only the gfx950 rule for vllm-project#62606.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Chen, Todd <zhenchen@amd.com>
@zhentaocc zhentaocc changed the title [ROCm][Spec Decode] Rank ROCM_ATTN last for spec decode and hybrid models, and TRITON_ATTN above unified attention on gfx950 [ROCm] Rank TRITON_ATTN above unified attention on gfx950 until the aiter pin has the #62606 fix Oct 27, 2026
@zhentaocc

zhentaocc commented Oct 27, 2026 •

Copy link
Copy Markdown
Author

@simondanielsson @tpopp I narrowed this PR to the gfx950 rule only (b2c4f38) and updated the title and description.

Dropped: ranking ROCM_ATTN last for spec decode and hybrid models.

Its motivation was accuracy, and both bugs in ROCM_ATTN's Triton fallback are now fixed on main. The sliding-window NaN (#62828) no longer occurs since #60767 nulls the below-window block-table entries: at 8k input / concurrency 16, DFlash2 acceptance length on main is 3.22, and 1.19 with only #60767 reverted. The unwritten output heads on gfx942 were fixed by #63268. What remains is a performance gain, and that belongs in #61433. I am closing #62415 for the same reason.

Kept: TRITON_ATTN right above unified attention on gfx950 (#62606).

ROCM_ATTN stays first, so this only changes runs that do not get ROCM_ATTN, for example KV connector runs. With the current aiter pin (v0.1.26), unified attention there gives GSM8K 0.004 vs 0.860 for TRITON_ATTN on the Qwen3-8B check in the description. aiter v0.1.27 has the fix (0.864). Once AITER_BRANCH moves to v0.1.27, #61433 removes this rule.

cc @tjtanaa: the default-order change you were asked about is now limited to that gfx950 rule.

@simondanielsson simondanielsson left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: What aiter release will contain the fix?

EDIT: I see you said v0.1.27, thanks! So fix should be coming very soon

(False, AttentionBackendEnum.TRITON_ATTN),
(True, False, AttentionBackendEnum.ROCM_AITER_UNIFIED_ATTN),
(True, True, AttentionBackendEnum.TRITON_ATTN),
(False, False, AttentionBackendEnum.TRITON_ATTN),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: should we add the False, True case here as well to retain same coverage as before?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, added (False, True, TRITON_ATTN) in 68853bd so the no-aiter case is covered on gfx950 too. And yes, the fix is in aiter v0.1.27; once AITER_BRANCH moves to it, #61433 removes this rule.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Chen, Todd <zhenchen@amd.com>
@zhentaocc

Copy link
Copy Markdown
Author

vLLM main now pins aiter v0.1.27 (#63774), which contains the #62606 fix (ROCm/aiter#6661). The gfx950 rule here is no longer needed, so I am closing this PR. The remaining priority change continues in #61433.

@zhentaocc zhentaocc closed this Nov 3, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants