Repository navigation
[ROCm][Kimi-K3][Perf] Fuse MLA decode KV-cache write and Q-prep via AITER - #57640
dllehr-amd merged 5 commits into
Conversation
|
/ci run |
4ec495c to
30573bf
Compare
| and q_dcp_replicated is None | ||
| and not torch.compiler.is_compiling() | ||
| ): | ||
| attn_out = self._try_fused_decode( |
There was a problem hiding this comment.
so instead of a try_fused_decode, let's just move the logic here to the main block so if the requirements are satisfied we always do the fused kv_write+q_prep, else we do mla_attn. You also don't need to check torch.compiler.is_compiling since that'll always be False.
There was a problem hiding this comment.
@Rohan138 Done in 4e333f070b. Apologies that this landed as a force-push over the commit you reviewed... the later fixes are stacked as their own commit.
All three parts:
is_compiling gone.
No more _try_fused_decode returning None. It's now _fused_decode -> torch.Tensor, with the eight return None bail-outs split by when they're actually knowable:
- load-time (DCP, PCP, HiSparse,
q_pad_num_heads, whichW_UKvariant is live, shapes, kv-cache dtype) ->_fused_qk_prep_supported(), evaluated once in__init__ - per-batch -> inline in
forward()
Logic moved into the main block:
attn_metadata = layer = kv_cache = slot_mapping = None
fuse = self._fused_qk_prep and q_dcp_replicated is None
if fuse:
attn_metadata, layer, kv_cache, slot_mapping = get_attention_context(
self.mla_attn.layer_name
)
fuse = (
attn_metadata is not None
and slot_mapping is not None
and kv_cache.numel() > 0
and attn_metadata.num_actual_tokens == attn_metadata.num_decode_tokens
)
if fuse:
attn_out = self._fused_decode(...)
else:
attn_out = self.mla_attn(...)So if the requirements hold we always take the fused kv_write+q_prep, else mla_attn — the only per-batch condition left is that the batch is decode-only, plus the degenerate cases (profile/dummy run, unallocated KV cache, missing slot mapping).
One consequence: _fused_decode now takes attn_metadata, layer, kv_cache, slot_mapping as arguments, since get_attention_context() has to be called in forward() to evaluate the gate. That's why the signature went from 5 args to 9
| return cos, sin | ||
|
|
||
|
|
||
| def _fused_mla_qk_prep_impl( |
There was a problem hiding this comment.
I don't think this needs to be a custom op anymore, since again, torch.compile isn't active. Can you try just folding this into mla.py directly?
There was a problem hiding this comment.
@Rohan138 Done in 4e333f070b — folded into mla.py and the file is deleted.
Removed: direct_register_custom_op, _fused_mla_qk_prep_impl, _fused_mla_qk_prep_fake, the torch.ops.vllm.kimi_k3_fused_mla_qk_prep indirection and the fused_mla_qk_prep wrapper. supports_fused_qk_prep() and make_identity_rope_cache() became _fused_qk_prep_supported() and an inline _identity_rope construction on the class.
_fused_decode now calls the kernel directly:
from aiter.ops.cache import fused_qk_rope_concat_and_cache_mla
...
fused_qk_rope_concat_and_cache_mla(
ql_nope.contiguous(),
q_pe,
kv_c_normed,
k_pe.squeeze(1),
kv_cache.view(kv_cache.shape[0], -1, head_size),
q_out,
slot_mapping.flatten(),
layer._k_scale,
layer._q_scale,
positions,
cos_cache,
sin_cache,
is_neox=True,
is_nope_first=True,
)Net across both comments: -249/+91, and the PR is now a single file.
Re-validated on the stock nightly after the refactor — GSM8K 0.9545 strict / 0.9553 flexible, and the kernel census still shows 144x fuse_qk_rope_concat_and_cache_mla with zero concat_and_cache_mla and zero triton_poi_fused.
…ITER On a stock build, Kimi-K3 MLA decode runs two kernels per layer per step: the fp8 KV-cache write (`vllm::concat_and_cache_mla_kernel`) and the `[ql_nope | q_pe]` concat plus that query's static fp8 quant, which the `mla_decode_concat_quant_fp8` custom op emits as a single `triton_poi_fused__to_copy_cat_clamp_mul_reciprocal_view` kernel. Collapse both into one AITER `fused_qk_rope_concat_and_cache_mla` launch. Measured on MI355X at TP=8, plain `vllm serve` on the stock upstream nightly with no speculative decoding and no compilation-config override: 288 -> 144 launches and 1351.8 -> 682.8 us per rank over 6 decode steps, i.e. 9.39 -> 4.74 us per MLA layer per decode step. End-to-end serving throughput is unchanged within measurement noise and this change does not claim a throughput win. K3 MLA is NoPE, while the kernel always applies RoPE and takes the cos/sin caches as required arguments. Feed it an identity rotation (cos=1, sin=0) so the rotation is a no-op -- the approach ROCm/ATOM uses for K3. The cache is a single row: the `_opt` decode kernel clamps `pos` into `[0, cos_cache.size(0))`, so real positions pass through untouched and K3's 1M max_position_embeddings does not cost ~268 MB to express a no-op. AITER's general decode kernel does not clamp, so the head counts that would select it are excluded. Kept entirely in `vllm/models/kimi_k3/amd/mla.py`, mirroring the q/kv RMSNorm fusion this wrapper already owns (vllm-project#52494); no shared MLA code is touched, so this carries no dependency on vllm-project#47757 and cannot affect any other model or backend. Everything that is fixed at load time -- platform, KV dtype, shapes, DCP/PCP/HiSparse, head padding and the W_UK variant -- is resolved once in `_fused_qk_prep_supported()`, so the only per-batch condition is that the batch is decode-only; anything else falls through to the stock `MLAAttention` path. Signed-off-by: Rita Brugarolas Brufau <rita.brugarolasbrufau@amd.com> Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Rita Brugarolas Brufau <rita.brugarolasbrufau@amd.com>
30573bf to
4e333f0
Compare
|
Thanks @Rohan138 for the comments. I have made the edits and updated the PR description |
| from vllm.model_executor.layers.layernorm import RMSNorm | ||
| from vllm.model_executor.layers.mla import MultiHeadLatentAttentionWrapper | ||
| from vllm.platforms import current_platform | ||
|
|
||
| # AITER dispatch constants, mirrored from csrc/kernels/cache_kernels.cu. With all |
| """Kimi-K3 MLA wrapper with eager AITER q/kv RMSNorm fusion and a fused | ||
| decode Q-prep path. | ||
|
|
||
| Both fusions are kept here rather than in the shared MLA layer so only AMD |
| self._identity_rope: tuple[torch.Tensor, torch.Tensor] | None = None | ||
|
|
||
| def _fused_qk_prep_supported(self) -> bool: | ||
| """Whether this layer can ever take the fused decode path. |
| """ | ||
| attn = self.mla_attn | ||
| if not ( | ||
| current_platform.is_rocm() |
There was a problem hiding this comment.
platform will always be rocm under this path, similarly you can also skip checking fp8_ds_mla since that's NV deepgemm-only.
| # Only the W_UK variants reproduced in _fused_decode are supported. | ||
| return attn.is_aiter_triton_fp4_bmm_enabled or ( | ||
| not attn.is_aiter_triton_fp8_bmm_enabled | ||
| and not attn.is_amx_bmm_enabled |
amd-mghanimi
left a comment
There was a problem hiding this comment.
nice work. I just highlighted potential improvements over aiter_ops imports.
| the concat and quant already arrive as one inductor-fused kernel, so | ||
| this collapses two launches into one. | ||
| """ | ||
| from aiter.ops.cache import fused_qk_rope_concat_and_cache_mla |
There was a problem hiding this comment.
it seems @xaguilar-amd had prior attempts to import this kernel in aiter_ops:
maybe we can refactor to something like that?
There was a problem hiding this comment.
answered below
| # path feeds its concat+quant, because this only replaces what happens | ||
| # after the bmm. Both variants return bf16, which the kernel consumes. | ||
| if attn.is_aiter_triton_fp4_bmm_enabled: | ||
| from aiter.ops.triton.batched_gemm_a16wfp4 import batched_gemm_a16wfp4 |
There was a problem hiding this comment.
we have a better import option already:
Line 3405 in 0549e8d
There was a problem hiding this comment.
Thanks @amd-mghanimi
batched_gemm_a16wfp4: the wrapper takes the output tensor as a required
argument, so it fits _v_up_proj (which already has out to hand) but not the
decode-absorb site I'm mirroring, which lets the kernel allocate:
vllm/vllm/model_executor/layers/attention/mla_attention.py
Lines 1027 to 1037 in 92b40f5
This PR's correctness argument is that ql_nope is bit-identical to what the
unfused path feeds its concat+quant, so I'd rather keep this call than add a caller-allocated buffer now.
fused_qk_rope_concat_and_cache_mla: agreed it belongs in _aiter_ops, but
it isn't on main yet — the binding you linked is one #47757 adds. Using it
now means either editing _aiter_ops.py here (shared code, conflicting with
#47757 adding the same method) or depending on #47757, and this PR was
restructured to have neither so it can merge standalone.
My proposal would be once #47757 lands and we decide on the refactor @Rohan138 is proposing below I'll do both in one follow-up. How does that sound @amd-mghanimi @Rohan138 ?
| slot_mapping, | ||
| ) | ||
| else: | ||
| attn_out = self.mla_attn( |
There was a problem hiding this comment.
Honestly I wonder if we should just do a clean aiter mla impl for Kimi similar to https://github.com/vllm-project/vllm/blob/main/vllm/models/kimi_k3/nvidia/mla.py
Then you don't need a separate self._fused_decode that does GEMM+RoPE+kv_cache+q_prep+MLA+out_proj, you can just do something like:
if supported:
fused_qk_rope_concat_and_cache_mla
else:
rope()
kv_cache_write()
q_prep()
Within your mla function. This may be out of scope for this PR, cc @dllehr-amd
There was a problem hiding this comment.
@Rohan138 @dllehr-amd I thought about that too, my 2 cents:
- the mla_attention.py has lots of commits over time(sparse, DCP,PCP…) and doing mla.py full on amd side would require track those by hand, right?. With the fused_path weight prep, the backend impl behind
forward_mqa,_v_up_proj, the KV cache spec and every non-fused batch (prefill, mixed,..) goes through unchanged mla_attention.py. Current solution might be lower maintenance at the expense of little less readability.
-For nvidia might be more justified because they diverge from the shared implementation more
Thoughts? please correct me if I am misunderstanding
Per review on vllm-project#57640: - remove the three explanatory comment blocks - drop `current_platform.is_rocm()`: kimi_k3/__init__.py only imports the amd/ wrapper under is_rocm(), so it cannot be instantiated elsewhere - drop the `fp8_ds_mla` check: only FLASHMLA_SPARSE and FLASHINFER_MLA_SPARSE_SM120 promote to that dtype, both NVIDIA-only - drop `is_amx_bmm_enabled`: set only by AMXMLAImpl, which is CPU-only All four gates are unreachable on this path, so the fused path's behavior is unchanged. Verified by kernel census on 8x MI355X TP=8: the fused kernel still fires 144x (24 MLA layers x 6 decode steps) with zero concat_and_cache_mla and zero triton_poi_fused, matching the pre-change build. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Rita Brugarolas Brufau <rita.brugarolasbrufau@amd.com>
Rohan138
left a comment
There was a problem hiding this comment.
LGTM! Can do further refactoring in follow-ups per discussion in comments
|
✅ @rbrugaro-amd, CI is now available for this PR.
|
|
/ci run |
1 similar comment
|
/ci run |
|
❌ @okorzh-amd, Only reviewers with write access can use CI commands before CI is delegated to the PR author. |
|
/ci run |
|
/ci run |
|
✅ Triggered Buildkite CI #91746 for commit |
|
✅ CI is already running for this commit: https://buildkite.com/vllm/ci/builds/91746 |
dllehr-amd
left a comment
There was a problem hiding this comment.
Looks good! THanks @rbrugaro-amd
Purpose
On ROCm, Kimi-K3 MLA decode runs the per-layer query preparation and the KV-cache
write as separate launches:
vllm::concat_and_cache_mla_kernel— the fp8 KV-cache writetriton_poi_fused__to_copy_cat_clamp_mul_reciprocal_view_0— the[ql_nope | q_pe]concat and that query's static fp8 quantThis collapses both into a single AITER
fused_qk_rope_concat_and_cache_mlalaunch: 288 → 144 kernel launches and 1351.8 → 682.8 µs (1.98×) per rank over
6 decode steps, i.e. 9.39 → 4.74 µs per MLA layer per decode step on
MI355X at TP=8.
Kimi-K3 MLA is NoPE (
mla_use_nope), while that kernel always applies RoPE andtakes the cos/sin caches as required arguments. It is fed an identity rotation
(
cos = 1, sin = 0) so the rotation degenerates to a no-op — the same approachROCm/ATOM uses to route K3 through this kernel.
The identity cache is a single row. AITER's per-head and
_optdecode kernelsclamp
posinto[0, cos_cache.size(0))before indexing, so real positions passstraight through; a full-length constant cache would otherwise cost ~268 MB at
K3's
max_position_embeddingsof 1 048 576 to express a provable no-op. AITER'sgeneral decode kernel does not clamp, so the layer refuses
the head counts that would select it (
kv_lora_rank * num_heads < 2048, i.e.≤3 heads/rank, reachable at TP=32).
Scope, and why this is small
This is a kernel-count / device-time optimization on the MLA decode path. The
two replaced kernels are a sub-1% slice of a decode step, on 24 MLA layers in a
model that is roughly 1:3 MLA:KDA. End-to-end serving throughput improvement is below 1%
Relationship to #47757 and #57452
This supersedes #57452, which I opened for the same optimization and will
close in favor of this one. That PR stacked on #47757: it carried #47757's six
commits so CI could run, and extended its hook in the shared
MLAAttention.forward_impl, also touchingvllm/model_executor/layers/mla.py. Two problems with that shape:radius of a Kimi-K3-only optimization
This PR drops both. It touches no shared code at all — only
vllm/models/kimi_k3/amd/— mirroring the vendor-local q/kv RMSNorm fusion thatthe same wrapper already owns (#52494, merged). Consequences:
#47757 remains complementary: it fuses the sparse MLA path (deepseek_v32)
through the shared hook, while this fuses the dense Kimi-K3 path inside the
vendor wrapper. Different call sites, no conflict in either merge order.
Updated after review (thanks @Rohan138) — the fused call is no longer a
registered custom op, the
torch.compiler.is_compiling()guard is gone, and thebranch now lives directly in
forward()instead of a helper returningNone.That folded the separate ops module away, so this is now one file. All numbers
below were re-measured on the refactored code.
On
is_compiling(): the baseline'striton_poi_fused…kernel comes frommla_decode_concat_quant_fp8compiling its ownforward_native(class docstringat
mla_attention.py:1541), not from the model being compiled — Kimi-K3 is not@support_torch_compile. The guard was dead code.Why not a duplicate
and the NoPE handling is new.
layers/mla.py.graph-rewrite pass. Kimi-K3 is not
@support_torch_compile, so those never fire.Implementation
One file,
vllm/models/kimi_k3/amd/mla.py, on the existing K3 wrapper:_fused_qk_prep_supported()resolves everything fixed at load time —platform, KV-cache dtype, shapes, DCP/PCP/HiSparse, head padding, and which
W_UKvariant is live. Evaluated once in__init__._fused_decode()mirrorsMLAAttention.forward_impl'sW_UKdispatchincluding its precedence (fp4 before fp8; both flags can be set at once) so
ql_nopeis bit-identical to what the unfused path feeds its concat+quant,then issues the fused call and continues through
impl.forward_mqaand_v_up_proj.With the static gating hoisted into
__init__, the only per-batch condition leftin
forward()is that the batch is decode-only. Everything else takes thestock
MLAAttentionpath, unchanged:so a mixed batch's prefill rows would never reach the KV cache)
slot < 0rows, which would then be garbage after thequery all-gather), PCP and HiSparse (both rewrite the KV inputs/slots
before the standalone write, which this path bypasses)
W_UKvariants, and anyq_pad_num_headshead paddingnon-clamping general decode kernel
Test Plan
All numbers below are from the stock upstream nightly
vllm/vllm-openai-rocm:nightly-rocm100-0bfc7a15d095fe83ecc82b50561a93c177fece2d(2026-09-16), 8× MI355X, Kimi-K3 fp4, TP=8, with no benchmark harness and no
vendor patches — plain
vllm serve+vllm bench serve:Server (identical for both arms)
VLLM_ROCM_USE_AITER=1is the only environment variable set.Perf
Kernel census
Add
--profiler-configto the same serve command, then bracket a decode phase:Accuracy
lm_eval --model local-chat-completions --apply_chat_template \ --tasks gsm8k --output_path out --log_samples \ --model_args model=moonshotai/Kimi-K3,base_url=http://127.0.0.1:8010/v1/chat/completions,\ api_key=EMPTY,eos_string='</s>',num_concurrent=32,tokenized_requests=False,max_length=32768 \ --gen_kwargs max_tokens=4096,temperature=0,top_p=1Results
1. Decode kernel census — two kernels become one
Rank-0 torch profile, 6 engine steps, 24 MLA layers,
144 = 24 × 6:vllm::concat_and_cache_mla_kerneltriton_poi_fused__to_copy_cat_clamp_mul_reciprocal_view_0aiter::fuse_qk_rope_concat_and_cache_mla_kernel288 → 144 launches, 1351.8 → 682.8 µs = 1.98×.
Per MLA layer per decode step: 9.39 → 4.74 µs.
Dispatch lands on AITER's
_optdecode kernel rather thanper_head—per_headrequireskv_cache.size(1) == 1and vLLM derives 1536 here. The_optgate is
kv_lora_rank × num_heads ≥ 2048; at TP=8 that is 512 × 12 = 6144._optis also the position-clamping kernel the single-row identity cache needs.2. GSM8K — no accuracy regression
Full 1319 questions, 5-shot,
temperature=0, top_p=1,max_tokens=4096,chat template applied:
3. E2E serving — unchanged within noise
ISL 1024 / OSL 8192 C=1|8|32 one repeat showed TPOT p50 (ms), ITL p50 (ms), ITL p90 (ms), out tok/s for Conc 1 and 8 are flat to within 0.2%. Conc 32 moves ~0.7% in the baseline's favor, having moved ~1% the other way before the refactor — same essential code, opposite sign, so nothing is claimed in either direction.
This is the expected outcome. The fusion removes a sub-1% slice of a decode
step's device time; for scale,
cross_device_reduce_2stagealone is ~46% ofrank-0 decode device time in the same trace.