Skip to content

[Compile] Fix import error of vllm._C#26077

Closed
ZJY0516 wants to merge 6 commits intovllm-project:mainfrom
ZJY0516:fix-cmake
Closed

[Compile] Fix import error of vllm._C#26077
ZJY0516 wants to merge 6 commits intovllm-project:mainfrom
ZJY0516:fix-cmake

Conversation

@ZJY0516
Copy link
Member

@ZJY0516 ZJY0516 commented Oct 2, 2025

Purpose

PR #24673 removes nvfp4_blockwise_moe_kernel.cu from VLLM_EXT_SRC, which will cause an import error when using vllm built from source.

  File "/home/zjy/code/vllm-src/vllm/platforms/cuda.py", line 18, in <module>
    import vllm._C  # noqa
    ^^^^^^^^^^^^^^
ImportError: /home/zjy/code/vllm-src/vllm/_C.abi3.so: undefined symbol: _Z20cutlass_fp4_group_mmRN2at6TensorERKS0_S3_S3_S3_S3_S3_S3_S3_

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly fixes a build issue that was causing an ImportError due to an undefined symbol. The error was introduced in a previous pull request that removed nvfp4_blockwise_moe_kernel.cu from the list of compiled sources. This change re-adds the file to CMakeLists.txt, which resolves the issue. The fix is straightforward and effective. I have reviewed the change and found no issues.

Copy link
Collaborator

@ProExpertProg ProExpertProg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should instead just guard its use in torch_bindings.cpp on the arch supporting fp4 (and the cudaversion)

"csrc/quantization/fp4/activation_nvfp4_quant_fusion_kernels.cu"
"csrc/quantization/fp4/nvfp4_experts_quant.cu"
"csrc/quantization/fp4/nvfp4_scaled_mm_kernels.cu"
"csrc/quantization/fp4/nvfp4_blockwise_moe_kernel.cu")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's added to sources here

@ZJY0516
Copy link
Member Author

ZJY0516 commented Oct 2, 2025

We should instead just guard its use in torch_bindings.cpp on the arch supporting fp4 (and the cudaversion)

I didn't seen similar pattern like this in torch_bindings.cpp so I directly changed CMakeLists.txt.

Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
@ZJY0516 ZJY0516 requested a review from ProExpertProg October 2, 2025 13:49
@ZJY0516
Copy link
Member Author

ZJY0516 commented Oct 2, 2025

Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
@ZJY0516 ZJY0516 requested a review from LucasWilkinson October 2, 2025 15:48
@LucasWilkinson LucasWilkinson added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 2, 2025
Copy link
Collaborator

@LucasWilkinson LucasWilkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Collaborator

@ProExpertProg ProExpertProg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reworking this

@ProExpertProg ProExpertProg disabled auto-merge October 3, 2025 01:40
@ProExpertProg
Copy link
Collaborator

#26138 does it in a cleaner way actually

@DarkLight1337
Copy link
Member

Closing as superseded by #26138

@ZJY0516 ZJY0516 deleted the fix-cmake branch October 3, 2025 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/build ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants