CMake: Do not specify LLVMSupport in LINK_LIBS.
#2575
Merged
Conversation
This file contains 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
Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com>
|
I think I added LLVMSupport for bazel build dep parity as opposed to actually needing to specify it, so yes looks like we don't need it. LGTM! |
GleasonK
approved these changes
Oct 3, 2024
GleasonK
reviewed
Oct 3, 2024
Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com>
|
@GleasonK , please merge :-) |
bjacob
added a commit
to iree-org/iree
that referenced
this pull request
Oct 3, 2024
No cherry-picks anymore, our PRs are in: openxla/stablehlo#2576 openxla/stablehlo#2575 openxla/stablehlo#2572 Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com>
GleasonK
pushed a commit
that referenced
this pull request
Oct 16, 2024
This fixes downstream build errors tracked at iree-org/iree#18785. Sample error message when building on Windows with MSVC: ``` FAILED: llvm-external-projects/stablehlo/stablehlo/dialect/CMakeFiles/obj.VhloOps.dir/VhloOps.cpp.obj C:\ProgramData\Chocolatey\bin\ccache C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1441~1.341\bin\Hostx64\x64\cl.exe /nologo /TP -DGTEST_HAS_RTTI=0 -D_HAS_EXCEPTIONS=0 -D_USE_MATH_DEFINES -ID:\a\iree\iree\third_party\llvm-project\llvm\include -ID:\a\iree\iree\build-windows\llvm-project\include -ID:\a\iree\iree\third_party\llvm-project\mlir\include -ID:\a\iree\iree\third_party\stablehlo -ID:\a\iree\iree\build-windows\llvm-external-projects\stablehlo -ID:\a\iree\iree\third_party\llvm-project\lld\include -ID:\a\iree\iree\build-windows\llvm-project\tools\lld\include -external:I\..\mlir\include -external:ID:\a\iree\iree\build-windows\llvm-project\tools\mlir\include -external:W0 /DWIN32 /D_WINDOWS /EHsc /Z7 /O2 /Ob1 -std:c++17 -MD /EHs-c- /GR- /showIncludes /Follvm-external-projects\stablehlo\stablehlo\dialect\CMakeFiles\obj.VhloOps.dir\VhloOps.cpp.obj /Fdllvm-external-projects\stablehlo\stablehlo\dialect\CMakeFiles\obj.VhloOps.dir\ /FS -c D:\a\iree\iree\third_party\stablehlo\stablehlo\dialect\VhloOps.cpp cl : Command line warning D9025 : overriding '/EHs' with '/EHs-' cl : Command line warning D9025 : overriding '/EHc' with '/EHc-' D:\a\iree\iree\third_party\stablehlo\stablehlo\dialect\VhloOps.cpp : fatal error C1128: number of sections exceeded object file format limit: compile with /bigobj ``` I think our downstream Windows builds worked with #2573 but they do not work with #2575, without this change. Hooray for compiler options whack-a-mole.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a new take on #2573.
There, I was trying to fix CMake errors in the external build, with the error message saying that
LINK_COMPONENTSshould be used instead ofLINK_LIBSfor LLVM libraries. That change fixed my external build, but broke the standalone build on CI.Reading the
AddMLIR.cmakecode generating the error, I came across this line:https://github.com/llvm/llvm-project/blob/ee4dd147baff8f971f3ec5aad5a216ca9837a732/mlir/cmake/modules/AddMLIR.cmake#L287-L288
This looks like hardcoding always depending on
LLVMSupport(though I wasn't quite sure about the nuance betweenSupportandLLVMSupport). If that's correct, then we never needed specifyingLLVMSupportin the first place. So I tried just omitting it, and that seems to work. WDYT?