-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[llvm-libgcc][CMake] Refactor llvm-libgcc #65455
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
eb938ef
[llvm-libgcc][CMake] Fix broken libgcc_s.so
ur4t 1b8a9ba
[llvm-libgcc][CMake] Refactor llvm-libgcc CMakeLists
ur4t c790fd0
[llvm-libgcc][CMake] Fix mismatched target names of llvm-libgcc
ur4t 358ff00
[llvm-libgcc][CMake] Refuse improper COMPILER_RT_BUILD_BUILTINS
ur4t 6f7088b
[llvm-libgcc][CMake] Fix broken component llvm-libgcc
ur4t b61aed7
[llvm-libgcc][CMake] Restore to inhibit co-existence of llvm-libgcc a…
ur4t 656d900
[llvm-libgcc][CMake] Remove uncessary dependency on compiler-rt or li…
ur4t 60eccd0
[llvm-libgcc][CMake] Improve standalone build for llvm-libgcc
ur4t File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
File renamed without changes.
This file was deleted.
Oops, something went wrong.
Empty file.
This file contains hidden or 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
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.
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.
Why is this being removed?
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.
libunwind/CMakeLists.txt
does not contains aproject(libunwind LANGUAGES C CXX ASM)
. To keep consistent withlibunwind/CMakeLists.txt
,project(llvm-libgcc LANGUAGES C CXX ASM)
is not added.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.
compiler-rt does have it, and I'd prefer to keep this regardless.
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.
Now
llvm-libgcc
use the same approach ascompiler-rt
in 60eccd0.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'm not following what you mean, sorry. Since this is the driver for compiler-rt and libunwind, it makes sense to me for this project to indicate this information.
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.
compiler-rt/CMakeLists.txt
detects whethercompiler-rt
is built as a subproject ofruntimes
or a standalone project. In standalone mode it sets options to detectllvm
and declares the project name withproject(CompilerRT C CXX ASM)
.Now
llvm-libgcc/CMakeLists.txt
does the same: in standalone mode, it tells addedcompiler-rt
that we are not built as a subproject ofruntimes
and we have to detectllvm
ourselves.Here "standalone" means
cmake -B build -S compiler-rt
orcmake -B build -S llvm-libgcc
, notcmake -B build -S runtimes
norcmake -B build -S llvm
. Even in the wholellvm-project
source tree, directly usingcompiler-rt/CMakeLists.txt
orllvm-libgcc/CMakeLists.txt
is "standalone".If included by other
CMakeLists.txt
,runtimes/CMakeLists.txt
for example, it does not matter whether there is aproject(CompilerRT C CXX ASM)
orproject(llvm-libgcc LANGUAGES C CXX ASM)
declaration actually performed. To keep consistent withcompiler-rt
,llvm-libgcc/CMakeLists.txt
is adjusted correspondingly in 60eccd0.Correspondingly lines from
compiler-rt/CMakeLists.txt
:Correspondingly lines in
llvm-libgcc/CMakeLists.txt
:And I am a bit confused how should we indicate "this is the driver for
compiler-rt
andlibunwind
". It is completely acceptable to performproject(llvm-libgcc LANGUAGES C CXX ASM)
declaration unconditionally, but does it makes sense in this scenario?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.
Understood, thanks for highlighting the change to llvm-libgcc/CMakeLists.txt, which I'd missed.