This revision was automatically updated to reflect the committed changes.
Closed by commit rL289668: [Driver] Allow setting the default linker during
build (authored by phosek).
Changed prior to commit:
https://reviews.llvm.org/D25263?vs=81276&id=81394#toc
Repository:
rL LLVM
https://review
phosek updated this revision to Diff 81276.
phosek marked 3 inline comments as done.
Repository:
rL LLVM
https://reviews.llvm.org/D25263
Files:
CMakeLists.txt
include/clang/Config/config.h.cmake
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains.cpp
lib
Hahnfeld accepted this revision.
Hahnfeld added a comment.
This revision is now accepted and ready to land.
LGTM with one nit
Comment at: lib/Driver/ToolChain.cpp:362
+ return UseLinker;
+ } else if (UseLinker == "ld") {
+// If we're passed the argument ld, then use w
phosek added inline comments.
Comment at: lib/Driver/ToolChain.cpp:362
+ return UseLinker;
+ } else if (UseLinker == "ld") {
+// If we're passed the argument ld, then use whatever the default system
I'm wandering whether we shouldn't use `"platform"` in
phosek updated this revision to Diff 80934.
phosek marked 4 inline comments as done.
Repository:
rL LLVM
https://reviews.llvm.org/D25263
Files:
CMakeLists.txt
include/clang/Config/config.h.cmake
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains.cpp
lib
Hahnfeld added inline comments.
Comment at: lib/Driver/ToolChain.cpp:721-724
+
+const char *ToolChain::getDefaultLinker() const {
+ return CLANG_DEFAULT_LINKER;
+}
sfertile wrote:
> Hahnfeld wrote:
> > I think this could go into the header
> The CLANG_DEFAULT_LI
sfertile added inline comments.
Comment at: lib/Driver/ToolChain.cpp:721-724
+
+const char *ToolChain::getDefaultLinker() const {
+ return CLANG_DEFAULT_LINKER;
+}
Hahnfeld wrote:
> I think this could go into the header
The CLANG_DEFAULT_LINKER macro is getting
Hahnfeld added a comment.
I have two high level remarks here:
1. `CLANG_DEFAULT_LINKER` should override whatever the platform default is. So
`ToolChain::getDefaultLinker()` should return `ld` as the variable
`DefaultLinker` currently says and the logic should be in
`ToolChain::GetLinkerPath()`
phosek updated this revision to Diff 76720.
phosek marked 3 inline comments as done.
Repository:
rL LLVM
https://reviews.llvm.org/D25263
Files:
CMakeLists.txt
include/clang/Config/config.h.cmake
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains.cpp
lib
Hahnfeld added a comment.
Have you run all tests with `CLANG_DEFAULT_LINKER` not being the platform
default? I imagine there might be some tests that expect `ld` to be used...
Comment at: CMakeLists.txt:198
+set(CLANG_DEFAULT_LINKER "" CACHE STRING
+ "Default linker to use
bruno added inline comments.
Comment at: CMakeLists.txt:198
+set(CLANG_DEFAULT_LINKER "" CACHE STRING
+ "Default linker to use (\"bfd\" or \"gold\" or \"lld\", empty for platform
default")
mgorny wrote:
> Is there a reason not to allow using the absolute path
mgorny added inline comments.
Comment at: CMakeLists.txt:198
+set(CLANG_DEFAULT_LINKER "" CACHE STRING
+ "Default linker to use (\"bfd\" or \"gold\" or \"lld\", empty for platform
default")
Is there a reason not to allow using the absolute path here, like for
phosek created this revision.
phosek added a subscriber: cfe-commits.
phosek set the repository for this revision to rL LLVM.
Herald added subscribers: mgorny, beanz.
This change allows setting the default linker used by the Clang driver when
configuring the build.
Repository:
rL LLVM
https:
13 matches
Mail list logo
