Skip to content

fix(rocm): correct AITER decode backend gaps — sliding window, CUDA graph, return_lse - #234

Merged
demandal25 merged 4 commits into
AMD-Ecosystem:amd-integrationfrom
demandal25:fix/aiter-decode-gaps
May 20, 2026
Merged

fix(rocm): correct AITER decode backend gaps — sliding window, CUDA graph, return_lse#234
demandal25 merged 4 commits into
AMD-Ecosystem:amd-integrationfrom
demandal25:fix/aiter-decode-gaps

Conversation

@demandal25

@demandal25 demandal25 commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

The AITER PA v1 decode backend on amd-integration has three call patterns that produce wrong output, hard crashes, or unhelpful NotImplementedErrors. This PR fixes each one at the level it's actually broken at, rather than blanket-disabling AITER.

Case Behavior on amd-integration This PR
Sliding-window attention (window_left >= 0) AITER selected. Wrapper passes sliding_window = window_left (off-by-one), and window_left = 0 collides with AITER's "disabled" sentinel — silently wrong output. AITER runs with corrected convention mapping (window_left + 1).
use_cuda_graph=True with explicit backend="aiter" AITER selected. Per-plan scalars (max_kv_len, max_blocks_per_seq) are baked into the captured graph; replay against a larger batch launches with an under-sized grid. Clear ValueError at plan() time. (auto-select already routes to fa2.)
run(return_lse=True) Raises NotImplementedError("AITER decode backend does not currently return LSE"). Transparent dispatch through a pre-built FA2 shadow plan; one-time warning at plan() time so the per-call backend switch is not silent.

Why each fix

1. Sliding window — wrapper convention bug, not a kernel gap

AITER PA v1's kernel does implement window masking (csrc/cpp_itfs/pa/pa_kernels.cuh:457):

if (local_token_idx + i < context_len - sliding_window)
    tmp = -FLT_MAX;

gated by the sliding_window_enabled template flag (set by the compile step at csrc/cpp_itfs/pa/pa_v1.py:144). The wrapper already plumbs sliding_window through _aiter_pa_v1_resolve and the run-time call.

The bug on trunk is a convention difference:

  • FlashInfer: window_left = W → query at position kv_len-1 sees positions [kv_len-1-W, kv_len-1] = W+1 tokens.
  • AITER: sliding_window = S (with S > 0 enabling the mask) → admits S tokens.
  • S = 0 is AITER's compile-time "disabled" sentinel.

Trunk passes sliding_window = window_left — off by one, plus window_left = 0 (one visible token) collides with AITER's disabled sentinel and silently returns full attention. Fixed: sliding_window = window_left + 1 when window_left >= 0, else 0.

This keeps AITER on the hot path for sliding-window models (Gemma, Mistral, etc.) instead of giving up perf to FA2.

2. CUDA graph — wrapper-level limitation, hard-rejected in explicit path

AITER's launch grid is computed at plan() time from max_kv_len and max_blocks_per_seq of the current batch and passed by value to the kernel launch. Under CUDA-graph capture these scalars are baked into the captured graph and can't be widened on replay against a larger batch.

Supporting this properly would require capturing with worst-case dimensions — a new API parameter (e.g. max_seq_len_per_request) — which is out of scope for this PR. The auto-select fallback to FA2 stays in place; the explicit backend="aiter" path (which on trunk silently produces broken launches) now raises:

ValueError: AITER decode backend is incompatible with CUDA-graph capture:
the kernel's launch grid is sized from per-plan scalars (max_kv_len,
max_blocks_per_seq) that are baked into the captured graph at capture time.
Use backend='fa2' for CUDA-graph workflows, or backend='auto' which routes
around this automatically.

3. return_lse — replace NotImplementedError with transparent fallback

AITER PA v1 does not output LSE (only out; the kernel computes per-partition exp_sums/max_logits internally for split-K but does not expose normalized LSE). Trunk raises NotImplementedError at run() time, breaking any caller that needs LSE under an AITER plan.

This PR pre-builds an FA2 decode plan at AITER plan() time and dispatches through it whenever return_lse=True arrives at run(). This is the only correct option since return_lse is per-call, not per-plan. Two details worth flagging:

  • The shadow plan uses the real window_left (and the corresponding template flag), so it produces correct LSE under sliding-window AITER plans (now supported per fix Update Changelog #1).
  • A one-time-per-device warning is emitted at AITER plan() time. Without it, a user toggling return_lse=True on a hot path would silently move from AITER → FA2 with no signal.

Tests added

tests/rocm_tests/test_batch_decode_aiter_hip.py:

  • test_batch_decode_aiter_sliding_window_vs_fa2 — AITER↔FA2 parity over window_left ∈ {0, 31, 127, 1023}, fp16/bf16, batch sizes, GQA ratios, including the saturation regime (window_left >= max_kv_len-1) that exercises the kernel's no-op masking branch.
  • test_batch_decode_aiter_return_lse_via_fa2 — verifies (a) return_lse=False still runs through AITER, (b) return_lse=True falls back to the shadow FA2 plan and returns (output, lse) matching the pure-FA2 reference, with and without sliding window.
  • test_batch_decode_auto_routes_cuda_graph_to_fa2backend="auto" + use_cuda_graph=True resolves to fa2.
  • Extended test_batch_decode_aiter_rejects_invalid_config — explicit backend="aiter" + use_cuda_graph=True raises a ValueError mentioning "CUDA-graph".

Test plan

  • pytest tests/rocm_tests/test_batch_decode_aiter_hip.py -v178 passed in 69 s. New parity + LSE-fallback + CUDA-graph rejection cases.
  • pytest tests/rocm_tests/test_sliding_window_hip.py -m "not slow"1248 passed in 60 s. Exercises the AITER path on every sliding-window decode shape that was previously silently wrong on trunk.
  • pytest tests/rocm_tests/test_batch_decode_kernels_hip.py -m "not slow" -n auto --reruns 21872 passed in 174 s. Covers the broader decode matrix including return_lse=True (which on trunk raised NotImplementedError under AITER) and CUDA-graph wrappers (which now route to FA2 cleanly).

API impact

  • BatchDecodeWithPagedKVCacheWrapper docstring updated to document the AITER-specific constraints (CUDA-graph incompatible; sliding-window supported transparently; return_lse falls back to FA2).
  • No public signature changes.

🤖 Generated with Claude Code

…h, and return_lse cases

AITER PA v1 has three known gaps on ROCm:
1. Sliding-window attention: the kernel does not implement the window
   constraint, so auto-select now forces fa2 when window_left != -1.
2. CUDA graph capture: scalar run() arguments (max_kv_len,
   max_blocks_per_seq) are captured by value and cannot be updated on
   replay; force fa2 for CUDAGraphBatchDecodeWithPagedKVCacheWrapper.
3. return_lse=True: AITER does not expose log-sum-exp output.  Pre-compute
   a FA2 decode plan at plan() time alongside the AITER plan and use it
   transparently whenever return_lse=True is requested.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 20, 2026 05:22

Copilot AI 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.

Pull request overview

This PR updates the ROCm batch-decode wrapper to avoid selecting the AITER decode backend in cases where it produces incorrect results or fails, and to transparently fall back to the FA2 decode path for those cases.

Changes:

  • In backend="auto" mode, force FA2 when sliding-window attention is requested (window_left != -1) or when CUDA graph mode is enabled.
  • For backend="aiter" execution with return_lse=True, precompute a FA2 decode plan during plan() and use it in run() to return (out, lse) instead of raising NotImplementedError.
Comments suppressed due to low confidence (1)

flashinfer/decode_rocm.py:1182

  • The inline comment # window_left == -1 is enforced above is not universally true: the only enforcement is in the backend == "auto" resolution, so callers using backend="aiter" can still reach this code with window_left != -1. Either enforce window_left == -1 in the AITER branch (or handle sliding-window here) or update the comment to reflect the actual constraint.
                head_dim,
                PosEncodingMode[pos_encoding_mode].value,
                False,  # window_left == -1 is enforced above
                logits_soft_cap > 0,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flashinfer/decode_rocm.py
Comment thread flashinfer/decode_rocm.py Outdated
…, hard CUDA-graph reject, LSE warning

Replaces the earlier blanket exclusions with correctness-preserving fixes:

1. Sliding window: AITER PA v1's kernel does implement window masking
   (pa_kernels.cuh: `if (local_token_idx + i < context_len - sliding_window)`
   gated by the `sliding_window_enabled` template flag). The previous fallback
   was based on a misreading; the actual bug is a convention difference. AITER
   admits `sliding_window` tokens; flashinfer's `window_left=W` admits `W+1`
   tokens. Map `aiter_sliding_window = window_left + 1` for `window_left >= 0`
   (else 0, which matches AITER's "disabled" sentinel) and let AITER run.

2. CUDA graph: keep the auto-select fallback to fa2, but also raise a clear
   ValueError in the explicit `backend="aiter"` branch (previously slipped
   through and silently produced wrong output / out-of-bounds launches on
   replay because the launch grid is sized from per-plan scalars that get
   baked into the captured graph).

3. return_lse: keep the shadow-FA2-plan dispatch but (a) propagate the real
   `window_left` into the shadow plan so it works under sliding-window AITER
   plans, (b) emit a one-time warning at plan() time so the per-call backend
   switch is not silent.

Adds tests:
- AITER↔FA2 parity under window_left ∈ {0, 31, 127, 1023} including the
  saturation regime where `sliding_window >= context_len` (kernel no-op
  branch).
- `run(return_lse=True)` correctness vs FA2 reference, with and without
  sliding window.
- Explicit `backend="aiter"` with `use_cuda_graph=True` raises with a
  clear message; `backend="auto"` with `use_cuda_graph=True` resolves to
  fa2.

All 178 cases in tests/rocm_tests/test_batch_decode_aiter_hip.py pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@demandal25 demandal25 changed the title fix(rocm): exclude AITER decode backend for sliding window, CUDA graph, and return_lse fix(rocm): correct AITER decode backend gaps — sliding window, CUDA graph, return_lse May 20, 2026
Previously the AITER plan() path eagerly JIT-compiled and planned the FA2
decode shadow module that backs return_lse=True calls. AITER-only workloads
paid that compile + plan() cost (50-200us per plan, plus one-shot JIT compile
on first encounter of a new template-param combo) for a fallback they never
exercised.

Move the build to a new _ensure_fa2_lse_plan() helper that runs on the first
run(return_lse=True) call. Store the build args captured at plan() time so the
helper can reconstruct the exact same module + plan parameters (window_left,
soft cap, indptr_host, etc.).

The one-time-per-device warning also moves to the deferred build site, where
it fires exactly when the per-call backend switch actually happens — better
signal-to-noise than warning at plan() time on every AITER plan.

Addresses Copilot review feedback on PR #234.
Copilot AI review requested due to automatic review settings May 20, 2026 06:06
…zy-init

Drop the 5-line block comment and the paragraph docstring added in 8a7920d;
the method name + the one-line WHY at the field declaration carry the same
information. Also drop the Optional annotation on _fa2_lse_build_args — it
is always assigned in the same block where it is declared and never re-checked
against None.

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@demandal25
demandal25 merged commit 8592ec7 into AMD-Ecosystem:amd-integration May 20, 2026
0 of 2 checks passed
@demandal25
demandal25 deleted the fix/aiter-decode-gaps branch May 20, 2026 06:18
demandal25 added a commit that referenced this pull request May 20, 2026
## Summary

Restrict `test_mla_aiter_hip.py` parametrize to `page_size=1` and
`dtype=bfloat16` — the only combination that produces correct output
through the AITER MLA decode entrypoint (`aiter.mla.mla_decode_fwd`) for
the FlashInfer-style call pattern used by
`flashinfer.mla_rocm.BatchMLAPagedAttentionWrapper`.

The other failures listed in trunk (sliding-window decode, CUDA-graph
batch decode, shared-prefix decode) were already fixed by PR #234;
rebasing onto the current `amd-integration` resolves them, so this PR
only carries the MLA test fix.

## Why this restriction is correct (not over-restrictive)

### fp16 unsupported — verified at three independent levels

1. `aiter_meta/csrc/py_itfs_cu/asm_mla.cu` (the dispatch the wrapper
routes through) only branches on `Q.dtype == BFloat16` and `Q.dtype ==
Float8_e4m3fnuz/fn`. There is no `Half` branch — fp16 falls through to
`TORCH_CHECK(impl_ptr != nullptr, ": unsupport current data type or
shape")`, which is exactly the runtime error trunk produces.
2. The alternative dispatch path
`aiter_meta/csrc/cpp_itfs/mla/asm_mla_decode_fwd.py:115` has an explicit
`raise ValueError("only support dtype == torch.bfloat16 for now")`.
3. The pre-compiled `.co` kernel files in `aiter_meta/hsa/gfx942/mla/`
exist only as `bf16` (`a16w16`), `fp8` (`a8w8`), and mixed `a16w8`
variants. No fp16 kernels are shipped.

### page_size=1 only — verified empirically and by canonical AITER usage

`page_size` is templated into the kernel as a runtime parameter
(`s_log2_plen`), so it is not statically rejected — but in practice the
kernel produces silently-wrong output for page_size > 1 through this
code path. Evidence:

- AITER's own canonical test `asm_mla_decode_fwd_test.py` hardcodes
`block_size = 1` and `kv_last_page_lens = torch.ones(batch_size)`.
- AITER's AOT compilation `aiter/aot/asm_mla_decode_fwd.py` only targets
`page_size=1`.
- Empirical sweep through the wrapper (max abs diff vs pure-PyTorch
reference):

  | page_size | kv_len=1 | kv_len=5 | kv_len=16 | kv_len=64 |
  |-----------|----------|----------|-----------|-----------|
  | 1         | 0.0000   | 0.0010   | 0.0005    | 0.0002    |
  | 16        | 0.0000   | 0.3182   | 0.3513    | 0.1787    |
  | 32        | 0.0000   | 0.3182   | 0.3513    | 0.2473    |

(kv_len=1 is a trivial pass: softmax over 1 element is `[1.0]`
regardless of the score, so it does not exercise the kernel's per-page
masking/indexing.)

This matches the existing project note from PR #232 wiring up
`mla_rocm.py` ("page_size=1 only — AITER ASM limitation"). Production
frameworks (vLLM, SGLang) use AITER MLA with `page_size > 1` only
through different entrypoints (NSA decode path, custom KV-cache
repacking) that don't match the FlashInfer
`BatchMLAPagedAttentionWrapper` contract.

A future improvement would be to wire up an alternate AITER path that
supports larger page sizes, but that is a new wrapper, not a test fix.

## Test plan

- [x] `pytest tests/rocm_tests/test_mla_aiter_hip.py` — 8 passed
- [x] `pytest -n auto --reruns 2 -m "not slow"` — exit 0 (full fast
suite)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
demandal25 added a commit that referenced this pull request May 21, 2026
## Summary

Refresh the FlashInfer+ROCm README aimed at library consumers, refresh
the Feature Support Matrix to match what has actually landed on
`amd-integration`, and align the ROCm MLA wrapper with the rest of the
ROCm backends so `backend="auto"` is accepted everywhere.

### What changed

#### `README.md`

- **Intro and structure.** Tighten the intro to call out HIP-in-repo
kernels vs AITER dispatch up front; link to the Feature Support Matrix
and AITER sections from the first paragraph. Cross-link CDNA3 / CDNA4 to
AMD's official architecture whitepapers on first mention.
- **Feature Support Matrix.** Replaced with a five-column table (Kernel
/ HIP / AITER / `backend="auto"` resolves to / Notes). New ✅ rows:
Cascade (#221), MLA via AITER (#232), RoPE (#223), paged KV-cache
append, RMSNorm via AITER (#232), sliding-window decode on the AITER
path (#234), activation, quantization, and opt-in `torch.compile`
(#210). Every ✅ is backed by a `tests/rocm_tests/test_*_hip.py`. FP8
status is folded into per-row notes rather than a dedicated column.
- **GPU / ROCm / PyTorch.** Consolidated into one section with arch
codenames inline (gfx942 → MI300X/MI325X = CDNA3, gfx950 → MI355X =
CDNA4). `pip install torch` uses `--index-url` instead of `-f` so pip
cannot silently fall back to a CPU-only PyPI wheel (matches CLAUDE.md).
- **Getting Started.** Collapsed the Docker image table to the latest
validated tag and pointed at Docker Hub for older releases. Dropped the
manual `micromamba activate base` step (the env is auto-activated). Used
the concrete image tag plus a `--name=flashinfer-rocm` in the `docker
run` snippet.
- **Trying the Examples.** Simplified to point at `examples/` plus one
run command — no wget-based downloads.
- **Install from Source.** Renamed from "Build from Source"; rewrote the
ambiguous "Environment name varies …" note (and later removed it once
the build / run blocks made the matching tag self-evident).
- **AITER Support.** Collapsed the section intro to avoid re-listing
conditions already in the matrix; cross-link Known Limitations. Rewrote
Known Limitations preamble to state the two-group split (hard errors vs
silently-ignored kwargs). Dropped the redundant Single Prefill Example
(Basic Usage already shows the call pattern).
- **Environment Variables.** New section documenting runtime env vars —
`FLASHINFER_USE_TORCH_CUSTOM_OPS`, `FLASHINFER_HIP_FUSED_CASCADE`,
`FLASHINFER_LOGGING_LEVEL`, `FLASHINFER_DISABLE_JIT`, `ROCM_PATH` /
`ROCM_HOME`. Build-time vars stay in `CLAUDE.md` and are linked from
here.
- **Runtime Helpers.** Short snippet showing `is_aiter_supported` and
`check_torch_rocm_compatibility`; calls out
`validate_flashinfer_rocm_arch` as a build-time validator, not a runtime
helper.
- **CPX-mode pytest notes.** Split the dense paragraph into labelled
bullets (Worker count / Reruns / `slow` marker / HIPBLAS retry).
- **Basic Usage.** Moved to the end of the README as a closing example.
- **License and Acknowledgements.** Added; the contributing reminder
lives on its own line.

#### `flashinfer/mla_rocm.py` + `tests/rocm_tests/test_mla_aiter_hip.py`

- Accept `backend="auto"` as an alias for `"aiter"` on the ROCm MLA
wrapper (default is now `"auto"` to match every other ROCm wrapper).
Previously the wrapper raised `ValueError` on anything other than
`"aiter"`, leaving MLA as the odd one out in the public API even though
there is exactly one implementation to pick from on ROCm.
- New tests: `test_mla_backend_accepts_auto_and_aiter` (parametrized
over both values) and `test_mla_backend_rejects_unsupported` (confirms
`backend="fa2"` still raises; runs without a GPU since the check fires
before the AITER probe).

## Test plan

- [x] `pre-commit run -a` passes.
- [x] `pre-commit run markdownlint --files README.md` passes after every
change.
- [x] Every TOC entry resolves to an `##` heading in the body.
- [x] Every ✅ in the Feature Support Matrix has a backing
`tests/rocm_tests/test_*_hip.py`.
- [x] `pytest tests/rocm_tests/test_mla_aiter_hip.py` — 11 passed.
- [x] Render the README on the PR page and visually confirm tables, code
blocks, and `<details>` sections look right.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants