Repository navigation
[Performance][KDA] Reduce preprocessing copies and redundant output masks - #16067
Conversation
Signed-off-by: Dawn952 <zhaojunbo13@huawei.com>
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [Feature] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for non-contiguous Q/K/V inputs in the recurrent KDA operator, significantly improving performance by avoiding unnecessary memory copies. The changes involve updating the operator definition to relax contiguous requirements, enhancing the host-side tiling logic to resolve and validate strides, and modifying the underlying kernels to utilize these strides for direct memory access. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Attention][Feature] Support non-contiguous QKV views in recurrent KDASuggested PR Summary:
### What this PR does / why we need it?
This pull request adds support for non-contiguous Query, Key, and Value (QKV) views in the recurrent KDA operator. It allows the operator to directly consume independent token and head strides for QKV views, avoiding unnecessary contiguous operations. It introduces stride validation, updates tiling logic to pass strides to the kernel, and modifies the Ascend C kernel to handle strided QKV inputs.
Feedback: A high-severity issue was identified in `CheckQkvMaterializationSafety` where the check `tokenStride < minTokenStride` incorrectly rejects valid non-contiguous layouts (like transposed tensors) that could otherwise be safely materialized via `DataContiguous`. Removing this check allows safe fallback to materialization.
### Does this PR introduce _any_ user-facing change?
Yes, the `recurrent_kda` operator now accepts non-contiguous QKV tensors directly without requiring manual `.contiguous()` calls.
### How was this patch tested?
An end-to-end test `test_kimi_k3_tp16_recurrent_kda_non_contiguous_qkv_and_state_pool` was added to verify correctness with non-contiguous QKV views.| const int64_t maxInt64 = std::numeric_limits<int64_t>::max(); | ||
| if (headCount > 1 && headStride > (maxInt64 - featureCount) / (headCount - 1)) { | ||
| OP_LOGE(ACLNN_ERR_PARAM_INVALID, "npu_recurrent_kda: %s head span overflows.", name); | ||
| return false; | ||
| } | ||
| const int64_t minTokenStride = (headCount - 1) * headStride + featureCount; | ||
| if (tokenStride < minTokenStride) { | ||
| OP_LOGE(ACLNN_ERR_PARAM_INVALID, | ||
| "npu_recurrent_kda: %s token/head axis order or overlap is unsupported.", name); | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
The safety check tokenStride < minTokenStride in CheckQkvMaterializationSafety incorrectly rejects valid non-contiguous layouts (such as transposed QKV tensors) and fails the operator with ACLNN_ERR_PARAM_INVALID.
While tokenStride < minTokenStride is indeed unsupported for the direct-view kernel (and is correctly caught by IsSupportedQkvView to trigger materialization), it is completely safe to materialize. Materializing a transposed or non-contiguous tensor via DataContiguous resolves the stride/overlap issues and produces a standard contiguous layout that the kernel can safely execute.
By failing the operator here, you prevent these valid layouts from being materialized. Removing this check from CheckQkvMaterializationSafety allows the operator to safely fall back to materialization for these layouts.
const int64_t maxInt64 = std::numeric_limits<int64_t>::max();
if (headCount > 1 && headStride > (maxInt64 - featureCount) / (headCount - 1)) {
OP_LOGE(ACLNN_ERR_PARAM_INVALID, "npu_recurrent_kda: %s head span overflows.", name);
return false;
}
return true;Signed-off-by: Dawn952 <zhaojunbo13@huawei.com>
…-5.3-Flash (#16251) ### What this PR does / why we need it? Refs #15665 Integrate the existing AscendC recurrent/chunk KDA and causal-convolution operators into GLM-5.3-Flash. GLM and Kimi K3 share the native KDA call helpers in `vllm_ascend/ops/kda.py`, while retaining their model-specific projections, weight loading, beta preparation, cache updates, and metadata handling. Preserve physical-page state views and cache-spec capability detection without changing the cache allocation, grouping, or shared MLA/Mamba tensor ownership from #15913. Mask inactive convolution slots before pointer arithmetic and preserve writeback to noncontiguous state views. The native operators and GDN interfaces are already in main. Full-model validation combines the GLM Flash integration PRs and their prerequisites; prerequisite commits are excluded from this branch. ### Does this PR introduce _any_ user-facing change? Yes. GLM-5.3-Flash uses AscendC KDA and causal convolution. Unsupported operator configurations are rejected during initialization. Kimi K3 retains its existing beta and gate semantics through the shared helpers. ### How was this patch tested? A3 integration validation included PR revision `2fc40a144`: - 320 unit tests passed, 1 skipped, covering GLM/Kimi KDA, cache pages, KeyPool routing, GDN metadata, MTP, ModelSlim, and shared SFA. Updated coverage includes `tests/ut/ops/test_kimi_kda.py` and `tests/ut/models/test_glm5next_kda_contracts.py`. - Four native NPU tests passed, including `tests/e2e/nightly/single_node/ops/singlecard_ops/test_glm5next_conv_state.py` for inactive slots and strided state layouts, plus existing Kimi recurrent/prefill tests. - Twelve comparisons against the prior implementations produced exactly equal outputs and backing-state tensors for GLM/Kimi gate variants, decode, MTP verification, and 65/131-token prefill. - GLM-5.3-Flash-w8a8 passed 8/8 inference cases, including a 5264-token prompt, with TP8/EP8, FULL_DECODE_ONLY, and MTP=3. The current revision rebases on #16067 and preserves its direct Q/K/V view handling and removal of redundant Kimi output masks. The shared recurrent helper forwards Q/K/V unchanged; GLM and Kimi regression cases assert that contiguous and separately strided views reach the native operator without Python-side materialization. Eight focused GLM/Kimi contract cases passed in an isolated CPU harness executing the actual function bodies and test cases. Ruff, formatting, `git diff --check`, and GitHub pre-commit passed. The hardware results above apply to the earlier integration; this rebased revision has not had a new hardware run. - vLLM main: vllm-project/vllm@a97dacb Signed-off-by: Li Jiahang <216526138+lijiahang226@users.noreply.github.com>
…hado/vllm-ascend into main_fix_mrv2_eagle3_mamba * 'main_fix_mrv2_eagle3_mamba' of https://github.com/windshado/vllm-ascend: (42 commits) Update vllm_ascend/worker/v2/model_states/mamba_hybrid.py [Feature][Kimi K3 DSPark] Enable TP for context_proj (vllm-project#16344) [BugFix][SpecDecode] Refresh replicated PCP draft graph cache mappings (vllm-project#16300) [Feature][Model] Integrate Triton KeyPool indexing for GLM-5.3-Flash (vllm-project#16253) [BugFix][Offloader] Re-bind params to NZ static buffers after npu_format_cast (vllm-project#15415) [Feature][Model] Integrate AscendC KDA and causal convolution for GLM-5.3-Flash (vllm-project#16251) [Performance][Communicator] Replace per-layer F.pad with cat of a persistent zero block in MoE prepare (vllm-project#16343) [Feature][Operator] Add DeepSeek V4.1 sparse attention operators (vllm-project#16422) [Doc][Misc] Document batch invariance scheduling limitations (vllm-project#16232) [CI][MRV2] Enable mrv2 dspark e2e test (vllm-project#16319) [BugFix] Precast MoE gate weight_fp32 to avoid aclop Cast (vllm-project#16189) [Feature][MRV2][310P] MRv2 adapting MTP on the 310P for Qwen3.5 (vllm-project#16043) [Revert] Revert "[Feature][MRV1][MRV2] Refactor Host-Side Parameter Updates for ACL Graph Replay." (vllm-project#15908) (vllm-project#16409) [Feature][Ops] Add Triton KeyPool compression and pooled indexing (vllm-project#16243) [Feature][Attention] Support NoPE in the shared SFA backend (vllm-project#16252) [Performance][Model] Reuse fused mHC operators for GLM-5.3-Flash (vllm-project#16321) [Feature][Model] Enable MiniMax-M3 FP8 MSA index score on A5 (vllm-project#15918) [Performance][KDA] Reduce preprocessing copies and redundant output masks (vllm-project#16067) [Feature][Model][MTP] Support speculative decoding for GLM-5.3-Flash (vllm-project#16214) [BugFix][Model] Skip unused hash-router bias when loading DeepSeek-V4 weights (vllm-project#16259) ...
…asks (vllm-project#16067) ### What this PR does / why we need it? Reduce small-operator overhead around Kimi K3 recurrent KDA on `main` by optimizing preprocessing copies and removing redundant framework padding cleanup after KDA and the output norm gate. - **Before KDA:** consume supported non-contiguous Q/K/V views from the fused projection directly, eliminating forced contiguous copies. Independent token/head strides flow through ACLNN, tiling, and the A2/A3 and A5 kernels. Unsupported layouts still materialize. - **After KDA and norm:** delete `_zero_padded_recurrent_output` on spec/non-spec outputs and `_zero_padded_output` after `o_norm`, including the now-unused live-token count calculation. These removals eliminate the corresponding `arange / comparison / where` chains, profiled as `Range / Less / Fill / SelectV2`. Norm output is copied directly into the existing result buffer. - Preserve state updates, mixed-token index copies, idle/dummy handling, and buffer initialization. No new kernel interface, switch, or replacement mask is needed. The deleted masks sanitize padding after recurrent state computation; the norm gate operates independently per token/head row. Padding rows inside the graph-shaped region no longer have a zero-value guarantee. Effective token computation is unchanged. ### Does this PR introduce _any_ user-facing change? Performance optimization of Kimi K3 KDA preprocessing and postprocessing. The Torch operator schema and contiguous-input behavior remain unchanged. ### How was this patch tested? - Accuracy validation (reported by the project owner): **GPQA Avg@5: 93.03**. - Added spec/decode/mixed framework regression cases with NaN padding, checking effective output rows, state updates, and output-buffer tail initialization. - All three cases passed on each branch in an isolated CPU harness executing the actual `_forward` and `_prepare_beta` bodies with mocked kernel/context dependencies (six cases total). This is not a full package or NPU test run. - Ruff check/format, Python AST parsing, `git diff --check`, and repository forbidden-import, boolean-context-manager, and long-function checks passed. - The project owner previously bypassed the padding masks and reported successful runtime validation. Full-model/NPU validation and profiling were not rerun for these commits. - Full `format.sh ci` was attempted on main and stopped during actionlint environment installation; the full lint suite did not complete. - The direct recurrent-operator regression covers distinct Q/K/V strides, non-contiguous state pools, guard holes, and numerical reference checks. - vLLM main: vllm-project/vllm@b2f6858 --------- Signed-off-by: Dawn952 <zhaojunbo13@huawei.com> Signed-off-by: tianming2009 <13246728590@163.com>
…-5.3-Flash (vllm-project#16251) ### What this PR does / why we need it? Refs vllm-project#15665 Integrate the existing AscendC recurrent/chunk KDA and causal-convolution operators into GLM-5.3-Flash. GLM and Kimi K3 share the native KDA call helpers in `vllm_ascend/ops/kda.py`, while retaining their model-specific projections, weight loading, beta preparation, cache updates, and metadata handling. Preserve physical-page state views and cache-spec capability detection without changing the cache allocation, grouping, or shared MLA/Mamba tensor ownership from vllm-project#15913. Mask inactive convolution slots before pointer arithmetic and preserve writeback to noncontiguous state views. The native operators and GDN interfaces are already in main. Full-model validation combines the GLM Flash integration PRs and their prerequisites; prerequisite commits are excluded from this branch. ### Does this PR introduce _any_ user-facing change? Yes. GLM-5.3-Flash uses AscendC KDA and causal convolution. Unsupported operator configurations are rejected during initialization. Kimi K3 retains its existing beta and gate semantics through the shared helpers. ### How was this patch tested? A3 integration validation included PR revision `2fc40a144`: - 320 unit tests passed, 1 skipped, covering GLM/Kimi KDA, cache pages, KeyPool routing, GDN metadata, MTP, ModelSlim, and shared SFA. Updated coverage includes `tests/ut/ops/test_kimi_kda.py` and `tests/ut/models/test_glm5next_kda_contracts.py`. - Four native NPU tests passed, including `tests/e2e/nightly/single_node/ops/singlecard_ops/test_glm5next_conv_state.py` for inactive slots and strided state layouts, plus existing Kimi recurrent/prefill tests. - Twelve comparisons against the prior implementations produced exactly equal outputs and backing-state tensors for GLM/Kimi gate variants, decode, MTP verification, and 65/131-token prefill. - GLM-5.3-Flash-w8a8 passed 8/8 inference cases, including a 5264-token prompt, with TP8/EP8, FULL_DECODE_ONLY, and MTP=3. The current revision rebases on vllm-project#16067 and preserves its direct Q/K/V view handling and removal of redundant Kimi output masks. The shared recurrent helper forwards Q/K/V unchanged; GLM and Kimi regression cases assert that contiguous and separately strided views reach the native operator without Python-side materialization. Eight focused GLM/Kimi contract cases passed in an isolated CPU harness executing the actual function bodies and test cases. Ruff, formatting, `git diff --check`, and GitHub pre-commit passed. The hardware results above apply to the earlier integration; this rebased revision has not had a new hardware run. - vLLM main: vllm-project/vllm@a97dacb Signed-off-by: Li Jiahang <216526138+lijiahang226@users.noreply.github.com> Signed-off-by: tianming2009 <13246728590@163.com>
…asks (vllm-project#16067) ### What this PR does / why we need it? Reduce small-operator overhead around Kimi K3 recurrent KDA on `main` by optimizing preprocessing copies and removing redundant framework padding cleanup after KDA and the output norm gate. - **Before KDA:** consume supported non-contiguous Q/K/V views from the fused projection directly, eliminating forced contiguous copies. Independent token/head strides flow through ACLNN, tiling, and the A2/A3 and A5 kernels. Unsupported layouts still materialize. - **After KDA and norm:** delete `_zero_padded_recurrent_output` on spec/non-spec outputs and `_zero_padded_output` after `o_norm`, including the now-unused live-token count calculation. These removals eliminate the corresponding `arange / comparison / where` chains, profiled as `Range / Less / Fill / SelectV2`. Norm output is copied directly into the existing result buffer. - Preserve state updates, mixed-token index copies, idle/dummy handling, and buffer initialization. No new kernel interface, switch, or replacement mask is needed. The deleted masks sanitize padding after recurrent state computation; the norm gate operates independently per token/head row. Padding rows inside the graph-shaped region no longer have a zero-value guarantee. Effective token computation is unchanged. ### Does this PR introduce _any_ user-facing change? Performance optimization of Kimi K3 KDA preprocessing and postprocessing. The Torch operator schema and contiguous-input behavior remain unchanged. ### How was this patch tested? - Accuracy validation (reported by the project owner): **GPQA Avg@5: 93.03**. - Added spec/decode/mixed framework regression cases with NaN padding, checking effective output rows, state updates, and output-buffer tail initialization. - All three cases passed on each branch in an isolated CPU harness executing the actual `_forward` and `_prepare_beta` bodies with mocked kernel/context dependencies (six cases total). This is not a full package or NPU test run. - Ruff check/format, Python AST parsing, `git diff --check`, and repository forbidden-import, boolean-context-manager, and long-function checks passed. - The project owner previously bypassed the padding masks and reported successful runtime validation. Full-model/NPU validation and profiling were not rerun for these commits. - Full `format.sh ci` was attempted on main and stopped during actionlint environment installation; the full lint suite did not complete. - The direct recurrent-operator regression covers distinct Q/K/V strides, non-contiguous state pools, guard holes, and numerical reference checks. - vLLM main: vllm-project/vllm@b2f6858 --------- Signed-off-by: Dawn952 <zhaojunbo13@huawei.com> Signed-off-by: like-0517 <ithwlike@126.com>
…-5.3-Flash (vllm-project#16251) ### What this PR does / why we need it? Refs vllm-project#15665 Integrate the existing AscendC recurrent/chunk KDA and causal-convolution operators into GLM-5.3-Flash. GLM and Kimi K3 share the native KDA call helpers in `vllm_ascend/ops/kda.py`, while retaining their model-specific projections, weight loading, beta preparation, cache updates, and metadata handling. Preserve physical-page state views and cache-spec capability detection without changing the cache allocation, grouping, or shared MLA/Mamba tensor ownership from vllm-project#15913. Mask inactive convolution slots before pointer arithmetic and preserve writeback to noncontiguous state views. The native operators and GDN interfaces are already in main. Full-model validation combines the GLM Flash integration PRs and their prerequisites; prerequisite commits are excluded from this branch. ### Does this PR introduce _any_ user-facing change? Yes. GLM-5.3-Flash uses AscendC KDA and causal convolution. Unsupported operator configurations are rejected during initialization. Kimi K3 retains its existing beta and gate semantics through the shared helpers. ### How was this patch tested? A3 integration validation included PR revision `2fc40a144`: - 320 unit tests passed, 1 skipped, covering GLM/Kimi KDA, cache pages, KeyPool routing, GDN metadata, MTP, ModelSlim, and shared SFA. Updated coverage includes `tests/ut/ops/test_kimi_kda.py` and `tests/ut/models/test_glm5next_kda_contracts.py`. - Four native NPU tests passed, including `tests/e2e/nightly/single_node/ops/singlecard_ops/test_glm5next_conv_state.py` for inactive slots and strided state layouts, plus existing Kimi recurrent/prefill tests. - Twelve comparisons against the prior implementations produced exactly equal outputs and backing-state tensors for GLM/Kimi gate variants, decode, MTP verification, and 65/131-token prefill. - GLM-5.3-Flash-w8a8 passed 8/8 inference cases, including a 5264-token prompt, with TP8/EP8, FULL_DECODE_ONLY, and MTP=3. The current revision rebases on vllm-project#16067 and preserves its direct Q/K/V view handling and removal of redundant Kimi output masks. The shared recurrent helper forwards Q/K/V unchanged; GLM and Kimi regression cases assert that contiguous and separately strided views reach the native operator without Python-side materialization. Eight focused GLM/Kimi contract cases passed in an isolated CPU harness executing the actual function bodies and test cases. Ruff, formatting, `git diff --check`, and GitHub pre-commit passed. The hardware results above apply to the earlier integration; this rebased revision has not had a new hardware run. - vLLM main: vllm-project/vllm@a97dacb Signed-off-by: Li Jiahang <216526138+lijiahang226@users.noreply.github.com> Signed-off-by: like-0517 <ithwlike@126.com>
What this PR does / why we need it?
Reduce small-operator overhead around Kimi K3 recurrent KDA on
mainby optimizing preprocessing copies and removing redundant framework padding cleanup after KDA and the output norm gate._zero_padded_recurrent_outputon spec/non-spec outputs and_zero_padded_outputaftero_norm, including the now-unused live-token count calculation. These removals eliminate the correspondingarange / comparison / wherechains, profiled asRange / Less / Fill / SelectV2. Norm output is copied directly into the existing result buffer.The deleted masks sanitize padding after recurrent state computation; the norm gate operates independently per token/head row. Padding rows inside the graph-shaped region no longer have a zero-value guarantee. Effective token computation is unchanged.
Does this PR introduce any user-facing change?
Performance optimization of Kimi K3 KDA preprocessing and postprocessing. The Torch operator schema and contiguous-input behavior remain unchanged.
How was this patch tested?
Accuracy validation (reported by the project owner): GPQA Avg@5: 93.03.
Added spec/decode/mixed framework regression cases with NaN padding, checking effective output rows, state updates, and output-buffer tail initialization.
All three cases passed on each branch in an isolated CPU harness executing the actual
_forwardand_prepare_betabodies with mocked kernel/context dependencies (six cases total). This is not a full package or NPU test run.Ruff check/format, Python AST parsing,
git diff --check, and repository forbidden-import, boolean-context-manager, and long-function checks passed.The project owner previously bypassed the padding masks and reported successful runtime validation. Full-model/NPU validation and profiling were not rerun for these commits.
Full
format.sh ciwas attempted on main and stopped during actionlint environment installation; the full lint suite did not complete.The direct recurrent-operator regression covers distinct Q/K/V strides, non-contiguous state pools, guard holes, and numerical reference checks.
vLLM main: vllm-project/vllm@b2f6858