Skip to content

[RISCV] Merging RISCVToolChain and BareMetal toolchains #118809

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quic-garvgupt
Copy link
Contributor

Currently, LLVM has two RISC-V toolchain classes in Clang for baremetal development, creating unnecessary maintenance overhead. This patch extends the BareMetal toolchain to support an existing GCC installation, resolving this issue.

The latest patchset preserves the behavior of both toolchain objects with minor differences. If no --sysroot option is passed on the command line or if the GCC installation is invalid, the sysroot will first be formed as per the RISCVToolChain baremetal object. If this path does not exist, the sysroot will be formed as per the BareMetal toolchain object.

Additionally, the presence of --gcc-toolchain or --gcc-install-dir will imply that GNU linker is the default linker unless otherwise a differnt linker is passed through -fuse-ld flag.

RFC - https://discourse.llvm.org/t/merging-riscvtoolchain-and-baremetal-toolchains/75524

@quic-garvgupt quic-garvgupt self-assigned this Dec 5, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Dec 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 5, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Garvit Gupta (quic-garvgupt)

Changes

Currently, LLVM has two RISC-V toolchain classes in Clang for baremetal development, creating unnecessary maintenance overhead. This patch extends the BareMetal toolchain to support an existing GCC installation, resolving this issue.

The latest patchset preserves the behavior of both toolchain objects with minor differences. If no --sysroot option is passed on the command line or if the GCC installation is invalid, the sysroot will first be formed as per the RISCVToolChain baremetal object. If this path does not exist, the sysroot will be formed as per the BareMetal toolchain object.

Additionally, the presence of --gcc-toolchain or --gcc-install-dir will imply that GNU linker is the default linker unless otherwise a differnt linker is passed through -fuse-ld flag.

RFC - https://discourse.llvm.org/t/merging-riscvtoolchain-and-baremetal-toolchains/75524


Patch is 50.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/118809.diff

12 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (-4)
  • (modified) clang/lib/Driver/ToolChains/BareMetal.cpp (+189-36)
  • (modified) clang/lib/Driver/ToolChains/BareMetal.h (+24-10)
  • (added) clang/test/Driver/arm-gnutools.c (+12)
  • (modified) clang/test/Driver/baremetal-multilib.yaml (+2-2)
  • (modified) clang/test/Driver/baremetal-sysroot.cpp (+2-2)
  • (modified) clang/test/Driver/baremetal.cpp (+75-48)
  • (modified) clang/test/Driver/riscv-args.c (+1-1)
  • (modified) clang/test/Driver/riscv32-toolchain-extra.c (+2-2)
  • (modified) clang/test/Driver/riscv32-toolchain.c (+3-3)
  • (modified) clang/test/Driver/riscv64-toolchain-extra.c (+2-2)
  • (modified) clang/test/Driver/riscv64-toolchain.c (+2-2)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 7de8341b8d2d61..c5185ccedd6201 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -6521,10 +6521,6 @@ const ToolChain &Driver::getToolChain(const ArgList &Args,
         break;
       case llvm::Triple::riscv32:
       case llvm::Triple::riscv64:
-        if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args))
-          TC =
-              std::make_unique<toolchains::RISCVToolChain>(*this, Target, Args);
-        else
           TC = std::make_unique<toolchains::BareMetal>(*this, Target, Args);
         break;
       case llvm::Triple::ve:
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index f9a73f60973e4c..1d065562e9a6ef 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -97,7 +97,8 @@ static bool findRISCVMultilibs(const Driver &D,
   return false;
 }
 
-static std::string computeBaseSysRoot(const Driver &D, bool IncludeTriple) {
+static std::string computeInstalledToolchainSysRoot(const Driver &D,
+                                                    bool IncludeTriple) {
   if (!D.SysRoot.empty())
     return D.SysRoot;
 
@@ -110,20 +111,94 @@ static std::string computeBaseSysRoot(const Driver &D, bool IncludeTriple) {
   return std::string(SysRootDir);
 }
 
+// GCC sysroot here means form sysroot from either --gcc-install-dir, or from
+// --gcc-toolchain or if the toolchain is installed alongside clang in
+// bin/../<TargetTriple> directory if it is not explicitly specified on the command
+//  line through `--sysroot` option. libc here will be newlib.
+std::string BareMetal::computeGCCSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+    return getDriver().SysRoot;
+
+  SmallString<128> SysRootDir;
+  if (GCCInstallation.isValid()) {
+    StringRef LibDir = GCCInstallation.getParentLibPath();
+    StringRef TripleStr = GCCInstallation.getTriple().str();
+    llvm::sys::path::append(SysRootDir, LibDir, "..", TripleStr);
+  } else {
+    // Use the triple as provided to the driver. Unlike the parsed triple
+    // this has not been normalized to always contain every field.
+    llvm::sys::path::append(SysRootDir, getDriver().Dir, "..",
+                            getDriver().getTargetTriple());
+  }
+
+  if (!llvm::sys::fs::exists(SysRootDir))
+    return std::string();
+
+  return std::string(SysRootDir);
+}
+
+std::string BareMetal::computeSysRoot() const {
+  if (!SysRoot.empty())
+    return SysRoot;
+
+  std::string SysRoot = getDriver().SysRoot;
+  if (!SysRoot.empty() && llvm::sys::fs::exists(SysRoot))
+    return SysRoot;
+
+  // Verify the GCC installation from -gcc-install-dir, --gcc-toolchain, or
+  // alongside clang. If valid, form the sysroot. Otherwise, check
+  // lib/clang-runtimes above the driver.
+  SysRoot = computeGCCSysRoot();
+  if (!SysRoot.empty())
+    return SysRoot;
+
+  SysRoot =
+      computeInstalledToolchainSysRoot(getDriver(), /*IncludeTriple*/ true);
+
+  return SysRoot;
+}
+
+static void addMultilibsFilePaths(const Driver &D, const MultilibSet &Multilibs,
+                                  const Multilib &Multilib,
+                                  StringRef InstallPath,
+                                  ToolChain::path_list &Paths) {
+  if (const auto &PathsCallback = Multilibs.filePathsCallback())
+    for (const auto &Path : PathsCallback(Multilib))
+      addPathIfExists(D, InstallPath + Path, Paths);
+}
+
 BareMetal::BareMetal(const Driver &D, const llvm::Triple &Triple,
                      const ArgList &Args)
-    : ToolChain(D, Triple, Args),
-      SysRoot(computeBaseSysRoot(D, /*IncludeTriple=*/true)) {
-  getProgramPaths().push_back(getDriver().Dir);
-
-  findMultilibs(D, Triple, Args);
-  SmallString<128> SysRoot(computeSysRoot());
-  if (!SysRoot.empty()) {
-    for (const Multilib &M : getOrderedMultilibs()) {
-      SmallString<128> Dir(SysRoot);
-      llvm::sys::path::append(Dir, M.osSuffix(), "lib");
-      getFilePaths().push_back(std::string(Dir));
-      getLibraryPaths().push_back(std::string(Dir));
+    : Generic_ELF(D, Triple, Args){
+  GCCInstallation.init(Triple, Args);
+  SysRoot = computeSysRoot();
+  UseLD = Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_insensitive("ld");
+  if (GCCInstallation.isValid()) {
+    Multilibs = GCCInstallation.getMultilibs();
+    SelectedMultilibs.assign({GCCInstallation.getMultilib()});
+    path_list &Paths = getFilePaths();
+    // Add toolchain/multilib specific file paths.
+    addMultilibsFilePaths(D, Multilibs, SelectedMultilibs.back(),
+                          GCCInstallation.getInstallPath(), Paths);
+    getFilePaths().push_back(GCCInstallation.getInstallPath().str());
+    ToolChain::path_list &PPaths = getProgramPaths();
+    // Multilib cross-compiler GCC installations put ld in a triple-prefixed
+    // directory off of the parent of the GCC installation.
+    PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
+                           GCCInstallation.getTriple().str() + "/bin")
+                         .str());
+    PPaths.push_back((GCCInstallation.getParentLibPath() + "/../bin").str());
+    getFilePaths().push_back(computeSysRoot() + "/lib");
+  } else {
+    getProgramPaths().push_back(getDriver().Dir);
+    findMultilibs(D, Triple, Args);
+    if (!SysRoot.empty()) {
+      for (const Multilib &M : getOrderedMultilibs()) {
+        SmallString<128> Dir(SysRoot);
+        llvm::sys::path::append(Dir, M.osSuffix(), "lib");
+        getFilePaths().push_back(std::string(Dir));
+        getLibraryPaths().push_back(std::string(Dir));
+      }
     }
   }
 }
@@ -236,7 +311,7 @@ getMultilibConfigPath(const Driver &D, const llvm::Triple &Triple,
       return {};
     }
   } else {
-    MultilibPath = computeBaseSysRoot(D, /*IncludeTriple=*/false);
+    MultilibPath = computeInstalledToolchainSysRoot(D, /*IncludeTriple=*/false);
     llvm::sys::path::append(MultilibPath, MultilibFilename);
   }
   return MultilibPath;
@@ -254,7 +329,7 @@ void BareMetal::findMultilibs(const Driver &D, const llvm::Triple &Triple,
   if (D.getVFS().exists(*MultilibPath)) {
     // If multilib.yaml is found, update sysroot so it doesn't use a target
     // specific suffix
-    SysRoot = computeBaseSysRoot(D, /*IncludeTriple=*/false);
+    SysRoot = computeInstalledToolchainSysRoot(D, /*IncludeTriple=*/false);
     findMultilibsFromYAML(*this, D, *MultilibPath, Args, Result);
     SelectedMultilibs = Result.SelectedMultilibs;
     Multilibs = Result.Multilibs;
@@ -279,8 +354,6 @@ Tool *BareMetal::buildStaticLibTool() const {
   return new tools::baremetal::StaticLibTool(*this);
 }
 
-std::string BareMetal::computeSysRoot() const { return SysRoot; }
-
 BareMetal::OrderedMultilibs BareMetal::getOrderedMultilibs() const {
   // Get multilibs in reverse order because they're ordered most-specific last.
   if (!SelectedMultilibs.empty())
@@ -291,6 +364,36 @@ BareMetal::OrderedMultilibs BareMetal::getOrderedMultilibs() const {
   return llvm::reverse(Default);
 }
 
+ToolChain::CXXStdlibType BareMetal::GetDefaultCXXStdlibType() const {
+  if (getTriple().isRISCV()) {
+    return GCCInstallation.isValid() ? ToolChain::CST_Libstdcxx
+                                     : ToolChain::CST_Libcxx;
+  }
+  return ToolChain::CST_Libcxx;
+}
+
+ToolChain::RuntimeLibType BareMetal::GetDefaultRuntimeLibType() const {
+  if (getTriple().isRISCV()) {
+    return GCCInstallation.isValid() ? ToolChain::RLT_Libgcc
+                                     : ToolChain::RLT_CompilerRT;
+  }
+  return ToolChain::RLT_CompilerRT;
+}
+
+ToolChain::UnwindLibType
+BareMetal::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
+  if (getTriple().isRISCV())
+    return ToolChain::UNW_None;
+
+  return ToolChain::GetUnwindLibType(Args);
+}
+
+const char *BareMetal::getDefaultLinker() const {
+  if(isUsingLD())
+    return "ld";
+  return "ld.lld";
+}
+
 void BareMetal::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
                                           ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc))
@@ -325,6 +428,19 @@ void BareMetal::addClangTargetOptions(const ArgList &DriverArgs,
   CC1Args.push_back("-nostdsysteminc");
 }
 
+void BareMetal::addLibStdCxxIncludePaths(
+    const llvm::opt::ArgList &DriverArgs,
+    llvm::opt::ArgStringList &CC1Args) const {
+  if (GCCInstallation.isValid()) {
+    const GCCVersion &Version = GCCInstallation.getVersion();
+    StringRef TripleStr = GCCInstallation.getTriple().str();
+    const Multilib &Multilib = GCCInstallation.getMultilib();
+    addLibStdCXXIncludePaths(computeSysRoot() + "/include/c++/" + Version.Text,
+                            TripleStr, Multilib.includeSuffix(), DriverArgs,
+                            CC1Args);
+  }
+}
+
 void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
                                              ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc,
@@ -355,15 +471,15 @@ void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
   };
 
   switch (GetCXXStdlibType(DriverArgs)) {
-    case ToolChain::CST_Libcxx: {
-      SmallString<128> P(D.Dir);
-      llvm::sys::path::append(P, "..", "include");
-      AddCXXIncludePath(P);
-      break;
-    }
-    case ToolChain::CST_Libstdcxx:
-      // We only support libc++ toolchain installation.
-      break;
+  case ToolChain::CST_Libcxx: {
+    SmallString<128> P(D.Dir);
+    llvm::sys::path::append(P, "..", "include");
+    AddCXXIncludePath(P);
+    break;
+  }
+  case ToolChain::CST_Libstdcxx:
+    addLibStdCxxIncludePaths(DriverArgs, CC1Args);
+    break;
   }
 
   std::string SysRoot(computeSysRoot());
@@ -428,6 +544,10 @@ void BareMetal::AddCXXStdlibLibArgs(const ArgList &Args,
     CmdArgs.push_back("-lsupc++");
     break;
   }
+
+  if (getTriple().isRISCV() && GCCInstallation.isValid())
+    return;
+
   CmdArgs.push_back("-lunwind");
 }
 
@@ -503,12 +623,22 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   const llvm::Triple::ArchType Arch = TC.getArch();
   const llvm::Triple &Triple = getToolChain().getEffectiveTriple();
 
-  AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
+  if (!D.SysRoot.empty())
+    CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
 
+  Args.addAllArgs(CmdArgs, {options::OPT_u});
   CmdArgs.push_back("-Bstatic");
 
-  if (TC.getTriple().isRISCV() && Args.hasArg(options::OPT_mno_relax))
-    CmdArgs.push_back("--no-relax");
+  if (TC.getTriple().isRISCV()) {
+    if (Args.hasArg(options::OPT_mno_relax))
+      CmdArgs.push_back("--no-relax");
+    if (TC.isUsingLD()) {
+      CmdArgs.push_back("-m");
+      CmdArgs.push_back(TC.getArch() == llvm::Triple::riscv64 ? "elf64lriscv"
+                                                              : "elf32lriscv");
+    }
+    CmdArgs.push_back("-X");
+  }
 
   if (Triple.isARM() || Triple.isThumb()) {
     bool IsBigEndian = arm::isARMBigEndian(Triple, Args);
@@ -519,9 +649,24 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back(Arch == llvm::Triple::aarch64_be ? "-EB" : "-EL");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
-                   options::OPT_r)) {
-    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crt0.o")));
+  bool WantCRTs =
+      !Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles);
+
+  const char *crtbegin, *crtend;
+  if (WantCRTs) {
+    if (!Args.hasArg(options::OPT_r))
+      CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crt0.o")));
+    auto RuntimeLib = TC.GetRuntimeLibType(Args);
+    if (RuntimeLib == ToolChain::RLT_Libgcc) {
+      crtbegin = "crtbegin.o";
+      crtend = "crtend.o";
+    } else {
+      assert(RuntimeLib == ToolChain::RLT_CompilerRT);
+      crtbegin =
+          TC.getCompilerRTArgString(Args, "crtbegin", ToolChain::FT_Object);
+      crtend = TC.getCompilerRTArgString(Args, "crtend", ToolChain::FT_Object);
+    }
+    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath(crtbegin)));
   }
 
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
@@ -536,12 +681,20 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     TC.AddCXXStdlibLibArgs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
-    CmdArgs.push_back("-lc");
     CmdArgs.push_back("-lm");
-
+    if (TC.isUsingLD())
+      CmdArgs.push_back("--start-group");
+    CmdArgs.push_back("-lc");
+    if (TC.isUsingLD()) {
+      CmdArgs.push_back("-lgloss");
+      CmdArgs.push_back("--end-group");
+    }
     TC.AddLinkRuntimeLib(Args, CmdArgs);
   }
 
+  if (WantCRTs)
+    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath(crtend)));
+
   if (D.isUsingLTO()) {
     assert(!Inputs.empty() && "Must have at least one input.");
     // Find the first filename InputInfo object.
@@ -555,8 +708,8 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     addLTOOptions(TC, Args, CmdArgs, Output, *Input,
                   D.getLTOMode() == LTOK_Thin);
   }
-  if (TC.getTriple().isRISCV())
-    CmdArgs.push_back("-X");
+
+  AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
 
   // The R_ARM_TARGET2 relocation must be treated as R_ARM_REL32 on arm*-*-elf
   // and arm*-*-eabi (the default is R_ARM_GOT_PREL, used on arm*-*-linux and
diff --git a/clang/lib/Driver/ToolChains/BareMetal.h b/clang/lib/Driver/ToolChains/BareMetal.h
index b385c8cf76aab0..4fbf6a563784a1 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.h
+++ b/clang/lib/Driver/ToolChains/BareMetal.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
 
+#include "ToolChains/Gnu.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 
@@ -19,7 +20,7 @@ namespace driver {
 
 namespace toolchains {
 
-class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
+class LLVM_LIBRARY_VISIBILITY BareMetal : public Generic_ELF {
 public:
   BareMetal(const Driver &D, const llvm::Triple &Triple,
             const llvm::opt::ArgList &Args);
@@ -35,7 +36,7 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   Tool *buildStaticLibTool() const override;
 
 public:
-  bool useIntegratedAs() const override { return true; }
+  bool isUsingLD() const { return UseLD || GCCInstallation.isValid(); }
   bool isBareMetal() const override { return true; }
   bool isCrossCompiling() const override { return true; }
   bool HasNativeLLVMSupport() const override { return true; }
@@ -48,14 +49,18 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
 
   StringRef getOSLibName() const override { return "baremetal"; }
 
-  RuntimeLibType GetDefaultRuntimeLibType() const override {
-    return ToolChain::RLT_CompilerRT;
-  }
-  CXXStdlibType GetDefaultCXXStdlibType() const override {
-    return ToolChain::CST_Libcxx;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override {
+    return UnwindTableLevel::None;
   }
 
-  const char *getDefaultLinker() const override { return "ld.lld"; }
+  CXXStdlibType GetDefaultCXXStdlibType() const override;
+
+  RuntimeLibType GetDefaultRuntimeLibType() const override;
+
+  UnwindLibType GetUnwindLibType(const llvm::opt::ArgList &Args) const override;
+
+  const char *getDefaultLinker() const override;
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
@@ -67,6 +72,9 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   void AddClangCXXStdlibIncludeArgs(
       const llvm::opt::ArgList &DriverArgs,
       llvm::opt::ArgStringList &CC1Args) const override;
+  void
+  addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
+                           llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs) const override;
   void AddLinkRuntimeLib(const llvm::opt::ArgList &Args,
@@ -78,8 +86,9 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   using OrderedMultilibs =
       llvm::iterator_range<llvm::SmallVector<Multilib>::const_reverse_iterator>;
   OrderedMultilibs getOrderedMultilibs() const;
-
+  bool UseLD;
   std::string SysRoot;
+  std::string computeGCCSysRoot() const;
 };
 
 } // namespace toolchains
@@ -103,7 +112,12 @@ class LLVM_LIBRARY_VISIBILITY StaticLibTool : public Tool {
 
 class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
 public:
-  Linker(const ToolChain &TC) : Tool("baremetal::Linker", "ld.lld", TC) {}
+  Linker(const ToolChain &TC)
+      : Tool("baremetal::Linker",
+             static_cast<const toolchains::BareMetal &>(TC).isUsingLD()
+                 ? "ld"
+                 : "ld.lld",
+             TC) {}
   bool isLinkJob() const override { return true; }
   bool hasIntegratedCPP() const override { return false; }
   void ConstructJob(Compilation &C, const JobAction &JA,
diff --git a/clang/test/Driver/arm-gnutools.c b/clang/test/Driver/arm-gnutools.c
new file mode 100644
index 00000000000000..127e40dc74da7a
--- /dev/null
+++ b/clang/test/Driver/arm-gnutools.c
@@ -0,0 +1,12 @@
+// check that gnu assembler is invoked with arm baremetal as well
+
+// RUN: %clang --target=armv6m-none-eabi  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang --target=armv7-none-eabi  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang --target=aarch64-none-elf  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: "{{.*}}as{{(.exe)?}}"
\ No newline at end of file
diff --git a/clang/test/Driver/baremetal-multilib.yaml b/clang/test/Driver/baremetal-multilib.yaml
index b6bfd0ed3a94cb..58e66ba3d9a7e7 100644
--- a/clang/test/Driver/baremetal-multilib.yaml
+++ b/clang/test/Driver/baremetal-multilib.yaml
@@ -8,9 +8,9 @@
 # CHECK-SAME: "-internal-isystem" "[[SYSROOT:[^"]*]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include/c++/v1"
 # CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include"
 # CHECK-SAME: "-x" "c++" "{{.*}}baremetal-multilib.yaml"
-# CHECK-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+# CHECK-NEXT: ld{{(.exe)?}}" "-Bstatic"
 # CHECK-SAME: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/lib"
-# CHECK-SAME: "-lc" "-lm" "{{[^"]*}}libclang_rt.builtins.a"
+# CHECK-SAME: "-lm" "-lc" "{{[^"]*}}libclang_rt.builtins.a"
 # CHECK-SAME: "-o" "{{.*}}.tmp.out"
 
 # RUN: %clang --multi-lib-config=%s -no-canonical-prefixes -x c++ %s -### -o %t.out 2>&1 \
diff --git a/clang/test/Driver/baremetal-sysroot.cpp b/clang/test/Driver/baremetal-sysroot.cpp
index 18654be33b87c9..56cf738830aadc 100644
--- a/clang/test/Driver/baremetal-sysroot.cpp
+++ b/clang/test/Driver/baremetal-sysroot.cpp
@@ -16,7 +16,7 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" "{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECk-V6M-C-SAME: "-internal-isystem" "{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include"
 // CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal-sysroot.cpp"
-// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}lib"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "{{[^"]*}}libclang_rt.builtins.a"
+// CHECK-V6M-C-SAME: "-lm" "-lc" "{{[^"]*}}libclang_rt.builtins.a"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index f09d7361e6c138..1eb69b4b49121a 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -15,11 +15,12 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSR...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Dec 5, 2024

@llvm/pr-subscribers-clang

Author: Garvit Gupta (quic-garvgupt)

Changes

Currently, LLVM has two RISC-V toolchain classes in Clang for baremetal development, creating unnecessary maintenance overhead. This patch extends the BareMetal toolchain to support an existing GCC installation, resolving this issue.

The latest patchset preserves the behavior of both toolchain objects with minor differences. If no --sysroot option is passed on the command line or if the GCC installation is invalid, the sysroot will first be formed as per the RISCVToolChain baremetal object. If this path does not exist, the sysroot will be formed as per the BareMetal toolchain object.

Additionally, the presence of --gcc-toolchain or --gcc-install-dir will imply that GNU linker is the default linker unless otherwise a differnt linker is passed through -fuse-ld flag.

RFC - https://discourse.llvm.org/t/merging-riscvtoolchain-and-baremetal-toolchains/75524


Patch is 50.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/118809.diff

12 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (-4)
  • (modified) clang/lib/Driver/ToolChains/BareMetal.cpp (+189-36)
  • (modified) clang/lib/Driver/ToolChains/BareMetal.h (+24-10)
  • (added) clang/test/Driver/arm-gnutools.c (+12)
  • (modified) clang/test/Driver/baremetal-multilib.yaml (+2-2)
  • (modified) clang/test/Driver/baremetal-sysroot.cpp (+2-2)
  • (modified) clang/test/Driver/baremetal.cpp (+75-48)
  • (modified) clang/test/Driver/riscv-args.c (+1-1)
  • (modified) clang/test/Driver/riscv32-toolchain-extra.c (+2-2)
  • (modified) clang/test/Driver/riscv32-toolchain.c (+3-3)
  • (modified) clang/test/Driver/riscv64-toolchain-extra.c (+2-2)
  • (modified) clang/test/Driver/riscv64-toolchain.c (+2-2)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 7de8341b8d2d61..c5185ccedd6201 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -6521,10 +6521,6 @@ const ToolChain &Driver::getToolChain(const ArgList &Args,
         break;
       case llvm::Triple::riscv32:
       case llvm::Triple::riscv64:
-        if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args))
-          TC =
-              std::make_unique<toolchains::RISCVToolChain>(*this, Target, Args);
-        else
           TC = std::make_unique<toolchains::BareMetal>(*this, Target, Args);
         break;
       case llvm::Triple::ve:
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index f9a73f60973e4c..1d065562e9a6ef 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -97,7 +97,8 @@ static bool findRISCVMultilibs(const Driver &D,
   return false;
 }
 
-static std::string computeBaseSysRoot(const Driver &D, bool IncludeTriple) {
+static std::string computeInstalledToolchainSysRoot(const Driver &D,
+                                                    bool IncludeTriple) {
   if (!D.SysRoot.empty())
     return D.SysRoot;
 
@@ -110,20 +111,94 @@ static std::string computeBaseSysRoot(const Driver &D, bool IncludeTriple) {
   return std::string(SysRootDir);
 }
 
+// GCC sysroot here means form sysroot from either --gcc-install-dir, or from
+// --gcc-toolchain or if the toolchain is installed alongside clang in
+// bin/../<TargetTriple> directory if it is not explicitly specified on the command
+//  line through `--sysroot` option. libc here will be newlib.
+std::string BareMetal::computeGCCSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+    return getDriver().SysRoot;
+
+  SmallString<128> SysRootDir;
+  if (GCCInstallation.isValid()) {
+    StringRef LibDir = GCCInstallation.getParentLibPath();
+    StringRef TripleStr = GCCInstallation.getTriple().str();
+    llvm::sys::path::append(SysRootDir, LibDir, "..", TripleStr);
+  } else {
+    // Use the triple as provided to the driver. Unlike the parsed triple
+    // this has not been normalized to always contain every field.
+    llvm::sys::path::append(SysRootDir, getDriver().Dir, "..",
+                            getDriver().getTargetTriple());
+  }
+
+  if (!llvm::sys::fs::exists(SysRootDir))
+    return std::string();
+
+  return std::string(SysRootDir);
+}
+
+std::string BareMetal::computeSysRoot() const {
+  if (!SysRoot.empty())
+    return SysRoot;
+
+  std::string SysRoot = getDriver().SysRoot;
+  if (!SysRoot.empty() && llvm::sys::fs::exists(SysRoot))
+    return SysRoot;
+
+  // Verify the GCC installation from -gcc-install-dir, --gcc-toolchain, or
+  // alongside clang. If valid, form the sysroot. Otherwise, check
+  // lib/clang-runtimes above the driver.
+  SysRoot = computeGCCSysRoot();
+  if (!SysRoot.empty())
+    return SysRoot;
+
+  SysRoot =
+      computeInstalledToolchainSysRoot(getDriver(), /*IncludeTriple*/ true);
+
+  return SysRoot;
+}
+
+static void addMultilibsFilePaths(const Driver &D, const MultilibSet &Multilibs,
+                                  const Multilib &Multilib,
+                                  StringRef InstallPath,
+                                  ToolChain::path_list &Paths) {
+  if (const auto &PathsCallback = Multilibs.filePathsCallback())
+    for (const auto &Path : PathsCallback(Multilib))
+      addPathIfExists(D, InstallPath + Path, Paths);
+}
+
 BareMetal::BareMetal(const Driver &D, const llvm::Triple &Triple,
                      const ArgList &Args)
-    : ToolChain(D, Triple, Args),
-      SysRoot(computeBaseSysRoot(D, /*IncludeTriple=*/true)) {
-  getProgramPaths().push_back(getDriver().Dir);
-
-  findMultilibs(D, Triple, Args);
-  SmallString<128> SysRoot(computeSysRoot());
-  if (!SysRoot.empty()) {
-    for (const Multilib &M : getOrderedMultilibs()) {
-      SmallString<128> Dir(SysRoot);
-      llvm::sys::path::append(Dir, M.osSuffix(), "lib");
-      getFilePaths().push_back(std::string(Dir));
-      getLibraryPaths().push_back(std::string(Dir));
+    : Generic_ELF(D, Triple, Args){
+  GCCInstallation.init(Triple, Args);
+  SysRoot = computeSysRoot();
+  UseLD = Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_insensitive("ld");
+  if (GCCInstallation.isValid()) {
+    Multilibs = GCCInstallation.getMultilibs();
+    SelectedMultilibs.assign({GCCInstallation.getMultilib()});
+    path_list &Paths = getFilePaths();
+    // Add toolchain/multilib specific file paths.
+    addMultilibsFilePaths(D, Multilibs, SelectedMultilibs.back(),
+                          GCCInstallation.getInstallPath(), Paths);
+    getFilePaths().push_back(GCCInstallation.getInstallPath().str());
+    ToolChain::path_list &PPaths = getProgramPaths();
+    // Multilib cross-compiler GCC installations put ld in a triple-prefixed
+    // directory off of the parent of the GCC installation.
+    PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
+                           GCCInstallation.getTriple().str() + "/bin")
+                         .str());
+    PPaths.push_back((GCCInstallation.getParentLibPath() + "/../bin").str());
+    getFilePaths().push_back(computeSysRoot() + "/lib");
+  } else {
+    getProgramPaths().push_back(getDriver().Dir);
+    findMultilibs(D, Triple, Args);
+    if (!SysRoot.empty()) {
+      for (const Multilib &M : getOrderedMultilibs()) {
+        SmallString<128> Dir(SysRoot);
+        llvm::sys::path::append(Dir, M.osSuffix(), "lib");
+        getFilePaths().push_back(std::string(Dir));
+        getLibraryPaths().push_back(std::string(Dir));
+      }
     }
   }
 }
@@ -236,7 +311,7 @@ getMultilibConfigPath(const Driver &D, const llvm::Triple &Triple,
       return {};
     }
   } else {
-    MultilibPath = computeBaseSysRoot(D, /*IncludeTriple=*/false);
+    MultilibPath = computeInstalledToolchainSysRoot(D, /*IncludeTriple=*/false);
     llvm::sys::path::append(MultilibPath, MultilibFilename);
   }
   return MultilibPath;
@@ -254,7 +329,7 @@ void BareMetal::findMultilibs(const Driver &D, const llvm::Triple &Triple,
   if (D.getVFS().exists(*MultilibPath)) {
     // If multilib.yaml is found, update sysroot so it doesn't use a target
     // specific suffix
-    SysRoot = computeBaseSysRoot(D, /*IncludeTriple=*/false);
+    SysRoot = computeInstalledToolchainSysRoot(D, /*IncludeTriple=*/false);
     findMultilibsFromYAML(*this, D, *MultilibPath, Args, Result);
     SelectedMultilibs = Result.SelectedMultilibs;
     Multilibs = Result.Multilibs;
@@ -279,8 +354,6 @@ Tool *BareMetal::buildStaticLibTool() const {
   return new tools::baremetal::StaticLibTool(*this);
 }
 
-std::string BareMetal::computeSysRoot() const { return SysRoot; }
-
 BareMetal::OrderedMultilibs BareMetal::getOrderedMultilibs() const {
   // Get multilibs in reverse order because they're ordered most-specific last.
   if (!SelectedMultilibs.empty())
@@ -291,6 +364,36 @@ BareMetal::OrderedMultilibs BareMetal::getOrderedMultilibs() const {
   return llvm::reverse(Default);
 }
 
+ToolChain::CXXStdlibType BareMetal::GetDefaultCXXStdlibType() const {
+  if (getTriple().isRISCV()) {
+    return GCCInstallation.isValid() ? ToolChain::CST_Libstdcxx
+                                     : ToolChain::CST_Libcxx;
+  }
+  return ToolChain::CST_Libcxx;
+}
+
+ToolChain::RuntimeLibType BareMetal::GetDefaultRuntimeLibType() const {
+  if (getTriple().isRISCV()) {
+    return GCCInstallation.isValid() ? ToolChain::RLT_Libgcc
+                                     : ToolChain::RLT_CompilerRT;
+  }
+  return ToolChain::RLT_CompilerRT;
+}
+
+ToolChain::UnwindLibType
+BareMetal::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
+  if (getTriple().isRISCV())
+    return ToolChain::UNW_None;
+
+  return ToolChain::GetUnwindLibType(Args);
+}
+
+const char *BareMetal::getDefaultLinker() const {
+  if(isUsingLD())
+    return "ld";
+  return "ld.lld";
+}
+
 void BareMetal::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
                                           ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc))
@@ -325,6 +428,19 @@ void BareMetal::addClangTargetOptions(const ArgList &DriverArgs,
   CC1Args.push_back("-nostdsysteminc");
 }
 
+void BareMetal::addLibStdCxxIncludePaths(
+    const llvm::opt::ArgList &DriverArgs,
+    llvm::opt::ArgStringList &CC1Args) const {
+  if (GCCInstallation.isValid()) {
+    const GCCVersion &Version = GCCInstallation.getVersion();
+    StringRef TripleStr = GCCInstallation.getTriple().str();
+    const Multilib &Multilib = GCCInstallation.getMultilib();
+    addLibStdCXXIncludePaths(computeSysRoot() + "/include/c++/" + Version.Text,
+                            TripleStr, Multilib.includeSuffix(), DriverArgs,
+                            CC1Args);
+  }
+}
+
 void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
                                              ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc,
@@ -355,15 +471,15 @@ void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
   };
 
   switch (GetCXXStdlibType(DriverArgs)) {
-    case ToolChain::CST_Libcxx: {
-      SmallString<128> P(D.Dir);
-      llvm::sys::path::append(P, "..", "include");
-      AddCXXIncludePath(P);
-      break;
-    }
-    case ToolChain::CST_Libstdcxx:
-      // We only support libc++ toolchain installation.
-      break;
+  case ToolChain::CST_Libcxx: {
+    SmallString<128> P(D.Dir);
+    llvm::sys::path::append(P, "..", "include");
+    AddCXXIncludePath(P);
+    break;
+  }
+  case ToolChain::CST_Libstdcxx:
+    addLibStdCxxIncludePaths(DriverArgs, CC1Args);
+    break;
   }
 
   std::string SysRoot(computeSysRoot());
@@ -428,6 +544,10 @@ void BareMetal::AddCXXStdlibLibArgs(const ArgList &Args,
     CmdArgs.push_back("-lsupc++");
     break;
   }
+
+  if (getTriple().isRISCV() && GCCInstallation.isValid())
+    return;
+
   CmdArgs.push_back("-lunwind");
 }
 
@@ -503,12 +623,22 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   const llvm::Triple::ArchType Arch = TC.getArch();
   const llvm::Triple &Triple = getToolChain().getEffectiveTriple();
 
-  AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
+  if (!D.SysRoot.empty())
+    CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
 
+  Args.addAllArgs(CmdArgs, {options::OPT_u});
   CmdArgs.push_back("-Bstatic");
 
-  if (TC.getTriple().isRISCV() && Args.hasArg(options::OPT_mno_relax))
-    CmdArgs.push_back("--no-relax");
+  if (TC.getTriple().isRISCV()) {
+    if (Args.hasArg(options::OPT_mno_relax))
+      CmdArgs.push_back("--no-relax");
+    if (TC.isUsingLD()) {
+      CmdArgs.push_back("-m");
+      CmdArgs.push_back(TC.getArch() == llvm::Triple::riscv64 ? "elf64lriscv"
+                                                              : "elf32lriscv");
+    }
+    CmdArgs.push_back("-X");
+  }
 
   if (Triple.isARM() || Triple.isThumb()) {
     bool IsBigEndian = arm::isARMBigEndian(Triple, Args);
@@ -519,9 +649,24 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back(Arch == llvm::Triple::aarch64_be ? "-EB" : "-EL");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
-                   options::OPT_r)) {
-    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crt0.o")));
+  bool WantCRTs =
+      !Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles);
+
+  const char *crtbegin, *crtend;
+  if (WantCRTs) {
+    if (!Args.hasArg(options::OPT_r))
+      CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crt0.o")));
+    auto RuntimeLib = TC.GetRuntimeLibType(Args);
+    if (RuntimeLib == ToolChain::RLT_Libgcc) {
+      crtbegin = "crtbegin.o";
+      crtend = "crtend.o";
+    } else {
+      assert(RuntimeLib == ToolChain::RLT_CompilerRT);
+      crtbegin =
+          TC.getCompilerRTArgString(Args, "crtbegin", ToolChain::FT_Object);
+      crtend = TC.getCompilerRTArgString(Args, "crtend", ToolChain::FT_Object);
+    }
+    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath(crtbegin)));
   }
 
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
@@ -536,12 +681,20 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     TC.AddCXXStdlibLibArgs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
-    CmdArgs.push_back("-lc");
     CmdArgs.push_back("-lm");
-
+    if (TC.isUsingLD())
+      CmdArgs.push_back("--start-group");
+    CmdArgs.push_back("-lc");
+    if (TC.isUsingLD()) {
+      CmdArgs.push_back("-lgloss");
+      CmdArgs.push_back("--end-group");
+    }
     TC.AddLinkRuntimeLib(Args, CmdArgs);
   }
 
+  if (WantCRTs)
+    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath(crtend)));
+
   if (D.isUsingLTO()) {
     assert(!Inputs.empty() && "Must have at least one input.");
     // Find the first filename InputInfo object.
@@ -555,8 +708,8 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     addLTOOptions(TC, Args, CmdArgs, Output, *Input,
                   D.getLTOMode() == LTOK_Thin);
   }
-  if (TC.getTriple().isRISCV())
-    CmdArgs.push_back("-X");
+
+  AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
 
   // The R_ARM_TARGET2 relocation must be treated as R_ARM_REL32 on arm*-*-elf
   // and arm*-*-eabi (the default is R_ARM_GOT_PREL, used on arm*-*-linux and
diff --git a/clang/lib/Driver/ToolChains/BareMetal.h b/clang/lib/Driver/ToolChains/BareMetal.h
index b385c8cf76aab0..4fbf6a563784a1 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.h
+++ b/clang/lib/Driver/ToolChains/BareMetal.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
 
+#include "ToolChains/Gnu.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 
@@ -19,7 +20,7 @@ namespace driver {
 
 namespace toolchains {
 
-class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
+class LLVM_LIBRARY_VISIBILITY BareMetal : public Generic_ELF {
 public:
   BareMetal(const Driver &D, const llvm::Triple &Triple,
             const llvm::opt::ArgList &Args);
@@ -35,7 +36,7 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   Tool *buildStaticLibTool() const override;
 
 public:
-  bool useIntegratedAs() const override { return true; }
+  bool isUsingLD() const { return UseLD || GCCInstallation.isValid(); }
   bool isBareMetal() const override { return true; }
   bool isCrossCompiling() const override { return true; }
   bool HasNativeLLVMSupport() const override { return true; }
@@ -48,14 +49,18 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
 
   StringRef getOSLibName() const override { return "baremetal"; }
 
-  RuntimeLibType GetDefaultRuntimeLibType() const override {
-    return ToolChain::RLT_CompilerRT;
-  }
-  CXXStdlibType GetDefaultCXXStdlibType() const override {
-    return ToolChain::CST_Libcxx;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override {
+    return UnwindTableLevel::None;
   }
 
-  const char *getDefaultLinker() const override { return "ld.lld"; }
+  CXXStdlibType GetDefaultCXXStdlibType() const override;
+
+  RuntimeLibType GetDefaultRuntimeLibType() const override;
+
+  UnwindLibType GetUnwindLibType(const llvm::opt::ArgList &Args) const override;
+
+  const char *getDefaultLinker() const override;
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
@@ -67,6 +72,9 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   void AddClangCXXStdlibIncludeArgs(
       const llvm::opt::ArgList &DriverArgs,
       llvm::opt::ArgStringList &CC1Args) const override;
+  void
+  addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
+                           llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs) const override;
   void AddLinkRuntimeLib(const llvm::opt::ArgList &Args,
@@ -78,8 +86,9 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   using OrderedMultilibs =
       llvm::iterator_range<llvm::SmallVector<Multilib>::const_reverse_iterator>;
   OrderedMultilibs getOrderedMultilibs() const;
-
+  bool UseLD;
   std::string SysRoot;
+  std::string computeGCCSysRoot() const;
 };
 
 } // namespace toolchains
@@ -103,7 +112,12 @@ class LLVM_LIBRARY_VISIBILITY StaticLibTool : public Tool {
 
 class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
 public:
-  Linker(const ToolChain &TC) : Tool("baremetal::Linker", "ld.lld", TC) {}
+  Linker(const ToolChain &TC)
+      : Tool("baremetal::Linker",
+             static_cast<const toolchains::BareMetal &>(TC).isUsingLD()
+                 ? "ld"
+                 : "ld.lld",
+             TC) {}
   bool isLinkJob() const override { return true; }
   bool hasIntegratedCPP() const override { return false; }
   void ConstructJob(Compilation &C, const JobAction &JA,
diff --git a/clang/test/Driver/arm-gnutools.c b/clang/test/Driver/arm-gnutools.c
new file mode 100644
index 00000000000000..127e40dc74da7a
--- /dev/null
+++ b/clang/test/Driver/arm-gnutools.c
@@ -0,0 +1,12 @@
+// check that gnu assembler is invoked with arm baremetal as well
+
+// RUN: %clang --target=armv6m-none-eabi  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang --target=armv7-none-eabi  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang --target=aarch64-none-elf  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: "{{.*}}as{{(.exe)?}}"
\ No newline at end of file
diff --git a/clang/test/Driver/baremetal-multilib.yaml b/clang/test/Driver/baremetal-multilib.yaml
index b6bfd0ed3a94cb..58e66ba3d9a7e7 100644
--- a/clang/test/Driver/baremetal-multilib.yaml
+++ b/clang/test/Driver/baremetal-multilib.yaml
@@ -8,9 +8,9 @@
 # CHECK-SAME: "-internal-isystem" "[[SYSROOT:[^"]*]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include/c++/v1"
 # CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include"
 # CHECK-SAME: "-x" "c++" "{{.*}}baremetal-multilib.yaml"
-# CHECK-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+# CHECK-NEXT: ld{{(.exe)?}}" "-Bstatic"
 # CHECK-SAME: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/lib"
-# CHECK-SAME: "-lc" "-lm" "{{[^"]*}}libclang_rt.builtins.a"
+# CHECK-SAME: "-lm" "-lc" "{{[^"]*}}libclang_rt.builtins.a"
 # CHECK-SAME: "-o" "{{.*}}.tmp.out"
 
 # RUN: %clang --multi-lib-config=%s -no-canonical-prefixes -x c++ %s -### -o %t.out 2>&1 \
diff --git a/clang/test/Driver/baremetal-sysroot.cpp b/clang/test/Driver/baremetal-sysroot.cpp
index 18654be33b87c9..56cf738830aadc 100644
--- a/clang/test/Driver/baremetal-sysroot.cpp
+++ b/clang/test/Driver/baremetal-sysroot.cpp
@@ -16,7 +16,7 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" "{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECk-V6M-C-SAME: "-internal-isystem" "{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include"
 // CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal-sysroot.cpp"
-// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}lib"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "{{[^"]*}}libclang_rt.builtins.a"
+// CHECK-V6M-C-SAME: "-lm" "-lc" "{{[^"]*}}libclang_rt.builtins.a"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index f09d7361e6c138..1eb69b4b49121a 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -15,11 +15,12 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSR...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Dec 5, 2024

@llvm/pr-subscribers-clang-driver

Author: Garvit Gupta (quic-garvgupt)

Changes

Currently, LLVM has two RISC-V toolchain classes in Clang for baremetal development, creating unnecessary maintenance overhead. This patch extends the BareMetal toolchain to support an existing GCC installation, resolving this issue.

The latest patchset preserves the behavior of both toolchain objects with minor differences. If no --sysroot option is passed on the command line or if the GCC installation is invalid, the sysroot will first be formed as per the RISCVToolChain baremetal object. If this path does not exist, the sysroot will be formed as per the BareMetal toolchain object.

Additionally, the presence of --gcc-toolchain or --gcc-install-dir will imply that GNU linker is the default linker unless otherwise a differnt linker is passed through -fuse-ld flag.

RFC - https://discourse.llvm.org/t/merging-riscvtoolchain-and-baremetal-toolchains/75524


Patch is 50.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/118809.diff

12 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (-4)
  • (modified) clang/lib/Driver/ToolChains/BareMetal.cpp (+189-36)
  • (modified) clang/lib/Driver/ToolChains/BareMetal.h (+24-10)
  • (added) clang/test/Driver/arm-gnutools.c (+12)
  • (modified) clang/test/Driver/baremetal-multilib.yaml (+2-2)
  • (modified) clang/test/Driver/baremetal-sysroot.cpp (+2-2)
  • (modified) clang/test/Driver/baremetal.cpp (+75-48)
  • (modified) clang/test/Driver/riscv-args.c (+1-1)
  • (modified) clang/test/Driver/riscv32-toolchain-extra.c (+2-2)
  • (modified) clang/test/Driver/riscv32-toolchain.c (+3-3)
  • (modified) clang/test/Driver/riscv64-toolchain-extra.c (+2-2)
  • (modified) clang/test/Driver/riscv64-toolchain.c (+2-2)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 7de8341b8d2d61..c5185ccedd6201 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -6521,10 +6521,6 @@ const ToolChain &Driver::getToolChain(const ArgList &Args,
         break;
       case llvm::Triple::riscv32:
       case llvm::Triple::riscv64:
-        if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args))
-          TC =
-              std::make_unique<toolchains::RISCVToolChain>(*this, Target, Args);
-        else
           TC = std::make_unique<toolchains::BareMetal>(*this, Target, Args);
         break;
       case llvm::Triple::ve:
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index f9a73f60973e4c..1d065562e9a6ef 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -97,7 +97,8 @@ static bool findRISCVMultilibs(const Driver &D,
   return false;
 }
 
-static std::string computeBaseSysRoot(const Driver &D, bool IncludeTriple) {
+static std::string computeInstalledToolchainSysRoot(const Driver &D,
+                                                    bool IncludeTriple) {
   if (!D.SysRoot.empty())
     return D.SysRoot;
 
@@ -110,20 +111,94 @@ static std::string computeBaseSysRoot(const Driver &D, bool IncludeTriple) {
   return std::string(SysRootDir);
 }
 
+// GCC sysroot here means form sysroot from either --gcc-install-dir, or from
+// --gcc-toolchain or if the toolchain is installed alongside clang in
+// bin/../<TargetTriple> directory if it is not explicitly specified on the command
+//  line through `--sysroot` option. libc here will be newlib.
+std::string BareMetal::computeGCCSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+    return getDriver().SysRoot;
+
+  SmallString<128> SysRootDir;
+  if (GCCInstallation.isValid()) {
+    StringRef LibDir = GCCInstallation.getParentLibPath();
+    StringRef TripleStr = GCCInstallation.getTriple().str();
+    llvm::sys::path::append(SysRootDir, LibDir, "..", TripleStr);
+  } else {
+    // Use the triple as provided to the driver. Unlike the parsed triple
+    // this has not been normalized to always contain every field.
+    llvm::sys::path::append(SysRootDir, getDriver().Dir, "..",
+                            getDriver().getTargetTriple());
+  }
+
+  if (!llvm::sys::fs::exists(SysRootDir))
+    return std::string();
+
+  return std::string(SysRootDir);
+}
+
+std::string BareMetal::computeSysRoot() const {
+  if (!SysRoot.empty())
+    return SysRoot;
+
+  std::string SysRoot = getDriver().SysRoot;
+  if (!SysRoot.empty() && llvm::sys::fs::exists(SysRoot))
+    return SysRoot;
+
+  // Verify the GCC installation from -gcc-install-dir, --gcc-toolchain, or
+  // alongside clang. If valid, form the sysroot. Otherwise, check
+  // lib/clang-runtimes above the driver.
+  SysRoot = computeGCCSysRoot();
+  if (!SysRoot.empty())
+    return SysRoot;
+
+  SysRoot =
+      computeInstalledToolchainSysRoot(getDriver(), /*IncludeTriple*/ true);
+
+  return SysRoot;
+}
+
+static void addMultilibsFilePaths(const Driver &D, const MultilibSet &Multilibs,
+                                  const Multilib &Multilib,
+                                  StringRef InstallPath,
+                                  ToolChain::path_list &Paths) {
+  if (const auto &PathsCallback = Multilibs.filePathsCallback())
+    for (const auto &Path : PathsCallback(Multilib))
+      addPathIfExists(D, InstallPath + Path, Paths);
+}
+
 BareMetal::BareMetal(const Driver &D, const llvm::Triple &Triple,
                      const ArgList &Args)
-    : ToolChain(D, Triple, Args),
-      SysRoot(computeBaseSysRoot(D, /*IncludeTriple=*/true)) {
-  getProgramPaths().push_back(getDriver().Dir);
-
-  findMultilibs(D, Triple, Args);
-  SmallString<128> SysRoot(computeSysRoot());
-  if (!SysRoot.empty()) {
-    for (const Multilib &M : getOrderedMultilibs()) {
-      SmallString<128> Dir(SysRoot);
-      llvm::sys::path::append(Dir, M.osSuffix(), "lib");
-      getFilePaths().push_back(std::string(Dir));
-      getLibraryPaths().push_back(std::string(Dir));
+    : Generic_ELF(D, Triple, Args){
+  GCCInstallation.init(Triple, Args);
+  SysRoot = computeSysRoot();
+  UseLD = Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_insensitive("ld");
+  if (GCCInstallation.isValid()) {
+    Multilibs = GCCInstallation.getMultilibs();
+    SelectedMultilibs.assign({GCCInstallation.getMultilib()});
+    path_list &Paths = getFilePaths();
+    // Add toolchain/multilib specific file paths.
+    addMultilibsFilePaths(D, Multilibs, SelectedMultilibs.back(),
+                          GCCInstallation.getInstallPath(), Paths);
+    getFilePaths().push_back(GCCInstallation.getInstallPath().str());
+    ToolChain::path_list &PPaths = getProgramPaths();
+    // Multilib cross-compiler GCC installations put ld in a triple-prefixed
+    // directory off of the parent of the GCC installation.
+    PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
+                           GCCInstallation.getTriple().str() + "/bin")
+                         .str());
+    PPaths.push_back((GCCInstallation.getParentLibPath() + "/../bin").str());
+    getFilePaths().push_back(computeSysRoot() + "/lib");
+  } else {
+    getProgramPaths().push_back(getDriver().Dir);
+    findMultilibs(D, Triple, Args);
+    if (!SysRoot.empty()) {
+      for (const Multilib &M : getOrderedMultilibs()) {
+        SmallString<128> Dir(SysRoot);
+        llvm::sys::path::append(Dir, M.osSuffix(), "lib");
+        getFilePaths().push_back(std::string(Dir));
+        getLibraryPaths().push_back(std::string(Dir));
+      }
     }
   }
 }
@@ -236,7 +311,7 @@ getMultilibConfigPath(const Driver &D, const llvm::Triple &Triple,
       return {};
     }
   } else {
-    MultilibPath = computeBaseSysRoot(D, /*IncludeTriple=*/false);
+    MultilibPath = computeInstalledToolchainSysRoot(D, /*IncludeTriple=*/false);
     llvm::sys::path::append(MultilibPath, MultilibFilename);
   }
   return MultilibPath;
@@ -254,7 +329,7 @@ void BareMetal::findMultilibs(const Driver &D, const llvm::Triple &Triple,
   if (D.getVFS().exists(*MultilibPath)) {
     // If multilib.yaml is found, update sysroot so it doesn't use a target
     // specific suffix
-    SysRoot = computeBaseSysRoot(D, /*IncludeTriple=*/false);
+    SysRoot = computeInstalledToolchainSysRoot(D, /*IncludeTriple=*/false);
     findMultilibsFromYAML(*this, D, *MultilibPath, Args, Result);
     SelectedMultilibs = Result.SelectedMultilibs;
     Multilibs = Result.Multilibs;
@@ -279,8 +354,6 @@ Tool *BareMetal::buildStaticLibTool() const {
   return new tools::baremetal::StaticLibTool(*this);
 }
 
-std::string BareMetal::computeSysRoot() const { return SysRoot; }
-
 BareMetal::OrderedMultilibs BareMetal::getOrderedMultilibs() const {
   // Get multilibs in reverse order because they're ordered most-specific last.
   if (!SelectedMultilibs.empty())
@@ -291,6 +364,36 @@ BareMetal::OrderedMultilibs BareMetal::getOrderedMultilibs() const {
   return llvm::reverse(Default);
 }
 
+ToolChain::CXXStdlibType BareMetal::GetDefaultCXXStdlibType() const {
+  if (getTriple().isRISCV()) {
+    return GCCInstallation.isValid() ? ToolChain::CST_Libstdcxx
+                                     : ToolChain::CST_Libcxx;
+  }
+  return ToolChain::CST_Libcxx;
+}
+
+ToolChain::RuntimeLibType BareMetal::GetDefaultRuntimeLibType() const {
+  if (getTriple().isRISCV()) {
+    return GCCInstallation.isValid() ? ToolChain::RLT_Libgcc
+                                     : ToolChain::RLT_CompilerRT;
+  }
+  return ToolChain::RLT_CompilerRT;
+}
+
+ToolChain::UnwindLibType
+BareMetal::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
+  if (getTriple().isRISCV())
+    return ToolChain::UNW_None;
+
+  return ToolChain::GetUnwindLibType(Args);
+}
+
+const char *BareMetal::getDefaultLinker() const {
+  if(isUsingLD())
+    return "ld";
+  return "ld.lld";
+}
+
 void BareMetal::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
                                           ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc))
@@ -325,6 +428,19 @@ void BareMetal::addClangTargetOptions(const ArgList &DriverArgs,
   CC1Args.push_back("-nostdsysteminc");
 }
 
+void BareMetal::addLibStdCxxIncludePaths(
+    const llvm::opt::ArgList &DriverArgs,
+    llvm::opt::ArgStringList &CC1Args) const {
+  if (GCCInstallation.isValid()) {
+    const GCCVersion &Version = GCCInstallation.getVersion();
+    StringRef TripleStr = GCCInstallation.getTriple().str();
+    const Multilib &Multilib = GCCInstallation.getMultilib();
+    addLibStdCXXIncludePaths(computeSysRoot() + "/include/c++/" + Version.Text,
+                            TripleStr, Multilib.includeSuffix(), DriverArgs,
+                            CC1Args);
+  }
+}
+
 void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
                                              ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc,
@@ -355,15 +471,15 @@ void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
   };
 
   switch (GetCXXStdlibType(DriverArgs)) {
-    case ToolChain::CST_Libcxx: {
-      SmallString<128> P(D.Dir);
-      llvm::sys::path::append(P, "..", "include");
-      AddCXXIncludePath(P);
-      break;
-    }
-    case ToolChain::CST_Libstdcxx:
-      // We only support libc++ toolchain installation.
-      break;
+  case ToolChain::CST_Libcxx: {
+    SmallString<128> P(D.Dir);
+    llvm::sys::path::append(P, "..", "include");
+    AddCXXIncludePath(P);
+    break;
+  }
+  case ToolChain::CST_Libstdcxx:
+    addLibStdCxxIncludePaths(DriverArgs, CC1Args);
+    break;
   }
 
   std::string SysRoot(computeSysRoot());
@@ -428,6 +544,10 @@ void BareMetal::AddCXXStdlibLibArgs(const ArgList &Args,
     CmdArgs.push_back("-lsupc++");
     break;
   }
+
+  if (getTriple().isRISCV() && GCCInstallation.isValid())
+    return;
+
   CmdArgs.push_back("-lunwind");
 }
 
@@ -503,12 +623,22 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   const llvm::Triple::ArchType Arch = TC.getArch();
   const llvm::Triple &Triple = getToolChain().getEffectiveTriple();
 
-  AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
+  if (!D.SysRoot.empty())
+    CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
 
+  Args.addAllArgs(CmdArgs, {options::OPT_u});
   CmdArgs.push_back("-Bstatic");
 
-  if (TC.getTriple().isRISCV() && Args.hasArg(options::OPT_mno_relax))
-    CmdArgs.push_back("--no-relax");
+  if (TC.getTriple().isRISCV()) {
+    if (Args.hasArg(options::OPT_mno_relax))
+      CmdArgs.push_back("--no-relax");
+    if (TC.isUsingLD()) {
+      CmdArgs.push_back("-m");
+      CmdArgs.push_back(TC.getArch() == llvm::Triple::riscv64 ? "elf64lriscv"
+                                                              : "elf32lriscv");
+    }
+    CmdArgs.push_back("-X");
+  }
 
   if (Triple.isARM() || Triple.isThumb()) {
     bool IsBigEndian = arm::isARMBigEndian(Triple, Args);
@@ -519,9 +649,24 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back(Arch == llvm::Triple::aarch64_be ? "-EB" : "-EL");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
-                   options::OPT_r)) {
-    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crt0.o")));
+  bool WantCRTs =
+      !Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles);
+
+  const char *crtbegin, *crtend;
+  if (WantCRTs) {
+    if (!Args.hasArg(options::OPT_r))
+      CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crt0.o")));
+    auto RuntimeLib = TC.GetRuntimeLibType(Args);
+    if (RuntimeLib == ToolChain::RLT_Libgcc) {
+      crtbegin = "crtbegin.o";
+      crtend = "crtend.o";
+    } else {
+      assert(RuntimeLib == ToolChain::RLT_CompilerRT);
+      crtbegin =
+          TC.getCompilerRTArgString(Args, "crtbegin", ToolChain::FT_Object);
+      crtend = TC.getCompilerRTArgString(Args, "crtend", ToolChain::FT_Object);
+    }
+    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath(crtbegin)));
   }
 
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
@@ -536,12 +681,20 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     TC.AddCXXStdlibLibArgs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
-    CmdArgs.push_back("-lc");
     CmdArgs.push_back("-lm");
-
+    if (TC.isUsingLD())
+      CmdArgs.push_back("--start-group");
+    CmdArgs.push_back("-lc");
+    if (TC.isUsingLD()) {
+      CmdArgs.push_back("-lgloss");
+      CmdArgs.push_back("--end-group");
+    }
     TC.AddLinkRuntimeLib(Args, CmdArgs);
   }
 
+  if (WantCRTs)
+    CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath(crtend)));
+
   if (D.isUsingLTO()) {
     assert(!Inputs.empty() && "Must have at least one input.");
     // Find the first filename InputInfo object.
@@ -555,8 +708,8 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     addLTOOptions(TC, Args, CmdArgs, Output, *Input,
                   D.getLTOMode() == LTOK_Thin);
   }
-  if (TC.getTriple().isRISCV())
-    CmdArgs.push_back("-X");
+
+  AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
 
   // The R_ARM_TARGET2 relocation must be treated as R_ARM_REL32 on arm*-*-elf
   // and arm*-*-eabi (the default is R_ARM_GOT_PREL, used on arm*-*-linux and
diff --git a/clang/lib/Driver/ToolChains/BareMetal.h b/clang/lib/Driver/ToolChains/BareMetal.h
index b385c8cf76aab0..4fbf6a563784a1 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.h
+++ b/clang/lib/Driver/ToolChains/BareMetal.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
 
+#include "ToolChains/Gnu.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 
@@ -19,7 +20,7 @@ namespace driver {
 
 namespace toolchains {
 
-class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
+class LLVM_LIBRARY_VISIBILITY BareMetal : public Generic_ELF {
 public:
   BareMetal(const Driver &D, const llvm::Triple &Triple,
             const llvm::opt::ArgList &Args);
@@ -35,7 +36,7 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   Tool *buildStaticLibTool() const override;
 
 public:
-  bool useIntegratedAs() const override { return true; }
+  bool isUsingLD() const { return UseLD || GCCInstallation.isValid(); }
   bool isBareMetal() const override { return true; }
   bool isCrossCompiling() const override { return true; }
   bool HasNativeLLVMSupport() const override { return true; }
@@ -48,14 +49,18 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
 
   StringRef getOSLibName() const override { return "baremetal"; }
 
-  RuntimeLibType GetDefaultRuntimeLibType() const override {
-    return ToolChain::RLT_CompilerRT;
-  }
-  CXXStdlibType GetDefaultCXXStdlibType() const override {
-    return ToolChain::CST_Libcxx;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override {
+    return UnwindTableLevel::None;
   }
 
-  const char *getDefaultLinker() const override { return "ld.lld"; }
+  CXXStdlibType GetDefaultCXXStdlibType() const override;
+
+  RuntimeLibType GetDefaultRuntimeLibType() const override;
+
+  UnwindLibType GetUnwindLibType(const llvm::opt::ArgList &Args) const override;
+
+  const char *getDefaultLinker() const override;
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
@@ -67,6 +72,9 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   void AddClangCXXStdlibIncludeArgs(
       const llvm::opt::ArgList &DriverArgs,
       llvm::opt::ArgStringList &CC1Args) const override;
+  void
+  addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
+                           llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs) const override;
   void AddLinkRuntimeLib(const llvm::opt::ArgList &Args,
@@ -78,8 +86,9 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
   using OrderedMultilibs =
       llvm::iterator_range<llvm::SmallVector<Multilib>::const_reverse_iterator>;
   OrderedMultilibs getOrderedMultilibs() const;
-
+  bool UseLD;
   std::string SysRoot;
+  std::string computeGCCSysRoot() const;
 };
 
 } // namespace toolchains
@@ -103,7 +112,12 @@ class LLVM_LIBRARY_VISIBILITY StaticLibTool : public Tool {
 
 class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
 public:
-  Linker(const ToolChain &TC) : Tool("baremetal::Linker", "ld.lld", TC) {}
+  Linker(const ToolChain &TC)
+      : Tool("baremetal::Linker",
+             static_cast<const toolchains::BareMetal &>(TC).isUsingLD()
+                 ? "ld"
+                 : "ld.lld",
+             TC) {}
   bool isLinkJob() const override { return true; }
   bool hasIntegratedCPP() const override { return false; }
   void ConstructJob(Compilation &C, const JobAction &JA,
diff --git a/clang/test/Driver/arm-gnutools.c b/clang/test/Driver/arm-gnutools.c
new file mode 100644
index 00000000000000..127e40dc74da7a
--- /dev/null
+++ b/clang/test/Driver/arm-gnutools.c
@@ -0,0 +1,12 @@
+// check that gnu assembler is invoked with arm baremetal as well
+
+// RUN: %clang --target=armv6m-none-eabi  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang --target=armv7-none-eabi  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang --target=aarch64-none-elf  --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: "{{.*}}as{{(.exe)?}}"
\ No newline at end of file
diff --git a/clang/test/Driver/baremetal-multilib.yaml b/clang/test/Driver/baremetal-multilib.yaml
index b6bfd0ed3a94cb..58e66ba3d9a7e7 100644
--- a/clang/test/Driver/baremetal-multilib.yaml
+++ b/clang/test/Driver/baremetal-multilib.yaml
@@ -8,9 +8,9 @@
 # CHECK-SAME: "-internal-isystem" "[[SYSROOT:[^"]*]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include/c++/v1"
 # CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include"
 # CHECK-SAME: "-x" "c++" "{{.*}}baremetal-multilib.yaml"
-# CHECK-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+# CHECK-NEXT: ld{{(.exe)?}}" "-Bstatic"
 # CHECK-SAME: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/lib"
-# CHECK-SAME: "-lc" "-lm" "{{[^"]*}}libclang_rt.builtins.a"
+# CHECK-SAME: "-lm" "-lc" "{{[^"]*}}libclang_rt.builtins.a"
 # CHECK-SAME: "-o" "{{.*}}.tmp.out"
 
 # RUN: %clang --multi-lib-config=%s -no-canonical-prefixes -x c++ %s -### -o %t.out 2>&1 \
diff --git a/clang/test/Driver/baremetal-sysroot.cpp b/clang/test/Driver/baremetal-sysroot.cpp
index 18654be33b87c9..56cf738830aadc 100644
--- a/clang/test/Driver/baremetal-sysroot.cpp
+++ b/clang/test/Driver/baremetal-sysroot.cpp
@@ -16,7 +16,7 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" "{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECk-V6M-C-SAME: "-internal-isystem" "{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include"
 // CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal-sysroot.cpp"
-// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}lib"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "{{[^"]*}}libclang_rt.builtins.a"
+// CHECK-V6M-C-SAME: "-lm" "-lc" "{{[^"]*}}libclang_rt.builtins.a"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index f09d7361e6c138..1eb69b4b49121a 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -15,11 +15,12 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSR...
[truncated]

@quic-garvgupt quic-garvgupt requested a review from lenary December 5, 2024 14:10
Copy link

github-actions bot commented Dec 5, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

GCCInstallation.init(Triple, Args);
SysRoot = computeSysRoot();
UseLD =
Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_insensitive("ld");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the right hand side value if there are no -fuse-ld= in the argslist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value will be "0". The function to decide the default linker is "getDefaultLinker" which calls "isUsingLd". I have made this function virtual which can be overridden in derived classes and can return false for all cases.

@petrhosek
Copy link
Member

The BareMetal driver currently isn't very idiomatic (compared to other drivers like Generic_GCC) and duplicates a lot of the logic that has been already factored out in other drivers. I think this change could be made a lot smaller by doing a clean up first, bringing the BareMetal driver closer to other drivers, before attempting to merge the RISCVToolChain driver into it. I already landed #101259 which is a first step in that direction and I plan to make a few more follow up cleanup changes.

@efriedma-quic
Copy link
Collaborator

I think this change could be made a lot smaller by doing a clean up first, bringing the BareMetal driver closer to other drivers, before attempting to merge the RISCVToolChain driver into it.

Could you describe in a bit more detail which bits you think need to be cleaned up? Or are you planning to push additional changes yourself in the near future? This is a little vague. (We don't want this to be blocked indefinitely.)

Currently, LLVM has two RISC-V toolchain classes in Clang for baremetal
development, creating unnecessary maintenance overhead. This patch extends the
BareMetal toolchain to support an existing GCC installation, resolving this issue.

The latest patchset preserves the behavior of both toolchain objects with minor
differences. If no --sysroot option is passed on the command line or if the GCC
installation is invalid, the sysroot will first be formed as per the
RISCVToolChain baremetal object. If this path does not exist, the sysroot will
be formed as per the BareMetal toolchain object.

Additionally, the presence of --gcc-toolchain or --gcc-install-dir will imply
that GNU linker is the default linker unless otherwise a differnt linker is
passed through `-fuse-ld` flag.

RFC - https://discourse.llvm.org/t/merging-riscvtoolchain-and-baremetal-toolchains/75524

change-Id: Ie2cdefd3c95b25770a33319ce2e711c9300efc2e
@kito-cheng
Copy link
Member

I would suggest to break this PR into several small pieces, the clang/test folder should not having too much change during the merging, especially I feel not conformable changing the non-RISC-V file within this PR, I expect those change should happened in a separated patch.

@petrhosek
Copy link
Member

I think this change could be made a lot smaller by doing a clean up first, bringing the BareMetal driver closer to other drivers, before attempting to merge the RISCVToolChain driver into it.

Could you describe in a bit more detail which bits you think need to be cleaned up? Or are you planning to push additional changes yourself in the near future? This is a little vague. (We don't want this to be blocked indefinitely.)

Two areas I've specifically been looking into are BareMetal::Linker::ConstructJob and BareMetal::AddClangCXXStdlibIncludeArgs. I have local changes for both but they need more testing.

@quic-garvgupt
Copy link
Contributor Author

quic-garvgupt commented Dec 19, 2024

I would suggest to break this PR into several small pieces, the clang/test folder should not having too much change during the merging, especially I feel not conformable changing the non-RISC-V file within this PR, I expect those change should happened in a separated patch.

@kito-cheng I can divide this patch into smaller patchsets and modify ARM-specific tests in a separate commit. I propose the following division for the PR:

  1. Sysroot-related changes in the first patch.
  2. Changes related to the compile line, such as include paths and defaults for CXXstdlibtype, Runtimelibtype, and Unwindlibtype, in the second patch.
  3. Modifications to the linker job in the final patch.

Please let me know if this division looks good to you, and I will proceed accordingly.

@quic-garvgupt
Copy link
Contributor Author

quic-garvgupt commented Jan 9, 2025

I have divided this PR into three separate PRs to streamline the review process and ensured that ARM and RISCV tests are not modified in the same PR. Please review them and provide any feedback or comments. Thanks!

[RISCV] Teach Barmetal toolchain about GCC installation(1/3) - #121829
[RISCV] Change linker job in Baremetal toolchain object accomodate GCCInstallation.(2/3) - #121830
[RISCV] Integrate RISCV target in baremetal toolchain object and deprecate RISCVToolchain object.(3/3) - #121831

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants