Skip to content

fix(attention): zero empty decode rows in unified attention - #6566

Closed
XiaobingSuper wants to merge 1 commit into
mainfrom
fix/unified-attention-zero-kv
Closed

XiaobingSuper wants to merge 1 commit into
mainfrom
fix/unified-attention-zero-kv

Conversation

@XiaobingSuper

Copy link
Copy Markdown
Contributor

Summary

  • make unified_attention return deterministic zeros for all-decode rows whose KV length is zero
  • neutralize split-KV partial output/max/expsum for the same padding rows
  • add a gfx1250 Gluon regression test with lengths [2048, 0, 0, 0]

Root cause

CUDA-graph buckets can pad a decode batch with query rows whose seqused_k value is zero. The gfx1250 3D Gluon path returns before writing those rows, leaving NaN/uninitialized output. Later model layers consume the padding values and can produce repeated or non-terminating generations.

This was reproduced while bringing up MiniMax-M3 TP4 on MI455 for LLM-Booster #201 and the fallback in ATOM #1410. A standalone BF16 shuffled-cache check showed:

  • before: finite_by_row=[True, False, False, False]
  • after: finite_by_row=[True, True, True, True]
  • active-row cosine remained 0.99999619

Validation

  • test_unified_attention_zero_kv_decode_rows: passed
  • existing BF16 shuffled 3D unified-attention case: passed
  • MiniMax-M3 GSM8K debug, c4, first 8, max 2048: 0/8 before -> 3/8 after; the same result holds with the ATOM-side workaround removed
  • ruff check: passed
  • black --check: passed

Draft for correctness and performance review because the post-op mask adds a small decode kernel.

@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:gfx1250-ffm-triton Run the five-shard gfx1250 FFM Triton test suite
ci:triton-300x Run an additional Triton test job on MI300X in PRs (added automatically when gfx942 configs change); main branch always runs both MI35X and MI300X
ci:triton-355 Run the full Triton test suite on MI35X, not only the tests the change affects
ci:windows Windows JIT op smoke test on the gfx1201 ROCm runner
ci:multi-gpu Aiter multi-GPU tests on the 8-GPU runner
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: DeepSeek-V4-Pro, MiniMax-M3-MXFP4
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 6566 --add-label <label>

One backend per PR:
A PR changes one kernel backend: [Triton/Gluon] (Triton and Gluon count as one), [HIP], [ASM], [CK], [OPUS] or [FlyDSL]. If the title ends up with two backend tags, split the PR -- as stacked pull requests when one part cannot merge without the other.

PR title tags & labels:
Component tags ([Triton/Gluon], [HIP], [CK], [ASM], ...) are added to the PR title and as PR labels automatically from the changed files and re-synced on every push — change-type tags like [fix]/[Perf], op tags like [MLA], and human labels (ci:*) are left untouched. Add the no-auto-title label to stop the title rewrites; labels stay in sync either way.

Co-authored-by: Cursor <cursoragent@cursor.com>
@XiaobingSuper

Copy link
Copy Markdown
Contributor Author

MiniMax-M3 decode on gfx1250 now goes through FlyDSL (ROCm/ATOM#2710), so it no longer reaches unified attention with zero-length padding rows. Closing; happy to reopen if the unified-attention fallback needs this.

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.

1 participant