[BugFix][Kernel] Fix A5 QuantLightningIndexerV2 build dependencies - #16016
Merged
Merged
Conversation
Format QLIV2 shape diagnostics locally to avoid the external Ops::Base::ToString symbol while preserving bracketed output. Declare the LightningIndexerV2 kernel source dependency so its shared header is staged and packaged. Signed-off-by: Foriv <2293567056@qq.com>
Contributor
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 addresses a build failure in the A5 custom-op pipeline caused by missing header dependencies and unresolved external symbols. By explicitly declaring the required kernel source dependency and internalizing the shape formatting logic, the patch ensures that the QuantLightningIndexerV2 operator can be built and loaded correctly in the target environment. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Attention][Misc] Refactor shape logging to use local ShapeToStringForLog in quant_lightning_indexer_v2Suggested PR Summary:
### What this PR does / why we need it?
This PR refactors the shape logging in `quant_lightning_indexer_v2` to use a local helper function `ShapeToStringForLog` instead of the external `Ops::Base::ToString` call. This keeps shape diagnostics consistent and removes external opsbase formatting dependencies. Additionally, it updates `CMakeLists.txt` to define kernel source dependencies for `quant_lightning_indexer_v2`.
Feedback:
- A copy-paste bug was identified in `quant_lightning_indexer_v2_tiling.cpp` where `cuSeqLensK`'s shape is printed twice instead of printing `cuSeqLensQ` and `cuSeqLensK` respectively.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Not specified in the PR.
Comment on lines
+909
to
+910
| ShapeToStringForLog(opParamInfo_.cuSeqLensK.tensor->GetStorageShape()) + " and " + | ||
| ShapeToStringForLog(opParamInfo_.cuSeqLensK.tensor->GetStorageShape()), |
Contributor
There was a problem hiding this comment.
There is a copy-paste bug in the error logging here. The log message indicates a mismatch between cu_seqlens_q and cu_seqlens_k, but the shapes printed are both from cuSeqLensK (opParamInfo_.cuSeqLensK.tensor->GetStorageShape()). The first shape should be printed from cuSeqLensQ (opParamInfo_.cuSeqLensQ.tensor->GetStorageShape()).
ShapeToStringForLog(opParamInfo_.cuSeqLensQ.tensor->GetStorageShape()) + " and " +
ShapeToStringForLog(opParamInfo_.cuSeqLensK.tensor->GetStorageShape()),
Contributor
|
👋 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 [BugFix] 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! |
ZT-AIA
approved these changes
Sep 8, 2026
maoxx241
approved these changes
Sep 8, 2026
jiangli221
pushed a commit
to jiangli221/vllm-ascend
that referenced
this pull request
Sep 9, 2026
…llm-project#16016) ### What this PR does / why we need it? A5 custom-op builds can fail when OPC loads the Host tiling library with an unresolved `Ops::Base::ToString(const gert::Shape&)` reference from QuantLightningIndexerV2. The kernel source staging also misses the sibling LightningIndexerV2 header required by QLIV2. This patch: - Reuses the existing local shape formatter through a small wrapper that preserves bracketed diagnostics, replacing all 53 external formatting calls. - Declares the LightningIndexerV2 kernel source dependency so the shared header is copied into the build tree and included in the operator package. The change is limited to the QLIV2 Host CMake file and tiling source. See the [failing A5 build job](https://github.com/vllm-project/vllm-ascend/actions/runs/34178342519/job/101912219798). ### Does this PR introduce _any_ user-facing change? Yes: affected A5 source builds can complete again. Operator interfaces, numerical computation and the shape diagnostic format are unchanged. ### How was this patch tested? Environment: A5 / aarch64, CANN toolkit 9.1.0 with ops-transformer 9.2.0-beta.2, Python 3.11.10, PyTorch 2.10.0 and torch-npu 2.10.0.post4. On this PR commit: - `COMPILE_CUSTOM_KERNELS=1 SOC_VERSION=ascend950dt_9582 MAX_JOBS=32 OPS_CPU_NUMBER=32 python3 setup.py build` passed. This was an incremental build of the standard 30-operator A5 configuration: all 154 kernel artifacts were packaged and installed, and the Torch extension build completed. - The installed sibling header matches its source. The installed tiling library loads in a fresh process with `RTLD_NOW`; ELF inspection confirms no external Shape `ToString` reference or direct `libops_base.so` dependency. - `bash format.sh ci` passed with a clean worktree before and after. A separate Gitleaks scan of the nonempty PR commit range passed (one commit, no leaks). Separate regression diagnostics against the original CI baseline reproduced the original loader failure before the fix, compared seven SDK/local shape-format cases, and compiled all five QLIV2 kernel configurations. The existing custom-op build exercises the loading and source-staging failure, so no new Python unit test is added. NPU numerical/performance tests and the original GitHub image job have not been rerun. - vLLM main: vllm-project/vllm@e6bfe03 Signed-off-by: Foriv <2293567056@qq.com>
kunpengW-code
pushed a commit
that referenced
this pull request
Sep 10, 2026
### What this PR does / why we need it? Backport of #16016 (merged as 701a449) to `releases/v0.26.0rc`. The release branch contains the same QuantLightningIndexerV2 build dependency issues: an unresolved `Ops::Base::ToString(const gert::Shape&)` reference in Host tiling and a missing LightningIndexerV2 sibling header during kernel source staging. This applies the identical two-file patch without conflicts or release-specific changes: - Declare the sibling kernel source dependency for staging and packaging. - Replace 53 external shape-formatting calls with the existing local formatter through a bracket-preserving wrapper. ### Does this PR introduce _any_ user-facing change? Yes: it fixes these QLIV2 dependencies in affected A5 source builds. Operator interfaces, numerical computation and shape diagnostic formatting are unchanged. ### How was this patch tested? On this backport commit, in the A5 `zrr_dev` container: - `bash format.sh ci` passed all 18 hooks; the worktree was clean before and after. - Gitleaks scanned the nonempty release-base-to-HEAD range (one commit), with no leaks. - Compiled the release QLIV2 Host source before and after the patch into separate objects using CANN 9.1 compiler flags, with all repository include paths mapped to the release worktree. Both compiled successfully. `nm -uC` confirmed the external Shape `ToString` reference in the baseline object and its absence in the backport object. - Verified that both resulting source files are identical to the merged main fix, and that the release branch already provides the dependency collection, staging and installation machinery used by this patch. The original main-branch package build and runtime library-loading checks are recorded in #16016. A full release package/image build and NPU numerical/performance tests have not been rerun here. No new unit test is added: this is an unchanged build-dependency backport, checked with the targeted Host object regression above and the existing build pipeline. - vLLM main: vllm-project/vllm@d02df74 Signed-off-by: Foriv <2293567056@qq.com>
Leetrytry
pushed a commit
to Leetrytry/vllm-ascend
that referenced
this pull request
Sep 11, 2026
…project#16065) ### What this PR does / why we need it? Backport of vllm-project#16016 (merged as 701a449) to `releases/v0.26.0rc`. The release branch contains the same QuantLightningIndexerV2 build dependency issues: an unresolved `Ops::Base::ToString(const gert::Shape&)` reference in Host tiling and a missing LightningIndexerV2 sibling header during kernel source staging. This applies the identical two-file patch without conflicts or release-specific changes: - Declare the sibling kernel source dependency for staging and packaging. - Replace 53 external shape-formatting calls with the existing local formatter through a bracket-preserving wrapper. ### Does this PR introduce _any_ user-facing change? Yes: it fixes these QLIV2 dependencies in affected A5 source builds. Operator interfaces, numerical computation and shape diagnostic formatting are unchanged. ### How was this patch tested? On this backport commit, in the A5 `zrr_dev` container: - `bash format.sh ci` passed all 18 hooks; the worktree was clean before and after. - Gitleaks scanned the nonempty release-base-to-HEAD range (one commit), with no leaks. - Compiled the release QLIV2 Host source before and after the patch into separate objects using CANN 9.1 compiler flags, with all repository include paths mapped to the release worktree. Both compiled successfully. `nm -uC` confirmed the external Shape `ToString` reference in the baseline object and its absence in the backport object. - Verified that both resulting source files are identical to the merged main fix, and that the release branch already provides the dependency collection, staging and installation machinery used by this patch. The original main-branch package build and runtime library-loading checks are recorded in vllm-project#16016. A full release package/image build and NPU numerical/performance tests have not been rerun here. No new unit test is added: this is an unchanged build-dependency backport, checked with the targeted Host object regression above and the existing build pipeline. - vLLM main: vllm-project/vllm@d02df74 Signed-off-by: Foriv <2293567056@qq.com>
sunny-rain-63
pushed a commit
to sunny-rain-63/vllm-ascend
that referenced
this pull request
Sep 12, 2026
…llm-project#16016) ### What this PR does / why we need it? A5 custom-op builds can fail when OPC loads the Host tiling library with an unresolved `Ops::Base::ToString(const gert::Shape&)` reference from QuantLightningIndexerV2. The kernel source staging also misses the sibling LightningIndexerV2 header required by QLIV2. This patch: - Reuses the existing local shape formatter through a small wrapper that preserves bracketed diagnostics, replacing all 53 external formatting calls. - Declares the LightningIndexerV2 kernel source dependency so the shared header is copied into the build tree and included in the operator package. The change is limited to the QLIV2 Host CMake file and tiling source. See the [failing A5 build job](https://github.com/vllm-project/vllm-ascend/actions/runs/34178342519/job/101912219798). ### Does this PR introduce _any_ user-facing change? Yes: affected A5 source builds can complete again. Operator interfaces, numerical computation and the shape diagnostic format are unchanged. ### How was this patch tested? Environment: A5 / aarch64, CANN toolkit 9.1.0 with ops-transformer 9.2.0-beta.2, Python 3.11.10, PyTorch 2.10.0 and torch-npu 2.10.0.post4. On this PR commit: - `COMPILE_CUSTOM_KERNELS=1 SOC_VERSION=ascend950dt_9582 MAX_JOBS=32 OPS_CPU_NUMBER=32 python3 setup.py build` passed. This was an incremental build of the standard 30-operator A5 configuration: all 154 kernel artifacts were packaged and installed, and the Torch extension build completed. - The installed sibling header matches its source. The installed tiling library loads in a fresh process with `RTLD_NOW`; ELF inspection confirms no external Shape `ToString` reference or direct `libops_base.so` dependency. - `bash format.sh ci` passed with a clean worktree before and after. A separate Gitleaks scan of the nonempty PR commit range passed (one commit, no leaks). Separate regression diagnostics against the original CI baseline reproduced the original loader failure before the fix, compared seven SDK/local shape-format cases, and compiled all five QLIV2 kernel configurations. The existing custom-op build exercises the loading and source-staging failure, so no new Python unit test is added. NPU numerical/performance tests and the original GitHub image job have not been rerun. - vLLM main: vllm-project/vllm@e6bfe03 Signed-off-by: Foriv <2293567056@qq.com>
like-0517
pushed a commit
to like-0517/vllm-ascend
that referenced
this pull request
Sep 15, 2026
…llm-project#16016) ### What this PR does / why we need it? A5 custom-op builds can fail when OPC loads the Host tiling library with an unresolved `Ops::Base::ToString(const gert::Shape&)` reference from QuantLightningIndexerV2. The kernel source staging also misses the sibling LightningIndexerV2 header required by QLIV2. This patch: - Reuses the existing local shape formatter through a small wrapper that preserves bracketed diagnostics, replacing all 53 external formatting calls. - Declares the LightningIndexerV2 kernel source dependency so the shared header is copied into the build tree and included in the operator package. The change is limited to the QLIV2 Host CMake file and tiling source. See the [failing A5 build job](https://github.com/vllm-project/vllm-ascend/actions/runs/34178342519/job/101912219798). ### Does this PR introduce _any_ user-facing change? Yes: affected A5 source builds can complete again. Operator interfaces, numerical computation and the shape diagnostic format are unchanged. ### How was this patch tested? Environment: A5 / aarch64, CANN toolkit 9.1.0 with ops-transformer 9.2.0-beta.2, Python 3.11.10, PyTorch 2.10.0 and torch-npu 2.10.0.post4. On this PR commit: - `COMPILE_CUSTOM_KERNELS=1 SOC_VERSION=ascend950dt_9582 MAX_JOBS=32 OPS_CPU_NUMBER=32 python3 setup.py build` passed. This was an incremental build of the standard 30-operator A5 configuration: all 154 kernel artifacts were packaged and installed, and the Torch extension build completed. - The installed sibling header matches its source. The installed tiling library loads in a fresh process with `RTLD_NOW`; ELF inspection confirms no external Shape `ToString` reference or direct `libops_base.so` dependency. - `bash format.sh ci` passed with a clean worktree before and after. A separate Gitleaks scan of the nonempty PR commit range passed (one commit, no leaks). Separate regression diagnostics against the original CI baseline reproduced the original loader failure before the fix, compared seven SDK/local shape-format cases, and compiled all five QLIV2 kernel configurations. The existing custom-op build exercises the loading and source-staging failure, so no new Python unit test is added. NPU numerical/performance tests and the original GitHub image job have not been rerun. - vLLM main: vllm-project/vllm@e6bfe03 Signed-off-by: Foriv <2293567056@qq.com> Signed-off-by: like-0517 <ithwlike@126.com>
cursor Bot
pushed a commit
to yjyang62/vllm-ascend
that referenced
this pull request
Sep 15, 2026
vllm-project#16422 reintroduced SDK Shape ToString in QLIV2 and SparseFlashMLA tiling error logs. CANN 9.1.0 does not export that symbol, so A5 OPC fails to load liboptiling.so. Restore the vllm-project#16016 local formatter; tiling checks and kernel math are unchanged. Signed-off-by: yjyang62 <yjyang62@users.noreply.github.com> Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: yjyang62 <yjyang62@users.noreply.github.com>
ZT-AIA
pushed a commit
that referenced
this pull request
Sep 15, 2026
### What this PR does / why we need it? Fix two A5 QLIV2 build regressions introduced by #16422: - Restore the local shape formatter from #16016. The [A5 Ubuntu amd64 image build](https://github.com/vllm-project/vllm-ascend/actions/runs/34829507531/job/103929405987) cannot load `liboptiling.so` because the 53 restored `Ops::Base::ToString(const gert::Shape&)` calls leave `_ZN3Ops4Base8ToStringERKN4gert5ShapeE` unresolved. Use the bracket-preserving `ShapeToStringForLog` wrapper around the existing `ToStringRaw`. - Match the A5 dependency with `"ascend950" IN_LIST ASCEND_COMPUTE_UNIT`. The standard build passes `ascend950;`; the string-equality check skips the sibling dependency and causes `lightning_indexer_v2_vector1_base.h` to be missing from the staged kernel sources. List membership also handles multi-target configurations while keeping the dependency A5-specific. ### Does this PR introduce _any_ user-facing change? Yes: affected A5 source builds can resolve these QLIV2 Host and kernel-source dependencies. Operator interfaces, numerical computation, and diagnostic formatting are unchanged. ### How was this patch tested? Rebased on main `fbb75a47436901616849da155828f755ec58b619` (including the CPU UT fix #16574). Current HEAD is `5793e3135057281e08f36ab66f807d2c229843f3`; `git range-diff` confirms both patches are unchanged. All 18 native lint hooks passed on this HEAD. A new [CI run](https://github.com/vllm-project/vllm-ascend/actions/runs/34960944860) was triggered. The full build evidence below belongs to the pre-rebase HEAD; the full build was not repeated after rebase. In the A5-107 aarch64 development container with CANN 9.1.0: - Compiled the complete QLIV2 Host translation unit before and after the formatter fix into separate objects. `nm -u` confirms the exact external Shape formatter reference in the baseline object and its absence after the fix. - Compared the actual local formatting helpers against the SDK formatter: all seven shape cases match. - Built the shared Host tiling library with the formatter fix and loaded it in a fresh process using `RTLD_NOW`; the library has neither the unresolved Shape formatter nor a direct `libops_base.so` dependency. - Reproduced the missing sibling header with an actual QLIV2 kernel compilation. After the CMake fix, standard reconfiguration records the dependency and stages the missing header; all five generated QLIV2 configurations (0–4) compile successfully in isolated output directories and produce their final object and JSON artifacts. - On pre-rebase HEAD `5837fe671ec6ef180e48a350e9ecc33d71547b5f`, `bash format.sh ci` passed all 18 hooks with a clean worktree; Gitleaks passed for the two-commit baseline-to-HEAD range. - On that pre-rebase HEAD, the standard `python3 setup.py build` completed with exit 0 (`SOC_VERSION=ascend950dt_9582`, `MAX_JOBS=8`, `OPS_CPU_NUMBER=8`, `VLLM_BATCH_INVARIANT=0`). All 212 A5 kernel targets completed, including all five QLIV2 targets; the OPP installer and Python C++ extension were generated successfully. - Verified the packaged output: 212 kernel objects, the sibling header byte-identical to its source, and `liboptiling.so` loading successfully in a fresh `RTLD_NOW` process without `LD_PRELOAD` or `ASCEND_CUSTOM_OPP_PATH`. The installed library has no unresolved Shape formatter reference or direct `libops_base.so` dependency. The final build reused completed targets from the initial formatter-only attempt and the compiler cache; it is not an all-cache-disabled rebuild. This is a native aarch64 container build, not a rerun of the amd64 Docker image CI. NPU numerical/performance tests were not run. No new Python unit test is added: the native compilation, symbol, library-loading, and package checks directly exercise these build failures. - vLLM main: vllm-project/vllm@84030bb --------- Signed-off-by: Foriv <2293567056@qq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it?
A5 custom-op builds can fail when OPC loads the Host tiling library with an unresolved
Ops::Base::ToString(const gert::Shape&)reference from QuantLightningIndexerV2. The kernel source staging also misses the sibling LightningIndexerV2 header required by QLIV2.This patch:
The change is limited to the QLIV2 Host CMake file and tiling source. See the failing A5 build job.
Does this PR introduce any user-facing change?
Yes: affected A5 source builds can complete again. Operator interfaces, numerical computation and the shape diagnostic format are unchanged.
How was this patch tested?
Environment: A5 / aarch64, CANN toolkit 9.1.0 with ops-transformer 9.2.0-beta.2, Python 3.11.10, PyTorch 2.10.0 and torch-npu 2.10.0.post4.
On this PR commit:
COMPILE_CUSTOM_KERNELS=1 SOC_VERSION=ascend950dt_9582 MAX_JOBS=32 OPS_CPU_NUMBER=32 python3 setup.py buildpassed. This was an incremental build of the standard 30-operator A5 configuration: all 154 kernel artifacts were packaged and installed, and the Torch extension build completed.RTLD_NOW; ELF inspection confirms no external ShapeToStringreference or directlibops_base.sodependency.bash format.sh cipassed with a clean worktree before and after. A separate Gitleaks scan of the nonempty PR commit range passed (one commit, no leaks).Separate regression diagnostics against the original CI baseline reproduced the original loader failure before the fix, compared seven SDK/local shape-format cases, and compiled all five QLIV2 kernel configurations. The existing custom-op build exercises the loading and source-staging failure, so no new Python unit test is added. NPU numerical/performance tests and the original GitHub image job have not been rerun.