Skip to content

[ROCm] Fuse Kimi-K3 DSpark QK RoPE with the MLA cache write - #60864

Draft
gronsti-amd wants to merge 17 commits into
vllm-project:mainfrom
gronsti-amd:sgronroo/amd-dspark-fused_qk_rope_concat_and_cache_mla-reimplement
Draft

gronsti-amd wants to merge 17 commits into
vllm-project:mainfrom
gronsti-amd:sgronroo/amd-dspark-fused_qk_rope_concat_and_cache_mla-reimplement

Conversation

@gronsti-amd

@gronsti-amd gronsti-amd commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Apply AITER fused_qk_rope_concat_and_cache_mla to the Kimi-K3 DSpark draft model.
The kernel fuses the QK RoPE, the absorbed query, and the KV cache write into one launch.
This PR fills the hooks introduced in #60054.

The shared op schemas stay unchanged. unified_mla_kv_cache_update is on the compile path of every MLA model and is a splitting op. Positions stay on _rope_positions.

What changes

  • The factory returns KimiK3DSparkMLAWrapper. After super().__init__ it stores the embedding as _epilogue_rotary_emb.
  • When fusion is on, the wrapper skips its own YaRN and passes positions into MLAAttention.forward. positions is None raises RuntimeError. There is no fallback that writes unrotated KV.
  • Prefill context parallel is refused when fusion would otherwise be on. The gather changes the row count while positions stay local.
  • Decode rows are dropped from the generic cache write; the fused kernel writes them. Prefill k_pe is cloned, rotated for the cache, and the caller's tensor stays unrotated so attention can rotate it again in _prepare_mha_inputs.
  • q_out is FP8 when the KV cache is quantized and supports_quant_query_input is set (ROCM_AITER_MLA). Triton MLA clears that flag and keeps bf16. The absorbed query is made contiguous before the launch.
  • The NoPE identity-RoPE path stays closed when rotary_emb is set, including inside _fused_qk_prep_supported.
  • MLACommonMetadata is imported from vllm.model_executor.layers.attention.mla_attention.

Validation

  • CPU hook tests: fusion refuses PCP; fusion is off without an embedding or without the AITER probe; a decode-only update skips do_kv_cache_update; prefill k_pe is re-rotated for MHA and the caller's key stays unrotated until then; q_out matches the backend query dtype; fusion requires positions.
  • test_fused_qk_rope_concat_requires_fp32_cos_sin asserts the wrapper rejects a bf16 cos/sin table. The probe is false unless VLLM_ROCM_USE_AITER=1 and aiter.fused_qk_rope_concat_and_cache_mla imports. The AMD Basic Models job runs that test in its own pytest process with the flag
    set, and filters it out of the broader command.
  • E2E agentic run of Kimi-K3 with this drafter, compared with the NVIDIA draft module on the same prompts.
  • Perf comparison against the NVIDIA drafter: vllm bench serve / spec-bench over output length × max concurrency, with
{"method": "dspark", "model": "Inferact/Kimi-K3-DSpark", "num_speculative_tokens": 5, "attention_backend": "TRITON_MLA", "draft_sample_method": "probabilistic", "rejection_sample_method": "block"}

Out of scope

  • Changing DeepseekScalingRotaryEmbedding.forward_hip to AITER Triton RoPE. That would apply to every HIP user of the embedding. It can be a follow-up or alternative to this PR.

Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (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.

gronsti-amd and others added 17 commits October 9, 2026 15:06
Keep the NVIDIA module as a re-export so the registry path is unchanged.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
NVIDIA keeps MultiHeadLatentAttention as the cache owner. A platform
override can point context-KV insert and draft layer names at another module.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The defaults keep today's concat and cache write. A subclass can replace
the decode query and the KV rows it writes without a new custom-op argument.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The common module is a new file, so pre-commit lints it in full. The
(list, tuple) form was already on main and fails UP038 once the file moves.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The file was not listed in Buildkite, so the refactor would not be collected on either platform.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
DeepseekV32Attention already assigns a concrete RotaryEmbedding to that name. Typing the base attribute as optional made mypy reject its cache reads.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Replay reads the captured positions and slot-mapping allocations, so a replacement tensor freezes the pointer. Lock the NVIDIA binding class and the default hook returns.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ROCm keeps the wrapper factory and points the KV cache at mla_attn. The copied decoder, context-KV insert, and weight mapper stay in the common module.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
HIP YaRN RoPE returns fp32. The query slice assignment casts q back to bf16, but k_pe stays fp32 and concat_and_cache_mla stores those bits as the KV dtype.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The fused decode launch applies a one-row identity table, so it is only valid when the wrapper has not already rotated. Document the duplicated YaRN setup and bind the construction test to the wrapper signature.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
KimiLinearConfig types the latent sizes as optional. Assert them the same way the NVIDIA K3 MLA constructor does, so the factory arithmetic typechecks.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The post-absorb hooks already split the decode epilogue from MLAAttention.
Fill them from a DSpark subclass so the fp32 AITER kernel stays off the shared attention module.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
AITER MLA expects an FP8 decode query, and the prefill key must be rotated again on the tensor attention actually reads. Refuse fused RoPE when positions are missing or prefill context parallel would gather rows the local positions do not cover.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ForwardContext stores AttentionMetadata, which has no num_decode_tokens. The layer entry is MLACommonMetadata, so cast it before reading the decode count.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
MLACommonMetadata is defined next to MLAAttention. The backends.mla.common
module does not exist, so that import stopped the DSpark tests from collecting.

The bare attention fixture sets pcp_shard_decode_requests, which
update_kv_cache reads before the PCP gather. The prefill re-rotate test
installs a forward context so the MHA hook can read the decode split.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Slicing k_pe builds a new view on each evaluation, so the test compared
two equal tensors that were not the same object.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The AMD Basic Models job invoked test_dspark_mla.py without
VLLM_ROCM_USE_AITER, so the cos/sin dtype test skipped. Run that test
on its own with the flag set.

Signed-off-by: Stig-Arne Grönroos <stig-arne.gronroos@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@mergify

mergify Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @gronsti-amd.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo
Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant