Fix undefined symbol: cutlass_moe_mm_sm100#26098
Fix undefined symbol: cutlass_moe_mm_sm100#26098ProExpertProg merged 11 commits intovllm-project:mainfrom
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run You ask your reviewers to trigger select CI tests on top of Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. 🚀 |
60ec9a8 to
a4fc6fe
Compare
|
cc @johnnynunez |
There was a problem hiding this comment.
Code Review
This pull request aims to fix a build issue for new hardware by enabling compilation of cutlass_moe_mm_sm100 for newer GPU architectures. While the change to CMakeLists.txt correctly adds the new architectures to the build, a related change in csrc/quantization/cutlass_w8a8/scaled_mm_entry.cu introduces a bug in the dispatch logic. It prevents the newly compiled kernel from being used on sm_120 and newer architectures, which would lead to a runtime error. My review identifies this critical contradiction and suggests reverting the problematic condition in the C++ code to align with the build configuration and ensure the fix works as intended.
a4fc6fe to
5fe6fa1
Compare
|
Is this not fixed by #26077? |
i didn't saw that PR, thank you...! Checking |
|
OK, so I will test it later, and make a pure patch for the memory issue |
Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com> Signed-off-by: Jun Jiang <jasl9187@hotmail.com>
I have tested it, and that's not working for my case |
|
@jasl that fixes the issue for pre-10.0 platforms that don't support the function and adds a dummy implementation that just throws an error. You probably still need some of your fixes for the function to actually run on 10.0+ platforms. Or am I missing something? |
I don't know about how to write kernels. In my analysis, the root cause is somewhere leaking SM100 flag to SM110, yet SM110 shares many features with SM100. So I choose a simplest way to workaround it. compile the file, but restrict the code path |
|
@jasl let's wait for the other PR to land and you can rebase on top and see what you still need to resolve your issue. |
I guess the trouble is the 80 words per line rule. Could you force it to pass? Or should I break the URL into two lines? |
Run: pre-commir run -a |
d8db5ba to
07bb437
Compare
07bb437 to
9dd6445
Compare
|
@ProExpertProg Could you review again? |
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com>
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com> Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com> Signed-off-by: Tomer Asida <57313761+tomeras91@users.noreply.github.com>
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com> Signed-off-by: Karan Goel <3261985+karan@users.noreply.github.com>
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com>
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com>
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com>
Signed-off-by: Jun Jiang <jasl9187@hotmail.com> Co-authored-by: Luka Govedič <ProExpertProg@users.noreply.github.com>
Purpose
The fix is a little bit hacky, but it can make vLLM run on Thor
In addition, as @johnnynunez suggests, I add an essential patch to calculate free memory correctly for Nvidia UMA hardwares
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.