[RISCV] Pick the correct RISCV linker instead of calling riscv-gcc to link

[mgrang]

Summary

Currently we are invoking riscv-gcc to link. This causes all flags passed to clang to be passed down to gcc as well.
As a result we run into errors for flags not recognzied by gcc.

[asb]

My main concern with this patch is that the description doesn't really match what it does. The current in-tree code _doesn't_ call gcc to link for the tested configuration (a multilib toolchain), and this is verified with the tests in test/Driver/riscv32-toolchain.c. D39963 originally added support for a baremetal toolchain, but was changed to focus on the multilib Linux toolchain for a couple of reasons:

1. This was a more straight-forward change
2. Downstream users such as yourself had a higher priority for building for a Linux target
3. It looks like we'd want to discuss the possibility of adding RISC-V support to lib/Driver/ToolChains/BareMetal rather than adding yet another target-specific toolchain

This patch seems identical to my changes to add a baremetal toolchain (https://github.com/lowRISC/riscv-llvm/blob/master/clang/0003-RISCV-Implement-clang-driver-for-the-baremetal-RISCV.patch which was previously submitted as part of D39963), with a line changed (setting a different name for the linker). Was it meant to be a one-line patch on top of that work?

Are you compiling for a Linux target or for bare metal? One problem with the multilib detection code is that if it fails it tends to do so silently. I see two paths forward depending on the immediate problem you're seeing:

1. If you're seeing an issue with the correct linker being selected when compiling using a Linux toolchain, the correct fix would be to modify that detection logic committed in rL322276 and add new tests
2. If the issue is that you're now trying to use Clang for a bare-metal target, we should discuss whether to move forwards with my baremetal patch as-is or to try to modify lib/Driver/ToolChains/BareMetal - probably worth discussing this on cfe-dev.

The fact you've hardcoded "riscv32-unknown-linux-gnu-ld" as the linker name makes me wonder if the current issue you're seeing is 1)?

[mgrang]

In D42673#992056, @asb wrote:

My main concern with this patch is that the description doesn't really match what it does. The current in-tree code _doesn't_ call gcc to link for the tested configuration (a multilib toolchain), and this is verified with the tests in test/Driver/riscv32-toolchain.c. D39963 originally added support for a baremetal toolchain, but was changed to focus on the multilib Linux toolchain for a couple of reasons:

1. This was a more straight-forward change
2. Downstream users such as yourself had a higher priority for building for a Linux target
3. It looks like we'd want to discuss the possibility of adding RISC-V support to lib/Driver/ToolChains/BareMetal rather than adding yet another target-specific toolchain

This patch seems identical to my changes to add a baremetal toolchain (https://github.com/lowRISC/riscv-llvm/blob/master/clang/0003-RISCV-Implement-clang-driver-for-the-baremetal-RISCV.patch which was previously submitted as part of D39963), with a line changed (setting a different name for the linker). Was it meant to be a one-line patch on top of that work?

Are you compiling for a Linux target or for bare metal? One problem with the multilib detection code is that if it fails it tends to do so silently. I see two paths forward depending on the immediate problem you're seeing:

1. If you're seeing an issue with the correct linker being selected when compiling using a Linux toolchain, the correct fix would be to modify that detection logic committed in rL322276 and add new tests
2. If the issue is that you're now trying to use Clang for a bare-metal target, we should discuss whether to move forwards with my baremetal patch as-is or to try to modify lib/Driver/ToolChains/BareMetal - probably worth discussing this on cfe-dev.

The fact you've hardcoded "riscv32-unknown-linux-gnu-ld" as the linker name makes me wonder if the current issue you're seeing is 1)?

Alex,
This patch is based on your previously submitted patch as part of D39963. The issue is that if I pass -target riscv32 then it invokes riscv32-gcc but if I pass -target riscv32-unknown-linux-gnu then is correctly invokes riscv32-unknown-linux-gnu-ld. So what we need to sort out is the behavior we want for the -target option. We should let it fail to find the linker if the complete triple is not passed rather than invoking the gcc driver (that's the behavior on ARM).

[mgrang]

For now, by setting the full triple (riscv-unknown-linux-gnu) we are able to invoke the correct gnu linker. So will abandon this patch.

lib/Driver/CMakeLists.txt

add_clang_library(clangDriver
   ToolChains/MinGW.cpp
   ToolChains/Minix.cpp
   ToolChains/MSVC.cpp
   ToolChains/Myriad.cpp
   ToolChains/NaCl.cpp
   ToolChains/NetBSD.cpp
   ToolChains/OpenBSD.cpp
   ToolChains/PS4CPU.cpp
+  ToolChains/RISCV.cpp
   ToolChains/Solaris.cpp
   ToolChains/TCE.cpp
   ToolChains/WebAssembly.cpp
   ToolChains/XCore.cpp
   Types.cpp
   XRayArgs.cpp
 
   DEPENDS
   ClangDriverOptions
 
   LINK_LIBS
   clangBasic
   ${system_libs}
   )

lib/Driver/Driver.cpp

 #include "ToolChains/Minix.h"
 #include "ToolChains/MipsLinux.h"
 #include "ToolChains/MSVC.h"
 #include "ToolChains/Myriad.h"
 #include "ToolChains/NaCl.h"
 #include "ToolChains/NetBSD.h"
 #include "ToolChains/OpenBSD.h"
 #include "ToolChains/PS4CPU.h"
+#include "ToolChains/RISCV.h"
 #include "ToolChains/Solaris.h"
 #include "ToolChains/TCE.h"
 #include "ToolChains/WebAssembly.h"
 #include "ToolChains/XCore.h"
 #include "clang/Basic/Version.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Action.h"
default:
         break;
       case llvm::Triple::wasm32:
       case llvm::Triple::wasm64:
         TC = llvm::make_unique<toolchains::WebAssembly>(*this, Target, Args);
         break;
       case llvm::Triple::avr:
         TC = llvm::make_unique<toolchains::AVRToolChain>(*this, Target, Args);
         break;
+      case llvm::Triple::riscv32:
+      case llvm::Triple::riscv64:
+        TC = llvm::make_unique<toolchains::RISCVToolChain>(*this, Target, Args);
+        break;
       default:
         if (Target.getVendor() == llvm::Triple::Myriad)
           TC = llvm::make_unique<toolchains::MyriadToolChain>(*this, Target,
                                                               Args);
         else if (toolchains::BareMetal::handlesTarget(Target))
           TC = llvm::make_unique<toolchains::BareMetal>(*this, Target, Args);
         else if (Target.isOSBinFormatELF())
           TC = llvm::make_unique<toolchains::Generic_ELF>(*this, Target, Args);

lib/Driver/ToolChains/RISCV.h

+//===--- RISCV.h - RISCV ToolChain Implementations --------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCV_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCV_H
+
+#include "Gnu.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY RISCVToolChain : public Generic_ELF {
+public:
+  RISCVToolChain(const Driver &D, const llvm::Triple &Triple,
+                 const llvm::opt::ArgList &Args);
+
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+                            llvm::opt::ArgStringList &CC1Args) const override;
+
+protected:
+  Tool *buildLinker() const override;
+};
+
+} // end namespace toolchains
+
+namespace tools {
+namespace RISCV {
+class LLVM_LIBRARY_VISIBILITY Linker : public GnuTool {
+public:
+  Linker(const ToolChain &TC)
+      : GnuTool("RISCV::Linker",
+                TC.getTriple().isArch64Bit() ? "riscv64-unknown-elf-ld"
+                                             //: "riscv32-unknown-elf-ld",
+                                             : "riscv32-unknown-linux-gnu-ld",
+                TC) {}
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+  void ConstructJob(Compilation &C, const JobAction &JA,
+                    const InputInfo &Output, const InputInfoList &Inputs,
+                    const llvm::opt::ArgList &TCArgs,
+                    const char *LinkingOutput) const override;
+};
+} // end namespace RISCV
+} // end namespace tools
+
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_RISCV_H

lib/Driver/ToolChains/RISCV.cpp

+//===--- RISCV.cpp - RISCV ToolChain Implementations ------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "RISCV.h"
+#include "CommonArgs.h"
+#include "InputInfo.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang::driver;
+using namespace clang::driver::toolchains;
+using namespace clang::driver::tools;
+using namespace clang;
+using namespace llvm::opt;
+
+/// RISCV Toolchain
+RISCVToolChain::RISCVToolChain(const Driver &D, const llvm::Triple &Triple,
+                               const ArgList &Args)
+    : Generic_ELF(D, Triple, Args) {
+  GCCInstallation.init(Triple, Args);
+  getFilePaths().push_back(D.SysRoot + "/lib");
+  if (GCCInstallation.isValid()) {
+    getFilePaths().push_back(GCCInstallation.getInstallPath().str());
+    getProgramPaths().push_back(
+        (GCCInstallation.getParentLibPath() + "/../bin").str());
+  }
+}
+
+Tool *RISCVToolChain::buildLinker() const {
+  return new tools::RISCV::Linker(*this);
+}
+
+void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
+                                               ArgStringList &CC1Args) const {
+  if (DriverArgs.hasArg(options::OPT_nostdinc))
+    return;
+
+  if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
+    SmallString<128> Dir(getDriver().SysRoot);
+    llvm::sys::path::append(Dir, "include");
+    addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+}
+
+void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
+                                 const InputInfo &Output,
+                                 const InputInfoList &Inputs,
+                                 const ArgList &Args,
+                                 const char *LinkingOutput) const {
+  const ToolChain &ToolChain = getToolChain();
+  const Driver &D = ToolChain.getDriver();
+  ArgStringList CmdArgs;
+
+  if (!D.SysRoot.empty())
+    CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
+
+  std::string Linker = getToolChain().GetProgramPath(getShortName());
+
+  bool WantCRTs =
+      !Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles);
+
+  if (WantCRTs) {
+    CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+    CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
+  }
+
+  Args.AddAllArgs(CmdArgs, options::OPT_L);
+  ToolChain.AddFilePathLibArgs(Args, CmdArgs);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_T_Group, options::OPT_e, options::OPT_s,
+                   options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
+
+  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+
+  // TODO: add C++ includes and libs if compiling C++.
+
+  if (!Args.hasArg(options::OPT_nostdlib) &&
+      !Args.hasArg(options::OPT_nodefaultlibs)) {
+    CmdArgs.push_back("--start-group");
+    CmdArgs.push_back("-lc");
+    CmdArgs.push_back("-lgloss");
+    CmdArgs.push_back("--end-group");
+    CmdArgs.push_back("-lgcc");
+  }
+
+  if (WantCRTs)
+    CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtend.o")));
+
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(llvm::make_unique<Command>(JA, *this, Args.MakeArgString(Linker),
+                                          CmdArgs, Inputs));
+}
+// RISCV tools end.