[Fusion] [Graph] Add qknorm rope fusion operator#4711
[Fusion] [Graph] Add qknorm rope fusion operator#4711wangxiyuan merged 40 commits intovllm-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a graph fusion pass for qknorm_rope operations on Ascend hardware, which is a great step for performance optimization. The implementation includes a new configuration flag, a pattern matching pass using torch._inductor.pattern_matcher, and a custom Triton kernel for the fused operation. The code is well-structured, but I've identified several areas for improvement regarding code quality, robustness, and maintainability. My review comments focus on removing debug artifacts, improving code clarity and consistency, enhancing robustness by avoiding hardcoded values and unsafe module-level initializations, and addressing significant code duplication.
|
👋 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. |
| return q_output, k_output, v_output | ||
|
|
||
|
|
||
| direct_register_custom_op(op_name="qkv_rmsnorm_rope", |
There was a problem hiding this comment.
use import torch_npu._inductor
There was a problem hiding this comment.
The pattern_matcher method of inductor does not support the triton operator. It does support torch.ops.aten (aten operator), torch.ops.npu (custom operator), and torch.add (PyTorch API). Therefore, it is wrapped as a custom op.
| return driver.active.utils.get_device_properties(device) | ||
|
|
||
|
|
||
| num_vectorcore = get_npu_properties()["num_vectorcore"] |
There was a problem hiding this comment.
this parameter has already been defined in triton/utils.py
There was a problem hiding this comment.
Thanks. I have modified it.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
bac8b40 to
59f15a7
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
5a890c9 to
98a4d21
Compare
|
|
||
| return q_rope, k_rope, v | ||
|
|
||
| def replacement(qkv: torch.Tensor, q_weight: torch.Tensor, |
There was a problem hiding this comment.
pattern in 'if xxx else: torch.ops.vllm.qkv_rmsnorm_rope ’ need support in future releases
There was a problem hiding this comment.
We don't perform any special checks in the pattern. You can add a new pattern match.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
3d8050a to
95718dd
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: wxsIcey <1790571317@qq.com>
Signed-off-by: wxsIcey <1790571317@qq.com>
Signed-off-by: wxsIcey <1790571317@qq.com>
Signed-off-by: wxsIcey <1790571317@qq.com>
Signed-off-by: wxsIcey <1790571317@qq.com>
Signed-off-by: wxsIcey <1790571317@qq.com>
Signed-off-by: wxsIcey <1790571317@qq.com>
Signed-off-by: wxsIcey <1790571317@qq.com>
cf04ffc to
dd4617f
Compare
|
Additional config doc should be updated. Please submit a follow-up PR. @wxsIcey |
### What this PR does / why we need it? This PR add `qkv_rmsnorm_rope` operator and introduces a graph fusion pass for `qknorm_rope` operations. The implementation includes a new configuration flag, a pattern matching pass using `torch._inductor.pattern_matcher`, and a custom Triton kernel for the fused operation. Co-authored-by: Angazenn [supperccell@163.com](mailto:supperccell@163.com) ### Does this PR introduce _any_ user-facing change? Yes, add new additional_config - vLLM version: v0.12.0 - vLLM main: vllm-project/vllm@ad32e3e --------- Signed-off-by: wxsIcey <1790571317@qq.com>
### What this PR does / why we need it? This PR add `qkv_rmsnorm_rope` operator and introduces a graph fusion pass for `qknorm_rope` operations. The implementation includes a new configuration flag, a pattern matching pass using `torch._inductor.pattern_matcher`, and a custom Triton kernel for the fused operation. Co-authored-by: Angazenn [supperccell@163.com](mailto:supperccell@163.com) ### Does this PR introduce _any_ user-facing change? Yes, add new additional_config - vLLM version: v0.12.0 - vLLM main: vllm-project/vllm@ad32e3e --------- Signed-off-by: wxsIcey <1790571317@qq.com> Signed-off-by: zrj026 <zhangrunjiang026@gmail.com>
### What this PR does / why we need it? This PR add `qkv_rmsnorm_rope` operator and introduces a graph fusion pass for `qknorm_rope` operations. The implementation includes a new configuration flag, a pattern matching pass using `torch._inductor.pattern_matcher`, and a custom Triton kernel for the fused operation. Co-authored-by: Angazenn [supperccell@163.com](mailto:supperccell@163.com) ### Does this PR introduce _any_ user-facing change? Yes, add new additional_config - vLLM version: v0.12.0 - vLLM main: vllm-project/vllm@ad32e3e --------- Signed-off-by: wxsIcey <1790571317@qq.com> Signed-off-by: zrj026 <zhangrunjiang026@gmail.com>
What this PR does / why we need it?
This PR add
qkv_rmsnorm_ropeoperator and introduces a graph fusion pass forqknorm_ropeoperations. The implementation includes a new configuration flag, a pattern matching pass usingtorch._inductor.pattern_matcher, and a custom Triton kernel for the fused operation.Co-authored-by: Angazenn supperccell@163.com
Does this PR introduce any user-facing change?
Yes, add new additional_config
How was this patch tested?
local test