Repository navigation
[Bugfix] GLM-5.3-Flash: keep kpool top-k rows a valid prefix - #64339
Closed
MatthewBonanni wants to merge 1 commit into
Closed
MatthewBonanni wants to merge 1 commit into
MatthewBonanni wants to merge 1 commit into
Conversation
The kpool indexer's pooled path anchored the incomplete-pool tail at column index_topk. When a row has fewer than index_topk / index_kpool complete pools, that left -1 gaps between the selected pools and the tail. Sparse MLA backends read only the first valid-count entries, so they attended -1 slots and dropped the tail; FlashAttention MLA sparse uses them as an FA3 page table and hit an illegal memory access with HiSparse. FULL CUDA graphs always take this path for decode because the short-context exact-causal shortcut is decided on host at capture. Anchor the tail right after the selected pools, in both platform kernels and the pure-torch reference. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
MatthewBonanni
marked this pull request as ready for review
October 28, 2026 19:21
MatthewBonanni
requested review from
dllehr-amd,
hongxiayang and
tjtanaa
as code owners
October 28, 2026 19:21
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Make GLM-5.3-Flash's kpool indexer list each top-k row's valid entries first, with no
-1gaps. This is a prerequisite for Flash + HiSparse (#61243): HiSparse's top-k conversion keeps entries in place, so the gaps reachFLASH_ATTN_MLA_SPARSE, which crashes with an illegal memory access on Hopper.Claims
FLASH_ATTN_MLA_SPARSE(H200, CUDA graphs) now serves instead of failing every request withCUDA error: an illegal memory access.minper row.Validation
H200,
zai-org/GLM-5.3-Flash@eb9eb208eb0d988989d07a6a12d0fdeb5f52574a, TP4,FULL_AND_PIECEWISE, on top of #61243 head (dac3a3b). Log showsUsing FLASH_ATTN_MLA_SPARSE attention backend. GSM8K 5-shot completions, greedy, one server per row.With HiSparse:
Without HiSparse (same command,
--attention-config '{"backend":"FLASH_ATTN_MLA_SPARSE"}', no connector), this PR leaves the output unchanged:Unit (H200):
pytest tests/v1/attention/test_sparse_indexer_decode_seq_lens.py tests/models/glm5next/→ 41 passed, 8 skipped. The newtest_pooled_rows_keep_valid_prefixfails without the kernel change. CI runs it in the V1 Attention jobs (H100/B200 plus the AMD mirrors). The AMD kernel copy got the same one-line change; only those AMD CI jobs exercise it, not MI3xx hardware.Details
The kpool indexer's pooled path anchored the incomplete-pool tail at column
index_topk. When a row has fewer thanindex_topk / index_kpoolcomplete pools, this leaves-1gaps between the selected pools and the tail. The exact-causal shortcut formax_seq_len <= index_topkavoids these gaps, but it is a host check. FULL CUDA graphs capture withmax_seq_len = max_model_len, so decode replay always takes the pooled path.Without HiSparse the gaps are harmless.
triton_convert_req_index_to_global_indexmoves valid entries to the front whenever it returnsvalid_counts(#46076). HiSparse'shisparse_resolve_residencykeeps entries in place and assumes the indexer's rows have no gaps. With HiSparse, instrumented graph replay showed 288 decode rows per layer with-1inside[0, valid_count). FA3 uses those entries as its page table, so it reads row-1.The gappy layout dates to #53906. It only reaches an attention kernel through HiSparse, which #61243 enables for Flash.
The fix anchors the tail at
min(pool_len * kpool, topk)in the NVIDIA and AMD fused kernels and in the pure-torch reference. Short rows now match the exact-causal fill, and full rows are unchanged.fit_kpool_indices_to_aiterstays correct with either layout.Not a duplicate: no open PR changes the kpool tail layout.
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (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.
🤖 Generated with Claude Code