Skip to content

[ROCm] Unify ViT FlashAttention backend selection - #50449

Open
hyoon1 wants to merge 6 commits into
vllm-project:mainfrom
hyoon1:fix/rocm-vit-flash-attn-selection
Open

hyoon1 wants to merge 6 commits into
vllm-project:mainfrom
hyoon1:fix/rocm-vit-flash-attn-selection

Conversation

@hyoon1

@hyoon1 hyoon1 commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Update FlashAttention to 4a948e9c in the ROCm Docker base images and include RDNA targets in CK builds, allowing the existing ViT FlashAttention path to use CK by default on RDNA3/RDNA4.

  • Update FlashAttention: Change FA_BRANCH from 0e60e394 to 4a948e9c in docker/Dockerfile.rocm_base and docker/Dockerfile.rock_base. This commit includes RDNA CK support and the CK varlen_fwd binding fix.
  • Include RDNA build targets: Remove the filter that excluded RDNA architectures from FlashAttention builds and pass the full PYTORCH_ROCM_ARCH list to GPU_ARCHS.
  • Unify ViT backend selection: Apply the same automatic FlashAttention selection conditions to CDNA and RDNA: the flash_attn package is available and the dtype is FP16 or BF16.
  • Remove the Triton-only restriction: Remove the RDNA-specific requirement for FLASH_ATTENTION_TRITON_AMD_ENABLE=TRUE and the private implementation module probes.

Explicit backend selection and the existing CDNA AITER FlashAttention priority remain unchanged.

ViT Backend Setting FLASH_ATTENTION_TRITON_AMD_ENABLE Before Change After Change
Auto (no explicit override) Unset or FALSE TORCH_SDPA FLASH_ATTN (CK)
Auto (no explicit override) TRUE FLASH_ATTN (Triton) FLASH_ATTN (Triton)
Auto (no explicit override), CK missing Unset or FALSE TORCH_SDPA FLASH_ATTN (Triton via FA's own fallback)

Validation

Existing tests

The existing ROCm attention backend selection tests were run:

.venv/bin/python -m pytest \
  tests/v1/attention/test_rocm_attention_backends_selection.py -q

Result: 76 passed, 15 warnings.

No test files are changed in this PR.

Runtime validation

A 100-request multimodal workload using Qwen3.5-9B was validated under the following ViT attention configurations:

Configuration Setting Result
Torch SDPA --mm-encoder-attn-backend TORCH_SDPA Passed
FlashAttention default path FLASH_ATTENTION_TRITON_AMD_ENABLE unset or FALSE Passed
FlashAttention Triton FLASH_ATTENTION_TRITON_AMD_ENABLE=TRUE Passed
Runtime validation command
vllm bench throughput \
  --model /home/user/workspace/models/Qwen3.5-9B \
  --trust-remote-code \
  --backend vllm-chat \
  --dataset-name random-mm \
  --dataset-path /dev/null \
  --random-mm-bucket-config '{(1024, 1024, 1): 1.0}' \
  --random-input-len 1024 \
  --random-output-len 128 \
  --num-prompts 100 \
  --num-warmups 1 \
  --max-model-len 4096

Apply the setting listed in the table for each configuration.

Details

ViT FlashAttention support already exists in vLLM. Previously, the Docker build excluded RDNA targets from FlashAttention CK builds, and automatic FlashAttention selection on RDNA required Triton opt-in.

Following the maintainer discussion, this revision updates the existing PR to include both the FlashAttention dependency/build changes and the corresponding selector simplification. Updating the pin alone would leave RDNA CK kernels excluded by the existing build filter.

FlashAttention handles CK/Triton implementation selection. The pinned version also falls back to Triton if importing the CK extension raises ImportError. This fallback does not cover GPU execution errors after a successful import.


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

@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.

@mergify mergify Bot added rocm Related to AMD ROCm v1 labels Jul 30, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Jul 30, 2026
@hyoon1
hyoon1 force-pushed the fix/rocm-vit-flash-attn-selection branch from 45bd3b0 to 1010d5f Compare July 30, 2026 22:54
@mergify

mergify Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hyoon1.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Jul 31, 2026
@hyoon1
hyoon1 force-pushed the fix/rocm-vit-flash-attn-selection branch from 1010d5f to b57e115 Compare July 31, 2026 02:23
@mergify mergify Bot removed the needs-rebase label Jul 31, 2026
@mergify

mergify Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hyoon1.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Jul 31, 2026
@hyoon1
hyoon1 force-pushed the fix/rocm-vit-flash-attn-selection branch from b57e115 to 9e0a182 Compare July 31, 2026 22:26
@mergify mergify Bot removed the needs-rebase label Jul 31, 2026
@mergify

mergify Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hyoon1.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 10, 2026
Comment thread vllm/platforms/rocm.py Outdated
on_cdna()
and find_spec("flash_attn") is not None
and (dtype == torch.float16 or dtype == torch.bfloat16)
if (on_cdna() or on_rdna()) and (

@dllehr-amd dllehr-amd Sep 29, 2026 •

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.

you can use on_gfx1x() instead of on_rdna to just snag the 11 and 12 gfx arch's

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated

ViT already supports FlashAttention on ROCm. Update the ROCm and TheRock
Docker pins to 4a948e9c and include RDNA targets in FlashAttention CK builds.

With RDNA CK support in the updated dependency, remove the Triton-only
restriction from automatic ViT FlashAttention selection on RDNA. Use the
same package and dtype checks as CDNA, preserving CDNA AITER priority.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Hosang Yoon <hosang.yoon@amd.com>
@hyoon1
hyoon1 force-pushed the fix/rocm-vit-flash-attn-selection branch from 9e0a182 to b062ca7 Compare October 6, 2026 18:22
@hyoon1
hyoon1 requested a review from shen-shanshan as a code owner October 6, 2026 18:22
@mergify mergify Bot added ci/build and removed needs-rebase labels Oct 6, 2026
@mergify

mergify Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hyoon1.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 18, 2026

@dllehr-amd dllehr-amd 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.

Looks good! @AndreasKaratzas @Rohan138 I'll leave it to you to decide when to suck this into our Dockerfiles. Essentially upstream flash attention (tridao) works with rdna now, so we don't need to exclude them from the build, and only need a flash attention commit bump.

@dllehr-amd dllehr-amd added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 20, 2026
@dllehr-amd

Copy link
Copy Markdown
Contributor

/ci run

@github-actions

Copy link
Copy Markdown

✅ @hyoon1, 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.

@github-actions

Copy link
Copy Markdown

❌ This PR is 885 commits behind upstream main. The bot could not merge upstream main into this branch (API returned 422: merge conflict between base and head). No new CI build was started. Either merge or rebase onto the latest main yourself and rerun /ci run, or use /ci run --allow-stale to test this branch as is.

hyoon1 and others added 2 commits October 20, 2026 15:01
Adopt the current ROCm Dockerfile layout and preserve the existing ViT selector change. Reapply the FlashAttention build updates in a follow-up commit.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Hosang Yoon <hosang.yoon@amd.com>
Update the ROCm 10 and ROCm 7.2 base images to FlashAttention 4a948e9c and include RDNA targets in CK builds after merging the new Dockerfile layout.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Hosang Yoon <hosang.yoon@amd.com>
@Rohan138

Rohan138 commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

@hyoon1 could you rebase this PR against the current HEAD? Sorry we moved the dockerfiles around a bunch last week to enable ROCm 10

@hyoon1

hyoon1 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

@hyoon1 could you rebase this PR against the current HEAD? Sorry we moved the dockerfiles around a bunch last week to enable ROCm 10

@Rohan138 Just updated. Could you take a look?

@Rohan138

Copy link
Copy Markdown
Collaborator

/ci run

@Rohan138

Copy link
Copy Markdown
Collaborator

/amd-ci run nightly

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #96216 for commit 91d3ab78b356.

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite AMD CI #15072 for commit 91d3ab78b356.

@Rohan138

Copy link
Copy Markdown
Collaborator

I'm good to merge this (pending CI) btw! It'll cause a couple minor merge conflicts but hopefully nothing crazy @dllehr-amd

@mergify mergify Bot removed the needs-rebase label Oct 20, 2026
@hyoon1

hyoon1 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Rohan138, seems CI is interrupted during the base docker rebuilding. Could you please check?

@mergify

mergify Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hyoon1.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 29, 2026
Keep the unified ROCm base image layout and remove the retired ROCm 7.2
base Dockerfile. Preserve FlashAttention 4a948e9c, the full RDNA CK build
targets, and the reviewed ViT backend selection changes. Retain upstream's
build pipeline updates and the separate gfx1250 base image.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Hosang Yoon <hosang.yoon@amd.com>
@hyoon1
hyoon1 requested a review from a team November 2, 2026 22:21
@mergify mergify Bot removed the needs-rebase label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown

❌ @hyoon1, your CI command failed with an unexpected error, so CI may not have started. See the workflow run for details.

@Rohan138

Rohan138 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

/ci run

@Rohan138

Rohan138 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

/amd-ci run nightly

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #100003 for commit abe93ae937c2.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite AMD CI #15769 for commit abe93ae937c2.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants