Skip to content

[ROCm][Bugfix] Fix ROCM_ATTN sliding-window boundary - #59550

Merged
tjtanaa merged 4 commits into
vllm-project:mainfrom
tangzzycc:fix/rocm-attn-sliding-window
Oct 4, 2026
Merged

tjtanaa merged 4 commits into
vllm-project:mainfrom
tangzzycc:fix/rocm-attn-sliding-window

Conversation

@tangzzycc

Copy link
Copy Markdown
Contributor

Purpose

ROCM_ATTN stores the decoder window as (W - 1, 0), where W - 1 is the left span. The underlying decode and prefix-prefill kernels use query_position - key_position < SLIDING_WINDOW, so their argument must be the total window length W, including the current token. Passing W - 1 excludes the oldest valid token; for W=1, passing zero disables the window entirely.

This PR passes 1 + self.sliding_window[0] to chunked_prefill_paged_decode. This matches the existing KV-cache docstring example: with sliding_window=4 and seven computed tokens, the next token at position 7 must attend to tokens 4–7. The old argument is 3 and attends to 5–7; the corrected argument is 4 and attends to 4–7.

TRITON_ATTN stores the same (W - 1, 0) tuple and already passes SLIDING_WINDOW=(1 + window_size[0]). Applying the same conversion makes the causal window boundaries consistent between the two backends. The no-window sentinel maps to zero, which the helper treats as disabled; encoder attention uses a separate path.

Open-PR searches on 2026-10-01 found no duplicate. Related #49588, #53628, and #59132 do not modify this backend argument conversion.

Test Plan

Run the focused backend regression and the existing cached-KV boundary test:

.venv/bin/python -m pytest tests/kernels/attention/test_prefix_prefill.py \
  -k 'rocm_backend_sliding_window or cached_kv_block_table_boundary' -q

Test Result

Before the fix, all four windowed regression cases fail and both no-window controls pass. After the fix: 7 passed, including the existing boundary test. All applicable pre-commit checks passed.

Gemma3 1B IT (BF16, window 512) completed inference on Radeon 890M (gfx1150) with inputs within and beyond the window, including chunked prefill and HIP graph decode. Compared before/after ROCM_ATTN with TRITON_ATTN and Transformers FP32. Before/after prompt logprobs were identical for input lengths up to 512; longer inputs showed changes, with errors against the reference implementations increasing or decreasing depending on input length.

AI assistance was used during implementation, testing, and preparation of this PR. vLLM's /pr-checklist skill was used to review the change.

Convert the stored left window span to the window length expected by the paged attention kernels. Add regression coverage for decode and chunked prefill.

Signed-off-by: tangzzycc <3081129260@qq.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.

@mergify mergify Bot added rocm Related to AMD ROCm bug Something isn't working labels Oct 1, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Oct 1, 2026
Comment thread vllm/v1/attention/backends/rocm_attn.py
@tjtanaa tjtanaa added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

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

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Your branch must contain every commit currently on its upstream target branch. Merge or rebase onto the latest target branch, then rerun the command. Append --allow-stale to a run command to test an outdated branch 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.

@tjtanaa

tjtanaa commented Oct 2, 2026

Copy link
Copy Markdown
Member

Gemma3 1B IT (BF16, window 512) completed inference on Radeon 890M (gfx1150) with inputs within and beyond the window, including chunked prefill and HIP graph decode. Compared before/after ROCM_ATTN with TRITON_ATTN and Transformers FP32. Before/after prompt logprobs were identical for input lengths up to 512; longer inputs showed changes, with errors against the reference implementations increasing or decreasing depending on input length.

Please provide the vllm launch command, lmeval score of the model to show that acc is fixed. (show before and after this PR)

Signed-off-by: tangzzycc <3081129260@qq.com>
@tangzzycc

Copy link
Copy Markdown
Contributor Author

Hi @tjtanaa ,I evaluated Gemma3 1B IT on Radeon 890M (gfx1150) with lm-evaluation-harness 0.4.13, using the same first 128 GSM8K test samples, 5-shot, greedy decoding, seed 1234, and a 512-token generation limit. All prompts exceeded the model's 512-token sliding window.

Backend Strict exact match Flexible extraction
ROCM_ATTN before 22.66% (29/128) 23.44% (30/128)
ROCM_ATTN after 23.44% (30/128) 24.22% (31/128)
TRITON_ATTN 24.22% (31/128) 24.22% (31/128)

The before/after accuracy change is small on this subset: one additional correct answer (+0.78 percentage points). The main purpose of the fix is to pass the full window length W, including the current token, to the paged kernel rather than the stored left span W - 1. This matches the kernel parameter contract, the KV-cache window example, and TRITON_ATTN's existing conversion. The boundary regression tests verify this behavior.

Launch command (set PYTHONPATH to the before/after checkout; use --attention-backend TRITON_ATTN for the reference):

PYTHONPATH=/path/to/vllm-checkout .venv/bin/vllm serve \
  models/gemma-3-1b-it --served-model-name gemma3-1b \
  --host 127.0.0.1 --port 8055 --attention-backend ROCM_ATTN \
  --max-model-len 4096 --kv-cache-memory-bytes 134217728 --seed 1234

Evaluation command (use a separate output directory for each run; --limit 128 selects the same first 128 samples):

.venv/bin/python -m lm_eval run --model local-completions \
  --model_args 'model=gemma3-1b,base_url=http://127.0.0.1:8055/v1/completions,tokenizer=models/gemma-3-1b-it,tokenizer_backend=huggingface,tokenized_requests=True,add_bos_token=True,max_length=4096,max_gen_toks=512' \
  --tasks gsm8k --num_fewshot 5 --limit 128 --seed 1234 \
  --log_samples --output_path results-before

@tjtanaa

tjtanaa commented Oct 4, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92784 for commit 63637df73fba.

@vllm-agent

vllm-agent commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

CI selector (shadow): 5 test steps (12 jobs) instead of 74 (98 jobs)

Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works.

Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.

steps (jobs) Today's rules Selector Would skip Would add
NVIDIA, CPU and others 74 (98) 5 (12) 70 (88) 1 (2)
AMD mirrors 71 (93) 95 (133) 18 (23) 42 (63)
Selector would run (5)
  • amd-lm-eval-small-models-harness
  • cpu-kernel-tests ×2
  • fusion-e2e-tp2-ar-rms-dynamo-partition-amd
  • fusion-e2e-tp2-ar-rms-inductor-partition-amd
  • kernels-attention-test ×7
Would skip (today's rules run them) (70)
  • amd-kernels-mi355
  • amd-qwen3-next-mtp-async-eplb-accuracy
  • async-engine-inputs-utils-worker
  • basic-correctness ×2
  • basic-correctness-cpu-offload
  • basic-correctness-cumem
  • basic-correctness-prefetch-offload
  • basic-correctness-sleep-mode
  • basic-models-test-other-cpu
  • basic-models-tests-initialization
  • basic-models-tests-other
  • batch-invariance-b200
  • batch-invariance-h100
  • benchmarks-cli-test
  • cpu-language-generation-and-pooling-model-tests ×3
  • cpu-multimodal-config
  • cpu-params-env-tokenizers-parser
  • cpu-reasoning-renderers
  • cpu-tool-parsers
  • e2e-core-1-gpu
  • e2e-core-large-memory
  • e2e-scheduling-1-gpu
  • e2e-scheduling-accuracy-1-gpu
  • entrypoints-integration-api-server ×4
  • entrypoints-integration-api-server-generate
  • entrypoints-integration-api-server-openai-chat_completion
  • entrypoints-integration-api-server-openai-completion
  • entrypoints-integration-llm
  • entrypoints-integration-multimodal
  • entrypoints-integration-pooling
  • entrypoints-integration-responses-api
  • entrypoints-integration-speech_to_text
  • fusion-e2e-quick-h100
  • fusion-e2e-tp2-b200
  • fusion-e2e-tp2-quick-h100
  • kernels-fla-ops-test-b200
  • kernels-mhc-test-b200
  • kernels-root-misc-test-b200
  • language-models-tests-granite-l4-compatibility
  • language-models-tests-hybrid ×2
  • language-models-tests-standard
  • metrics-tracing-2-gpus
  • multi-modal-models-standard-1-qwen2
  • multi-modal-models-standard-2-qwen3-gemma
  • multi-modal-models-standard-3-llava-qwen2-vl
  • multi-modal-models-standard-4-other-whisper
  • multi-modal-processor ×4
  • multi-modal-processor-cpu ×4
  • pytorch-compilation-dynamic-shapes
  • pytorch-compilation-passes-unit-tests
  • pytorch-compilation-unit-tests
  • pytorch-compilation-unit-tests-h100
  • pytorch-fullgraph-cudagraph-l4-compatibility
  • pytorch-fullgraph-test
  • regression
  • spec-decode-mtp-deepseek-mimo
  • spec-decode-mtp-gemma4
  • spec-decode-mtp-qwen3-5
  • spec-decode-speculators
  • v1-attention-b200 ×2
  • v1-attention-h100-mi300 ×2
  • v1-core
  • v1-executor-worker
  • v1-kv-connectors ×4
  • v1-kv-offload
  • v1-logits-oracle
  • v1-metrics-lmeval
  • v1-others-cpu
  • v1-sample
  • v1-spec-decode
Would add (today's rules do not run them) (1)
  • cpu-kernel-tests ×2 (code map)
AMD mirrors: would skip (18)
  • basic-models-tests-initialization
  • e2e-scheduling-1-gpu
  • extract-hidden-states-integration-2-gpus
  • fusion-e2e-tp2-ar-rms-config-sweep-h100
  • fusion-e2e-tp2-b200
  • fusion-e2e-tp2-quick-h100
  • kernels-fla-ops-test-b200
  • kernels-mhc-test-b200
  • language-models-tests-granite-l4-compatibility
  • multi-modal-processor ×4
  • platform-tests
  • quantized-moe-test-b200
  • spec-decode-mtp-deepseek-mimo
  • spec-decode-mtp-gemma4
  • spec-decode-mtp-qwen3-5
  • v1-attention-b200 ×2
  • v1-attention-h100-mi300 ×2
  • v1-kv-offload
AMD mirrors: would add (42)
  • cudagraph (code map)
  • distributed-compile-rpc-tests-2-gpus (code map)
  • distributed-dp-tests-2-gpus (code map)
  • distributed-dp-tests-4-gpus (code map)
  • distributed-tests-8xh100 (code map)
  • distributed-torchrun-examples-4-gpus (code map)
  • distributed-torchrun-shutdown-tests-2-gpus (code map)
  • engine (code map)
  • engine-1-gpu (code map)
  • entrypoints-unit-tests (code map)
  • examples (code map)
  • fusion-e2e-config-sweep-h100 (code map)
  • jit-monitor-no-runtime-jit (code map)
  • kernels-moe-test ×5 (code map)
  • kernels-quantization-test ×6 (code map)
  • lm-eval-dspark-watermark-2xh100 (code map)
  • lm-eval-watermarking (code map)
  • lora ×4 (code map)
  • lora-tp-distributed ×4 (code map)
  • model-executor (code map)
  • mooncake-ec-tcp-e2e-2-gpus (code map)
  • multi-modal-accuracy-eval-small-models (code map)
  • plugin-tests-2-gpus (code map)
  • pytorch-nightly-dependency-override-check (code map)
  • quantization ×4 (code map)
  • quantized-models-test (code map)
  • ray-dependency-compatibility-check (code map)
  • rayexecutorv2-4-gpus (code map)
  • rust-frontend-cargo-style-clippy (code map)
  • rust-frontend-cargo-tests (code map)
  • rust-frontend-core-correctness (code map)
  • rust-frontend-distributed (code map)
  • rust-frontend-openai-coverage (code map)
  • rust-frontend-serve-admin-coverage (code map)
  • rust-frontend-tool-use (code map)
  • samplers-multimodal-beam-search (code map)
  • samplers-test (code map)
  • scale-out-ec-e2e-2-gpus (code map)
  • sharded-rdt-weight-transfer (code map)
  • spec-decode-draft-model ×4 (code map)
  • spec-decode-ngram-suffix (code map)
  • torch-stable-abi-audit (code map)

2 changed files · base d61081dc3d · head eb815579d5 · Python record: build 92785 at 18f8f96025 · kernel record: table 18f8f96 (build 92785), map 18f8f96 · not counted: 10 build steps, 5 A100 steps the generator no longer emits, 47 optional steps the selector would also run

@tangzzycc

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ The previous CI build is still running: https://buildkite.com/vllm/ci/builds/92784

@tangzzycc

Copy link
Copy Markdown
Contributor Author

/ci cancel

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ No cancelable CI build is running for branch fix/rocm-attn-sliding-window.

@tangzzycc

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ The previous CI build is still running: https://buildkite.com/vllm/ci/builds/92784

@tangzzycc

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92796 for commit eb815579d5fa.

@tjtanaa
tjtanaa enabled auto-merge (squash) October 4, 2026 13:02
@tjtanaa
tjtanaa merged commit f98d1fc into vllm-project:main Oct 4, 2026
192 checks passed
michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 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