Conversation
|
👋 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. |
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 performs an experimental revert of a previous change to the arch22 QLI v2 kernel. The goal is to isolate a performance regression observed in the DeepSeek-V4-Pro nightly benchmarks. This change is strictly for diagnostic purposes to verify if the previous update to the stride calculation is the source of the performance degradation. 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][Misc] Revert kScaleBlkStride to int32_t and remove high physical page testSuggested PR Summary:
### What this PR does / why we need it?
This pull request reverts `kScaleBlkStride` from `int64_t` to `int32_t` in `quant_lightning_indexer_v2_service_vector_arch22.h` and removes the regression test `test_quant_lightning_indexer_v2_high_physical_page_scale_offset` from `test_quant_lightning_indexer_v2.py`.
However, reverting `kScaleBlkStride` to `int32_t` reintroduces a critical signed 32-bit integer overflow vulnerability. When `blockId` is multiplied by `kScaleBlkStride`, the product can exceed `2^31 - 1` for large page IDs, leading to undefined behavior and incorrect memory indexing on `kScaleGm`. This change should not be merged.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
The regression test for high physical page scale offset was removed, and no new tests were added.| int32_t kScaleBlkStride = constInfo_.keyDequantScaleStride0 != 0 ? | ||
| static_cast<int32_t>(constInfo_.keyDequantScaleStride0) : kCacheBlockSize_; |
There was a problem hiding this comment.
Reverting kScaleBlkStride to int32_t reintroduces a critical signed 32-bit integer overflow vulnerability. When blockId is multiplied by kScaleBlkStride, the product can exceed 2^31 - 1 for large page IDs, leading to undefined behavior and incorrect memory indexing on kScaleGm. This change must not be merged into production.
int64_t kScaleBlkStride = constInfo_.keyDequantScaleStride0 != 0 ? static_cast<int64_t>(constInfo_.keyDequantScaleStride0) : kCacheBlockSize_;Experimental revert to isolate DeepSeek-V4-Pro-w4a8-prefix-cache-PD Nightly performance regression. Do not merge before CI comparison. Signed-off-by: pgzddxx <1697817735@qq.com>
f3cb349 to
14aee41
Compare
|
/nightly DeepSeek-V4-Pro-w4a8-prefix-cache-PD
|
Purpose
Experimental full revert of #16824 (merge commit e139b7d) on current upstream main d76a8c6. This PR is for isolating the DeepSeek-V4-Pro-w4a8-prefix-cache-PD Nightly performance regression. It is not a confirmed fix; please do not merge based on this PR alone.
Test request
Please run the named Nightly case on this branch and compare it with the same case on the pinned main baseline. Keep model weights, workload, hardware, image build settings, and benchmark parameters identical. Record the full AISBench table, Output TPS, Prefill TPS, TTFT, TPOT, and service logs. This PR must be evaluated independently of the #15514 revert PR; do not stack the two reverts.
Scope and risk
This reverts the arch22 QLI v2 K-scale block stride from int64_t to int32_t and removes the matching high-physical-page regression test. The original #16824 fixes signed-32-bit offset overflow for large page IDs. Reverting it can reintroduce incorrect results for that case, so the PR is strictly diagnostic and should not be merged as a production fix without addressing correctness. The target PD Nightly YAML is unchanged by this revert.
Validation
Reverted test file: Python AST parse passed.
Git diff whitespace check passed.
No NPU service or benchmark has been run locally. Correctness and performance remain unverified; CI is the intended experiment.
vLLM main: vllm-project/vllm@84030bb