[Driver] Allow setting the default linker during build

[phosek]

Summary

This change allows setting the default linker used by the Clang driver when configuring the build.

[mgorny]

CMakeLists.txt

198

Is there a reason not to allow using the absolute path here, like for the command-line option?

[bruno]

CMakeLists.txt

198

I agree here, if we're adding a cmake options for this, it should accept full paths to the linker to be used (without any need for its type like gold, bfd, etc) as well.

Additionally, if "" maps to "ld", plain CLANG_DEFAULT_LINKER="ld" should also work here.

[Hahnfeld]

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...

CMakeLists.txt

198

I agree with both points here.

lib/Driver/ToolChain.cpp

352

I wonder whether this is really correct: If DefaultLinker is not ld (it is lld for some ToolChains), -fuse-ld= with an empty argument should probably not use ld but rather whatever DefaultLinker says...

[Hahnfeld]

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().
2. The CMake variable is documented to be empty for the platform default. I think this will currently not work when used by GetProgramPath(getDefaultLinker()), will it?

lib/Driver/ToolChain.cpp

367

You can leave this return because Clang will exit anyway after emitting the error (as I had to learn...)

710–713

I think this could go into the header

[sfertile]

lib/Driver/ToolChain.cpp

710–713

The CLANG_DEFAULT_LINKER macro is getting defined in "clang/Config/config.h" which isn't meant to be included in other headers.

[Hahnfeld]

lib/Driver/ToolChain.cpp

710–713

Right, that does not work for the current version of the patch.

It was meant in conjunction with this comment:

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]

lib/Driver/ToolChain.cpp

352

I'm wandering whether we shouldn't use "platform" instead of "ld" here to match what we do for -rtlib= and -stdlib=?

[Hahnfeld]

LGTM with one nit

lib/Driver/ToolChain.cpp

352

I'd say this is fine for compatibility

356

I think this will give an error for -fuse-ld= with no argument. Please revert the condition above to UseLinker.empty() || UseLinker == "ld" to match the current behaviour

CMakeLists.txt

 set(DEFAULT_SYSROOT "" CACHE PATH
   "Default <path> to all compiler invocations for --sysroot=<path>." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")
 
 set(ENABLE_X86_RELAX_RELOCATIONS OFF CACHE BOOL
     "enable x86 relax relocations by default")
 
+set(CLANG_DEFAULT_LINKER "" CACHE STRING
+  "Default linker to use (\"bfd\" or \"gold\" or \"lld\", empty for platform default")
+if (NOT(CLANG_DEFAULT_LINKER STREQUAL "" OR
+        CLANG_DEFAULT_LINKER STREQUAL "bfd" OR
+        CLANG_DEFAULT_LINKER STREQUAL "gold" OR
+        CLANG_DEFAULT_LINKER STREQUAL "lld"))
+  message(WARNING "Resetting default linker to use platform default")
+  set(CLANG_DEFAULT_LINKER "" CACHE STRING
+    "Default linker to use (\"bfd\" or \"gold\" or \"lld\", empty for platform default" FORCE)
+endif()
+
 set(CLANG_DEFAULT_CXX_STDLIB "" CACHE STRING
   "Default C++ stdlib to use (\"libstdc++\" or \"libc++\", empty for platform default")
 if (NOT(CLANG_DEFAULT_CXX_STDLIB STREQUAL "" OR
         CLANG_DEFAULT_CXX_STDLIB STREQUAL "libstdc++" OR
         CLANG_DEFAULT_CXX_STDLIB STREQUAL "libc++"))
   message(WARNING "Resetting default C++ stdlib to use platform default")
   set(CLANG_DEFAULT_CXX_STDLIB "" CACHE STRING
     "Default C++ stdlib to use (\"libstdc++\" or \"libc++\", empty for platform default" FORCE)

include/clang/Config/config.h.cmake

 /* This generated file is for internal use. Do not include it from headers. */
 
 #ifdef CLANG_CONFIG_H
 #error config.h can only be included once
 #else
 #define CLANG_CONFIG_H
 
 /* Bug report URL. */
 #define BUG_REPORT_URL "${BUG_REPORT_URL}"
 
+/* Default linker to use. */
+#define CLANG_DEFAULT_LINKER "${CLANG_DEFAULT_LINKER}"
+
 /* Default C++ stdlib to use. */
 #define CLANG_DEFAULT_CXX_STDLIB "${CLANG_DEFAULT_CXX_STDLIB}"
 
 /* Default runtime library to use. */
 #define CLANG_DEFAULT_RTLIB "${CLANG_DEFAULT_RTLIB}"
 
 /* Default OpenMP runtime used by -fopenmp. */
 #define CLANG_DEFAULT_OPENMP_RUNTIME "${CLANG_DEFAULT_OPENMP_RUNTIME}"

lib/Driver/ToolChain.cpp

std::string ToolChain::GetFilePath(const char *Name) const {
   return D.GetFilePath(Name, *this);
 }
 
 std::string ToolChain::GetProgramPath(const char *Name) const {
   return D.GetProgramPath(Name, *this);
 }
 
 std::string ToolChain::GetLinkerPath() const {
-  if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-    StringRef UseLinker = A->getValue();
+  const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
 
-    if (llvm::sys::path::is_absolute(UseLinker)) {
-      // If we're passed -fuse-ld= with what looks like an absolute path,
-      // don't attempt to second-guess that.
-      if (llvm::sys::fs::exists(UseLinker))
-        return UseLinker;
-    } else {
-      // If we're passed -fuse-ld= with no argument, or with the argument ld,
-      // then use whatever the default system linker is.
-      if (UseLinker.empty() || UseLinker == "ld")
-        return GetProgramPath("ld");
-
-      llvm::SmallString<8> LinkerName("ld.");
-      LinkerName.append(UseLinker);
+  } else if (A && (UseLinker.empty() || UseLinker == "ld")) {
+  } else if (!UseLinker.empty()) {
 
-      std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-      if (llvm::sys::fs::exists(LinkerPath))
-        return LinkerPath;
-    }
 
+  if (A)
     getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
-    return "";
-  }
 
   return GetProgramPath(DefaultLinker);
 }
 
 types::ID ToolChain::LookupTypeForExtension(const char *Ext) const {
   return types::lookupTypeForExtension(Ext);
 }
 
if (getTriple().getArch() == llvm::Triple::x86 ||
     Res |= CFIICall;
   return Res;
 }
 
 void ToolChain::AddCudaIncludeArgs(const ArgList &DriverArgs,
                                    ArgStringList &CC1Args) const {}
 
 void ToolChain::AddIAMCUIncludeArgs(const ArgList &DriverArgs,
                                     ArgStringList &CC1Args) const {}