Use LINK_COMPONENTS to link LLVMSupport
#2573
Closed
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>
sdasgup3
approved these changes
Oct 2, 2024
|
This is actually failing to link in the CI. I don't have that problem in my dependent project (IREE) where Stablehlo is external. I don't know what the proper solution would look like. For now, I can just cherry-pick this. |
bjacob
added a commit
to iree-org/iree
that referenced
this pull request
Oct 2, 2024
Continuing from @hanhanW 's #18659: Stablehlo cherry-picks: 1. openxla/stablehlo#2572 2. openxla/stablehlo#2573 Torch-mlir cherry-picks: 1. llvm/torch-mlir#3755 --------- Signed-off-by: hanhanW <hanhan0912@gmail.com> Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com> Co-authored-by: hanhanW <hanhan0912@gmail.com>
|
Trying #2575 as a simpler alternative to this, if that works. |
|
Superseded by #2575. |
GleasonK
pushed a commit
that referenced
this pull request
Oct 3, 2024
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_COMPONENTS` should be used instead of `LINK_LIBS` for LLVM libraries. That change fixed my external build, but broke the standalone build on CI. Reading the `AddMLIR.cmake` code generating the error, I came across this line: https://github.com/llvm/llvm-project/blob/ee4dd147baff8f971f3ec5aad5a216ca9837a732/mlir/cmake/modules/AddMLIR.cmake#L287-L288 ```cmake # MLIR libraries uniformly depend on LLVMSupport. Just specify it once here. list(APPEND ARG_LINK_COMPONENTS Support) ``` This looks like hardcoding always depending on `LLVMSupport` (though I wasn't quite sure about the nuance between `Support` and `LLVMSupport`). If that's correct, then we never needed specifying `LLVMSupport` in the first place. So I tried just omitting it, and that seems to work. WDYT? --------- 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.
In #2494,
LLVMSupportwas added as a linked dependency to a few targets such asChloCAPI, using theadd_mlir_public_c_api_libraryfunction, passingLLVMSupportalong with other dependencies underLINK_LIBS.Trying to integrate these changes in https://github.com/iree-org/iree , I get these CMake errors:
This PR is simply following the suggesting in that error message, and that seems to work.