-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Build error seen on Power Architecture #10421
Conversation
👋 Hi! Thank you for contributing to the vLLM project. 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 do one of these:
🚀 |
cc: @khluu |
Please fix clang-format |
cmake/cpu_extension.cmake
Outdated
if (CMAKE_SYSTEM_PROCESSOR STREQUAL "ppc64le") | ||
list(APPEND CXX_COMPILE_FLAGS | ||
"-fopenmp" | ||
"-DVLLM_CPU_EXTENSION") | ||
else() | ||
list(APPEND CXX_COMPILE_FLAGS | ||
"-fopenmp" | ||
"-mf16c" | ||
"-DVLLM_CPU_EXTENSION") | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you could invert this to always apply
list(APPEND CXX_COMPILE_FLAGS
"-fopenmp"
"-DVLLM_CPU_EXTENSION")
And then add a case just if the processor is x86 to add "-mf16c"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar to this block -
vllm/cmake/cpu_extension.cmake
Line 57 in 284203f
if (AVX512_FOUND AND NOT AVX512_DISABLED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above flag was applied for all architectures. We know for Power its not applicable so added exclusive check to avoid it for Power.
Adding exclusive check for x86 may avoid it applying for other supported architectures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x86 is the only architecture that supports this flag, so I don't see how it would hurt
Please see this incoming PR for ARM support for an illustration of what I'm asking for here
https://github.com/vllm-project/vllm/pull/9957/files#diff-46b07f808e558aff84dcc365e53a91400c4be399757ff40c2da4278a50e89fbdL19-R24
Please merge from main to fix the failing tests. |
4d0d6cd
to
52e3d52
Compare
Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: B-201 <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
…0414) Signed-off-by: Isotr0py <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: B-201 <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
…#10406) Signed-off-by: youkaichao <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: ismael-dm <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: Andrew Nesbitt <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: mgoin <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: yan ma <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
…t#9014) Signed-off-by: Angus Wang <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: Lucas Wilkinson <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: youkaichao <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: youkaichao <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
…ct#9929) Signed-off-by: rickyx <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
We have too many dependencies and all patch updates can be a little noisy. This is to have dependabot ignore all patch version updates. Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: Jee Jee Li <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
…form` (vllm-project#10358) Signed-off-by: Mengqing Cao <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
…rst (vllm-project#10433) Signed-off-by: Manjul Mohan <[email protected]>
…oject#10430) Signed-off-by: Travis Johnson <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: Jee Jee Li <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: youkaichao <[email protected]> Signed-off-by: Manjul Mohan <[email protected]>
Signed-off-by: Manjul Mohan <[email protected]>
a07e591
to
cd96cde
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can land for now and fix the CPU cmake in the ARM support PRs
Signed-off-by: Manjul Mohan <[email protected]> Signed-off-by: B-201 <[email protected]> Signed-off-by: Isotr0py <[email protected]> Signed-off-by: youkaichao <[email protected]> Signed-off-by: ismael-dm <[email protected]> Signed-off-by: Andrew Nesbitt <[email protected]> Signed-off-by: mgoin <[email protected]> Signed-off-by: yan ma <[email protected]> Signed-off-by: Angus Wang <[email protected]> Signed-off-by: Lucas Wilkinson <[email protected]> Signed-off-by: rickyx <[email protected]> Signed-off-by: Jee Jee Li <[email protected]> Signed-off-by: Mengqing Cao <[email protected]> Signed-off-by: Travis Johnson <[email protected]> Co-authored-by: Manjul Mohan [email protected] <[email protected]> Co-authored-by: B-201 <[email protected]> Co-authored-by: Isotr0py <[email protected]> Co-authored-by: youkaichao <[email protected]> Co-authored-by: ismael-dm <[email protected]> Co-authored-by: Andrew Nesbitt <[email protected]> Co-authored-by: Michael Goin <[email protected]> Co-authored-by: Yan Ma <[email protected]> Co-authored-by: Angus Wang <[email protected]> Co-authored-by: Lucas Wilkinson <[email protected]> Co-authored-by: Ricky Xu <[email protected]> Co-authored-by: Kevin H. Luu <[email protected]> Co-authored-by: Jee Jee Li <[email protected]> Co-authored-by: Mengqing Cao <[email protected]> Co-authored-by: Travis Johnson <[email protected]> Co-authored-by: Russell Bryant <[email protected]>
Signed-off-by: Manjul Mohan <[email protected]> Signed-off-by: B-201 <[email protected]> Signed-off-by: Isotr0py <[email protected]> Signed-off-by: youkaichao <[email protected]> Signed-off-by: ismael-dm <[email protected]> Signed-off-by: Andrew Nesbitt <[email protected]> Signed-off-by: mgoin <[email protected]> Signed-off-by: yan ma <[email protected]> Signed-off-by: Angus Wang <[email protected]> Signed-off-by: Lucas Wilkinson <[email protected]> Signed-off-by: rickyx <[email protected]> Signed-off-by: Jee Jee Li <[email protected]> Signed-off-by: Mengqing Cao <[email protected]> Signed-off-by: Travis Johnson <[email protected]> Co-authored-by: Manjul Mohan [email protected] <[email protected]> Co-authored-by: B-201 <[email protected]> Co-authored-by: Isotr0py <[email protected]> Co-authored-by: youkaichao <[email protected]> Co-authored-by: ismael-dm <[email protected]> Co-authored-by: Andrew Nesbitt <[email protected]> Co-authored-by: Michael Goin <[email protected]> Co-authored-by: Yan Ma <[email protected]> Co-authored-by: Angus Wang <[email protected]> Co-authored-by: Lucas Wilkinson <[email protected]> Co-authored-by: Ricky Xu <[email protected]> Co-authored-by: Kevin H. Luu <[email protected]> Co-authored-by: Jee Jee Li <[email protected]> Co-authored-by: Mengqing Cao <[email protected]> Co-authored-by: Travis Johnson <[email protected]> Co-authored-by: Russell Bryant <[email protected]>
Signed-off-by: Manjul Mohan <[email protected]> Signed-off-by: B-201 <[email protected]> Signed-off-by: Isotr0py <[email protected]> Signed-off-by: youkaichao <[email protected]> Signed-off-by: ismael-dm <[email protected]> Signed-off-by: Andrew Nesbitt <[email protected]> Signed-off-by: mgoin <[email protected]> Signed-off-by: yan ma <[email protected]> Signed-off-by: Angus Wang <[email protected]> Signed-off-by: Lucas Wilkinson <[email protected]> Signed-off-by: rickyx <[email protected]> Signed-off-by: Jee Jee Li <[email protected]> Signed-off-by: Mengqing Cao <[email protected]> Signed-off-by: Travis Johnson <[email protected]> Co-authored-by: Manjul Mohan [email protected] <[email protected]> Co-authored-by: B-201 <[email protected]> Co-authored-by: Isotr0py <[email protected]> Co-authored-by: youkaichao <[email protected]> Co-authored-by: ismael-dm <[email protected]> Co-authored-by: Andrew Nesbitt <[email protected]> Co-authored-by: Michael Goin <[email protected]> Co-authored-by: Yan Ma <[email protected]> Co-authored-by: Angus Wang <[email protected]> Co-authored-by: Lucas Wilkinson <[email protected]> Co-authored-by: Ricky Xu <[email protected]> Co-authored-by: Kevin H. Luu <[email protected]> Co-authored-by: Jee Jee Li <[email protected]> Co-authored-by: Mengqing Cao <[email protected]> Co-authored-by: Travis Johnson <[email protected]> Co-authored-by: Russell Bryant <[email protected]> Signed-off-by: rickyx <[email protected]>
Signed-off-by: Manjul Mohan <[email protected]> Signed-off-by: B-201 <[email protected]> Signed-off-by: Isotr0py <[email protected]> Signed-off-by: youkaichao <[email protected]> Signed-off-by: ismael-dm <[email protected]> Signed-off-by: Andrew Nesbitt <[email protected]> Signed-off-by: mgoin <[email protected]> Signed-off-by: yan ma <[email protected]> Signed-off-by: Angus Wang <[email protected]> Signed-off-by: Lucas Wilkinson <[email protected]> Signed-off-by: rickyx <[email protected]> Signed-off-by: Jee Jee Li <[email protected]> Signed-off-by: Mengqing Cao <[email protected]> Signed-off-by: Travis Johnson <[email protected]> Co-authored-by: Manjul Mohan [email protected] <[email protected]> Co-authored-by: B-201 <[email protected]> Co-authored-by: Isotr0py <[email protected]> Co-authored-by: youkaichao <[email protected]> Co-authored-by: ismael-dm <[email protected]> Co-authored-by: Andrew Nesbitt <[email protected]> Co-authored-by: Michael Goin <[email protected]> Co-authored-by: Yan Ma <[email protected]> Co-authored-by: Angus Wang <[email protected]> Co-authored-by: Lucas Wilkinson <[email protected]> Co-authored-by: Ricky Xu <[email protected]> Co-authored-by: Kevin H. Luu <[email protected]> Co-authored-by: Jee Jee Li <[email protected]> Co-authored-by: Mengqing Cao <[email protected]> Co-authored-by: Travis Johnson <[email protected]> Co-authored-by: Russell Bryant <[email protected]> Signed-off-by: Tyler Michael Smith <[email protected]>
Signed-off-by: Manjul Mohan <[email protected]> Signed-off-by: B-201 <[email protected]> Signed-off-by: Isotr0py <[email protected]> Signed-off-by: youkaichao <[email protected]> Signed-off-by: ismael-dm <[email protected]> Signed-off-by: Andrew Nesbitt <[email protected]> Signed-off-by: mgoin <[email protected]> Signed-off-by: yan ma <[email protected]> Signed-off-by: Angus Wang <[email protected]> Signed-off-by: Lucas Wilkinson <[email protected]> Signed-off-by: rickyx <[email protected]> Signed-off-by: Jee Jee Li <[email protected]> Signed-off-by: Mengqing Cao <[email protected]> Signed-off-by: Travis Johnson <[email protected]> Co-authored-by: Manjul Mohan [email protected] <[email protected]> Co-authored-by: B-201 <[email protected]> Co-authored-by: Isotr0py <[email protected]> Co-authored-by: youkaichao <[email protected]> Co-authored-by: ismael-dm <[email protected]> Co-authored-by: Andrew Nesbitt <[email protected]> Co-authored-by: Michael Goin <[email protected]> Co-authored-by: Yan Ma <[email protected]> Co-authored-by: Angus Wang <[email protected]> Co-authored-by: Lucas Wilkinson <[email protected]> Co-authored-by: Ricky Xu <[email protected]> Co-authored-by: Kevin H. Luu <[email protected]> Co-authored-by: Jee Jee Li <[email protected]> Co-authored-by: Mengqing Cao <[email protected]> Co-authored-by: Travis Johnson <[email protected]> Co-authored-by: Russell Bryant <[email protected]>
The issue in the vLLM library involves the introduction of the -mf16c flag and FP16 vector types, which are specific to x86 architecture. These changes cause build failures on Power architecture, though the library builds and functions correctly on x86. To resolve this, architecture-specific checks will be implemented. The -mf16c flag will not be applied on the Power architecture systems, while FP32Vec types will be used as a fallback for unsupported FP16 vector types on Power architecture. This fix ensures compatibility and successful builds across different architectures.