Skip to content

fix(sparse-mla-sm120): add TOPK=256 DSV4 prefill instantiation - #3834

Closed
waynehacking8 wants to merge 1 commit into
flashinfer-ai:mainfrom
waynehacking8:wayne/fix-3828-prefill-topk256
Closed

waynehacking8 wants to merge 1 commit into
flashinfer-ai:mainfrom
waynehacking8:wayne/fix-3828-prefill-topk256

Conversation

@waynehacking8

@waynehacking8 waynehacking8 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Description

The SM120 sparse-MLA DSV4 prefill dispatch (dispatch_dsv4_single in csrc/sparse_mla_sm120_prefill.cu) only instantiates topk in {128, 512, 1024, 2048}, so topk=256 (a valid 4 x BI(64) shape, hit by DeepSeek-V4 + DSpark configs) fails with Unsupported sparse-MLA prefill configuration: model=DSV4 num_heads=32 topk=256 .... This adds the missing topk == 256 branch plus test coverage.

This is the prefill half of #3828; PR #3817 covers the decode half. No overlap: this PR touches only the prefill .cu, and the test configs are distinct from #3817's decode entries in the same test file (trivial rebase for whichever lands second).

Compute-mode choice: BF16, not FP8. Measured on an RTX PRO 6000 (SM120, CUDA 13), BF16 beats FP8 at topk=256 for every shape tested, consistent with the existing small-K-loop comment (the FP8 Q-quantize prologue doesn't amortise at K=256):

num_heads num_tokens FP8 median BF16 median
32 128 53.7 us 51.0 us
32 1024 278.5 us 254.0 us
32 4096 991.2 us 910.6 us
128 128 141.3 us 127.0 us
128 1024 958.5 us 862.2 us
128 4096 4442.1 us 4330.8 us

Verified on the same card: the issue's prefill repro (num_heads=32, topk=256, num_tokens=128) fails on current main and runs + matches the reference with this change.

Related Issues

Prefill portion of #3828 (decode portion: #3817).

Pull Request Checklist

Pre-commit Checks

  • I have installed pre-commit by running pip install pre-commit (or used your preferred method).
  • I have installed the hooks with pre-commit install.
  • I have run the hooks manually with pre-commit run --all-files and fixed any reported issues.

Tests

  • Tests have been added or updated as needed.
  • All tests are passing (unittest, etc.): new configs (16, 256) and (32, 256) exercise the NHG=1 and NHG=2 routes; full tests/attention/test_sparse_mla_sm120.py = 145 passed on SM120.

Reviewer Notes

dispatch_dsv4_dual is intentionally untouched: the reporter's trace has topk_extra=0, which routes through dispatch_dsv4_single.

Summary by CodeRabbit

  • Bug Fixes
    • Improved support for a 256 top-k setting in sparse MLA dispatch, preventing valid configurations from falling back unexpectedly.
    • Expanded coverage for additional head-count and top-k combinations to better verify SM120 sparse-MLA behavior.

dispatch_dsv4_single only instantiated topk in {128, 512, 1024, 2048};
topk=256 (a valid 4x BI(64) shape, used by DeepSeek-V4 DSpark configs)
fell through to the unsupported-configuration error in the prefill
orchestrator. Add the 256 branch with BF16 compute mode: measured on an
RTX PRO 6000 (SM120), BF16 beats FP8 at topk=256 for every tested shape
(nh 32/128 x nt 128/1024/4096, 2.5-9% faster), matching the existing
small-K-loop comment - the FP8 Q-quantize prologue doesn't amortise at
K=256. Complements PR flashinfer-ai#3817, which adds the decode half of flashinfer-ai#3828.

Test configs (16,256) and (32,256) cover the NHG=1 and NHG=2 routes;
full tests/attention/test_sparse_mla_sm120.py passes (145 tests).

AI-assisted.

Signed-off-by: waynehacking8 <waynehacking8@gmail.com>

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request adds support for a topk value of 256 using the BF16 data type in the sparse MLA prefill CUDA kernel dispatch logic. It also updates the test configurations in test_sparse_mla_sm120.py to include cases with topk=256 (specifically, (16, 256) and (32, 256)). There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f13e89e5-ff27-4feb-b17f-f983ef2be014

📥 Commits

Reviewing files that changed from the base of the PR and between c53229e and 0c5fda5.

📒 Files selected for processing (2)
  • csrc/sparse_mla_sm120_prefill.cu
  • tests/attention/test_sparse_mla_sm120.py

📝 Walkthrough

Walkthrough

Adds a new dispatch branch for topk == 256 in the DSV4 single-cache prefill dispatcher, routing it through the existing BF16 DISPATCH_BY_NH_CM macro. Corresponding test parametrization is extended with (16, 256) and (32, 256) configuration tuples.

Changes

topk=256 Dispatch Support

Layer / File(s) Summary
DSV4 dispatcher branch for topk=256
csrc/sparse_mla_sm120_prefill.cu
Adds a conditional branch handling topk == 256 in the single-cache dispatcher, routing it through the BF16 DISPATCH_BY_NH_CM macro instead of falling through.
Test parametrization update
tests/attention/test_sparse_mla_sm120.py
Extends _DSV4_PREFILL_CONFIGS with (16, 256) and (32, 256) tuples to exercise the new dispatch path.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding the missing TOPK=256 DSV4 prefill instantiation.
Description check ✅ Passed The description follows the template well and includes the change summary, related issues, checklist items, tests, and reviewer notes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ormandj

ormandj commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Tested together with #3817 on SM120 (details in my comment there: 4 quality runs of a 35-conversation agent workload with per-turn degeneration scanning, 0 degenerate turns / 0 parser failures, decode benched cc1-64, several days of serving). Both needed as a pair for dspark on these cards.

@waynehacking8

Copy link
Copy Markdown
Contributor Author

Thanks for running this at that scale -- a multi-day serving soak with per-turn degeneration scanning is exactly the evidence this pair needed on real SM120 hardware. Heads up that OsakaTX's #3896 implements the same dispatch branch with FP8 compute mode (per the in-source comment convention); this PR picked BF16 CM from measurement on RTX PRO 6000 (2.5-9% faster than FP8 at all six shapes I benched, numbers in the PR body), so whichever the maintainers take, the CM choice is worth a deliberate look.

@ormandj

ormandj commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Reverified the carried TOPK=256 prefill instantiation against current vLLM main d973cce3ca with the 0.6.14 wheel plus #3817/#3834 source snapshots.

DeepSeek-V4-Flash-DSpark, 2x RTX PRO 6000 Max-Q (SM120), TP2. Exact server-tokenizer targets: 8,194 tokens in 0.990 s (8,274 tok/s, n=10); 65,538 in 7.956 s (8,238 tok/s, n=2); 131,074 in 17.461 s (7,507 tok/s, n=1). One 960,164-token prompt also completed without error.

Benchmark SHA256: fa227030012a8b55545af6b6a50fa4adcbdff8d003bb16469dc8e2de024ed0c0.

Current Test Results Summary failure is an authorization gate, not a test failure. Its log says CI is skipped pending an approved @flashinfer-bot run comment from a member of @flashinfer-ai/ci-users.

@OsakaTX

OsakaTX commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Confirming this on GB10 (sm_121a) as well. I benchmarked the BF16 dispatch here against an FP8 variant of the same topk == 256 branch, built on a shared checkout so the compute mode is the only variable, with clean JIT recompiles and both (16, 256) and (32, 256) prefill correctness tests passing. BF16 is faster than or equal to FP8 across the whole grid: about 10 to 19% faster at short prefills (128 to 256 tokens) where the kernel is latency-bound, narrowing to roughly parity at long prefills (1024 to 2048 tokens). FP8 showed no throughput advantage. Full numbers are in #3896, which I closed in favor of this. BF16 is the right default here. Thanks @waynehacking8.

@waynehacking8

Copy link
Copy Markdown
Contributor Author

Thanks for closing the loop with the FP8 comparison on GB10 - good to have the CM choice backed by measurements on both sm_120 and sm_121 now.

@ormandj

ormandj commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Head 0c5fda59 adds the missing TOPK=256 prefill branch for (16,256) and (32,256) as the prefill companion to #3817. This PR reports those reference cases and 145 passing tests on SM120; independent SM121 results reported in #3896 also support BF16 over the superseded FP8 alternative, and #3896 was closed in favor of this PR. As previously reported on #3817, the carried pair also completed long-context SM120 TP2 DSpark serving without the prior missing-instantiation failure. This is functional evidence, not an isolated throughput claim. There are no unresolved review threads; the current failed summary reflects skipped, authorization-gated jobs. Requesting authorized CI and maintainer review for merge.

@saltyminty

Copy link
Copy Markdown
Collaborator

This PR is superseded by and will be closed after #4380

bkryu pushed a commit that referenced this pull request Aug 8, 2026
## 📌 Description

This PR consolidates the community work needed to run
DeepSeek-V4-Flash-0731 with DSpark through the SM120/SM121 sparse MLA
backend.

A downstream DSpark configuration has 133 active sparse entries per
query: 128 sliding-window entries plus 5 draft entries. The index tensor
is padded to 192 because the kernel consumes 64-entry tiles. Other
integrations use the 256 bucket. Neither shape was fully represented in
the existing SM120 decode and prefill dispatch matrices, so decode-sized
requests could miss the standalone kernel and fall through to the
prefill-only orchestrator, which aborts on `num_tokens <= 64`.

This change:

- adds DSV4 decode instantiations for `topk=192` and `topk=256` with 8,
16, 32, 64, and 128 query heads;
- adds DSV4 single-cache prefill dispatch for `topk=192` and `topk=256`
using BF16 QK;
- supports TP8 prefill by padding H8 into the existing 16-head MG tile
while gating all global Q, sink, output, and LSE accesses;
- enables the padded H8 MG path for both single-cache and dual-cache
prefill, including both supported secondary-cache page sizes and runtime
top-k lengths;
- keeps the Python and CUDA dispatch matrices and user-facing
documentation synchronized;
- raises an actionable Python `ValueError` when an unsupported decode
shape would otherwise enter the prefill-only kernel and abort the
process;
- fixes the sparse MLA benchmark KV cache view to match its default HND
layout; and
- expands correctness and benchmark coverage for the new dispatch
shapes, active-length truncation, HND/NHD layouts, caller-owned
workspaces, and output-buffer reuse.

## 🔍 Related Issues and Community PRs

After coordination with the maintainers, this PR is intended as the
single consolidated replacement for the overlapping open
implementations:

- #4309 — DSV4 `topk=192` decode/prefill and active-length coverage.
- #4372 — broad DSV4 `topk=256` decode/prefill dispatch and public API
coverage.
- #3989 — combined DSpark `topk=256` fix and benchmark HND layout
correction.
- #3817 — DSV4 `topk=256` decode instantiations and decode-to-prefill
fail-fast behavior.
- #3834 — DSV4 `topk=256` BF16 prefill dispatch.

It also covers the implementation scope of the earlier closed
alternatives #3896 and #3923. Related reports include #3828, #3988, and
#4336.

The NVFP4 KV-cache extension in #3937 is a separate feature and is not
superseded by this PR.

## 🚀 Pull Request Checklist

### ✅ Pre-commit Checks

- [x] I used the repository-compatible `prek` hook runner.
- [x] All hooks relevant to the changed files pass, including
clang-format, mypy, ruff check, and ruff format.
- [x] `git diff --check` passes.

## 🧪 Tests

- [x] Tests have been added or updated as needed.
- [x] `pytest -q tests/attention/test_sparse_mla_sm120.py`: 309 passed
on SM120.
- [x] `compute-sanitizer --tool memcheck` on representative H8
single-cache, dual-cache PBS64, dual-cache PBS2, and runtime-length
cases: 4 passed, `ERROR SUMMARY: 0 errors`.
- [x] The complete `benchmarks/bench_sparse_mla_sm120.py` sweep runs
successfully, including the corrected HND path and H8/H16 single- and
dual-cache cases.

## Reviewer Notes

The main review focus is the padded H8 MG specialization. The kernel
still computes a 16-head tile, but only the first 8 rows may touch
caller-owned global tensors; the remaining rows are zero-filled in
shared memory and excluded from sink, output, and LSE accesses. Tests
cover sinks, active-length truncation, the public HND API, dual-cache
full-tile and runtime-length paths, and both secondary-cache page sizes.

AI tools were used to assist with implementation, testing, and PR
preparation.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Expanded sparse MLA support for additional head counts and top-k
values, including 192, 256, and 2048.
* Added support for eight-head configurations and more prefill,
token-count, page-size, and dual-cache scenarios.
  * Improved compatibility with HND and NHD layouts.

* **Bug Fixes**
  * Unsupported decode shapes are now rejected before processing.
  * Improved truncation handling for top-k and extra top-k lengths.

* **Tests**
* Expanded coverage for layouts, buffer reuse, workspace, sequence
lengths, decode, and prefill scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: David Orman <ormandj@corenode.com>
Co-authored-by: JimZhang-lab <60953754+JimZhang-lab@users.noreply.github.com>
Co-authored-by: Albert Catalan Tatjer <ac.tatjer@gmail.com>
Co-authored-by: carlosmolina0615 <carlosmolina0615@gmail.com>
Co-authored-by: Wei-Cheng (Wayne) Chiu <waynehacking8@gmail.com>
Co-authored-by: OsakaTX <OsakaTX@users.noreply.github.com>
Co-authored-by: Martin Vit <martin@voipmonitor.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants