Driver: Refactor and support per target dirs in baremetal

[manojgupta]

Summary

Refactor baremetal driver code to reduce the bespoke
additions and base class overrides.
This lets us use the per target runtimes like other clang
targets. E.g. clang -target armv7m-cros-none-eabi will now
be able to use the runtimes installed at
<resource_dir>/lib/armv7m-cros-none-eabi instead of the hardcoded
path <resource_dir>/lib/baremetal.
The older code paths should still continue to work as before if
<resource_dir>/lib/<tuple> does not exist.

[abidh]

I think this patch is moving the things in the right direction. But please wait for some other reviewers to chime in.

[barannikov88]

In D131225#3706207, @abidh wrote:

I think this patch is moving the things in the right direction. But please wait for some other reviewers to chime in.

It seems quite opposite to me.

clang/include/clang/Driver/ToolChain.h

384

Is this a correct sentence? (My English is poor.)

388

The ToolChain class is an interface class. It is strange to see such kind of methods here. IsBareMetal should at least be virtual and overridden in concrete implementation of baremetal toolchains. IsRISCVBareMetal should not be here at all.
What was wrong with the previous implementation that made you move the methods here?

[abidh]

In D131225#3706239, @barannikov88 wrote:

In D131225#3706207, @abidh wrote:

I think this patch is moving the things in the right direction. But please wait for some other reviewers to chime in.

It seems quite opposite to me.

To be clear, my comment was aimed at using target specific directory instead of using lib/baremetal.

[manojgupta]

clang/include/clang/Driver/ToolChain.h

384

I'll fix it. Somehow I forgot to check them.

388

There is a need to check for IsBareMetal and variants. They can't be made virtual since the checks need to happen before instantiating ToolChain class. I think moving them to Triple class (Triple.h) is a clearer option.

[barannikov88]

clang/include/clang/Driver/ToolChain.h

388

I think moving them to Triple class (Triple.h) is a clearer option.

Is the triple is all that is necessary to decide whether the target is bare metal or not?
Sounds interesting, but one may argue that Triple should not know about toolchains (like it should not know about C data type bit widths, for example).
What if just add a few switch-cases to Driver::getToolChain as for every other toolchain? Retaining the former static method 'BareMetalToolChain::handlesTarget' is still better in my opinion.

[manojgupta]

Moved Baremetal triple related code to Triple.h.

[barannikov88]

clang/include/clang/Driver/ToolChain.h

388

They can't be made virtual since the checks need to happen before instantiating ToolChain class.

This is kind of two different things. It can be made virtual. The name of the method IsBareMetal of the ToolChain class suggests that it is checking whether this concrete instance of the ToolChain is baremetal. This (virtual) method could be used in getCompilerRTPath, for example.
To instantiate BareMetalToolchain you need another function (like the former BareMetalToolChain::handlesTarget, or the suggested approach with Triple). For these instantiations IsBareMetal will return true, and false for all other instantiations which are not bare metal.

[manojgupta]

clang/include/clang/Driver/ToolChain.h

388

Is the triple is all that is necessary to decide whether the target is bare metal or not?

At least for baremetal, from what I see, triple seems to be the way how baremetal toolchain is decided in practice.

Examples:

1. https://gitweb.gentoo.org/proj/crossdev.git/tree/crossdev#n331 ( # Bare metal targets *-newlib|*-elf|*-eabi|*-rtems*)
2. https://elinux.org/images/1/15/Anatomy_of_Cross-Compilation_Toolchains.pdf ( arm-foo-none-eabi, bare-metal toolchain targeting the ARM architecture, from

vendor foo )

[barannikov88]

llvm/include/llvm/ADT/Triple.h

916 ↗

(On Diff #450894)

Any future change to these methods will force recompilation of the whole project. It is better to move the definitions into the corresponding cpp file.

[manojgupta]

Moved defs to Triple.cpp

[barannikov88]

LGTM with the style issues fixed, but I'd like someone else to take a look.

clang/lib/Driver/ToolChains/BareMetal.cpp

279–287

There are a few style issues.

• return is usually within the braces
• Variable names should start with capital letter
• Looks like std::string could be avoided completely in favor of StringRef and Twine

Same applies to ConstructJob below, there are also redundant braces in for statement.

[barannikov88]

llvm/lib/Support/Triple.cpp

1936 ↗

(On Diff #450898)

(nit) at least llvm:: qualification is redundant, Triple:: might also be.

[manojgupta]

Address style nits.

[barannikov88]

In D131225#3707735, @manojgupta wrote:

Address style nits.

Neat, thank you!

[manojgupta]

Address more style lints.

[manojgupta]

clang/lib/Driver/ToolChains/BareMetal.cpp

279–287

Addressed most.
std::string is needed initially to avoid dangling storage issue:

warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]

[MaskRay]

llvm/lib/Support/Triple.cpp

1938 ↗

(On Diff #450910)

The prevailing style doesn't add a blank line for every if statement.

1980 ↗

(On Diff #450910)

Add a blank line before the comment.

[manojgupta]

Address maskray comments.

[manojgupta]

Fix clang-format complains.

[manojgupta]

Going back to older handlesTarget() style in Baremetal.
Apparently RISCV can be both Baremetal or RISCVToolchain for the
same tuple. I do not know of the nuances so trying not to disturb that for
now.

[barannikov88]

Still LGTM

clang/lib/Driver/ToolChains/BareMetal.h

2

Line overflowed

clang/include/clang/Driver/ToolChain.h

public:
   /// HasNativeLTOLinker - Check whether the linker and related tools have
   /// native LLVM support.
   virtual bool HasNativeLLVMSupport() const;
 
   /// LookupTypeForExtension - Return the default language type to use for the
   /// given extension.
   virtual types::ID LookupTypeForExtension(StringRef Ext) const;
 
   /// IsBlocksDefault - Does this tool chain enable -fblocks by default.
   virtual bool IsBlocksDefault() const { return false; }
 
   /// IsIntegratedAssemblerDefault - Does this tool chain enable -integrated-as
   /// by default.
   virtual bool IsIntegratedAssemblerDefault() const { return false; }
 
   /// IsIntegratedBackendDefault - Does this tool chain enable
   /// -fintegrated-objemitter by default.
   virtual bool IsIntegratedBackendDefault() const { return true; }
 
   /// IsIntegratedBackendSupported - Does this tool chain support
   /// -fintegrated-objemitter.
public:
   }
 
   /// UseDwarfDebugFlags - Embed the compile options to clang into the Dwarf
   /// compile unit information.
   virtual bool UseDwarfDebugFlags() const { return false; }
 
   /// Add an additional -fdebug-prefix-map entry.
   virtual std::string GetGlobalDebugPathRemapping() const { return {}; }
-  
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
   virtual unsigned GetDefaultDwarfVersion() const { return 5; }
 
   // Some toolchains may have different restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when host
   // compilation uses DWARF5.
   virtual unsigned getMaxDwarfVersion() const { return UINT_MAX; }
public:
   virtual bool SupportsEmbeddedBitcode() const { return false; }
 
   /// getThreadModel() - Which thread model does this target use?
   virtual std::string getThreadModel() const { return "posix"; }
 
   /// isThreadModelSupported() - Does this target support a thread model?
   virtual bool isThreadModelSupported(const StringRef Model) const;
 
+  /// isBareMetal - Is this a bare metal target.
+  virtual bool isBareMetal() const { return false; }
+
   virtual std::string getMultiarchTriple(const Driver &D,
                                          const llvm::Triple &TargetTriple,
                                          StringRef SysRoot) const {
     return TargetTriple.str();
   }
 
   /// ComputeLLVMTriple - Return the LLVM target triple to use, after taking
   /// command line arguments into account.

clang/lib/Driver/ToolChain.cpp

Tool *ToolChain::getTool(Action::ActionClass AC) const {
   llvm_unreachable("Invalid tool kind.");
 }
 
 static StringRef getArchNameForCompilerRTLib(const ToolChain &TC,
                                              const ArgList &Args) {
   const llvm::Triple &Triple = TC.getTriple();
   bool IsWindows = Triple.isOSWindows();
 
+  if (TC.isBareMetal())
+    return Triple.getArchName();
+
   if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb)
     return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows)
                ? "armhf"
                : "arm";
 
   // For historic reasons, Android library is using i686 instead of i386.
   if (TC.getArch() == llvm::Triple::x86 && Triple.isAndroid())
     return "i686";
case llvm::Triple::AIX:
     return "aix";
   default:
     return getOS();
   }
 }
 
 std::string ToolChain::getCompilerRTPath() const {
   SmallString<128> Path(getDriver().ResourceDir);
-  if (Triple.isOSUnknown()) {
+  if (isBareMetal()) {
+    llvm::sys::path::append(Path, "lib", getOSLibName());
+    Path += SelectedMultilib.gccSuffix();
+  } else if (Triple.isOSUnknown()) {
     llvm::sys::path::append(Path, "lib");
   } else {
     llvm::sys::path::append(Path, "lib", getOSLibName());
   }
   return std::string(Path.str());
 }
 
 std::string ToolChain::getCompilerRTBasename(const ArgList &Args,

clang/lib/Driver/ToolChains/BareMetal.h

-//===--- BareMetal.h - Bare Metal Tool and ToolChain -------------*- C++ -*-===//
+//===--- BareMetal.h - Bare Metal Tool and ToolChain ------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_BAREMETAL_H
public:
   static bool handlesTarget(const llvm::Triple &Triple);
 
   void findMultilibs(const Driver &D, const llvm::Triple &Triple,
                      const llvm::opt::ArgList &Args);
 
 protected:
   Tool *buildLinker() const override;
 
-  std::string buildCompilerRTBasename(const llvm::opt::ArgList &Args,
-                                      StringRef Component,
-                                      FileType Type = ToolChain::FT_Static,
-                                      bool AddArch = true) const override;
-
 public:
   bool useIntegratedAs() const override { return true; }
+  bool isBareMetal() const override { return true; }
   bool isCrossCompiling() const override { return true; }
   bool isPICDefault() const override { return false; }
   bool isPIEDefault(const llvm::opt::ArgList &Args) const override {
     return false;
   }
   bool isPICDefaultForced() const override { return false; }
   bool SupportsProfiling() const override { return false; }
 
   StringRef getOSLibName() const override { return "baremetal"; }
 
-  std::string getCompilerRTPath() const override;
-
   RuntimeLibType GetDefaultRuntimeLibType() const override {
     return ToolChain::RLT_CompilerRT;
   }
   CXXStdlibType GetDefaultCXXStdlibType() const override {
     return ToolChain::CST_Libcxx;
   }
 
   const char *getDefaultLinker() const override { return "ld.lld"; }
 
-  std::string getRuntimesDir() const;
-  void AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-                                 llvm::opt::ArgStringList &CC1Args) const override;
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
-                             llvm::opt::ArgStringList &CC1Args,
-                             Action::OffloadKind DeviceOffloadKind) const override;
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
   void AddClangCXXStdlibIncludeArgs(
       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,
                          llvm::opt::ArgStringList &CmdArgs) const;
   std::string computeSysRoot() const override;

clang/lib/Driver/ToolChains/BareMetal.cpp

if (TargetTriple.getArch() == llvm::Triple::riscv32) {
 
     Result.Multilibs = MultilibSet().Either(I, Im, Iac, Imac, Imafc);
     return Result.Multilibs.select(Flags, Result.SelectedMultilib);
   }
   return false;
 }
 
 BareMetal::BareMetal(const Driver &D, const llvm::Triple &Triple,
-                           const ArgList &Args)
     : ToolChain(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
   if (getDriver().getInstalledDir() != getDriver().Dir)
     getProgramPaths().push_back(getDriver().Dir);
 
   findMultilibs(D, Triple, Args);
   SmallString<128> SysRoot(computeSysRoot());
   if (!SysRoot.empty()) {
bool BareMetal::handlesTarget(const llvm::Triple &Triple) {
   return isARMBareMetal(Triple) || isAArch64BareMetal(Triple) ||
          isRISCVBareMetal(Triple);
 }
 
 Tool *BareMetal::buildLinker() const {
   return new tools::baremetal::Linker(*this);
 }
 
-std::string BareMetal::getCompilerRTPath() const { return getRuntimesDir(); }
-
-std::string BareMetal::buildCompilerRTBasename(const llvm::opt::ArgList &,
-                                               StringRef, FileType,
-                                               bool) const {
-  return ("libclang_rt.builtins-" + getTriple().getArchName() + ".a").str();
-}
-
-std::string BareMetal::getRuntimesDir() const {
-  SmallString<128> Dir(getDriver().ResourceDir);
-  llvm::sys::path::append(Dir, "lib", "baremetal");
-  Dir += SelectedMultilib.gccSuffix();
-  return std::string(Dir.str());
-}
-
 std::string BareMetal::computeSysRoot() const {
   if (!getDriver().SysRoot.empty())
     return getDriver().SysRoot + SelectedMultilib.osSuffix();
 
   SmallString<128> SysRootDir;
   llvm::sys::path::append(SysRootDir, getDriver().Dir, "../lib/clang-runtimes",
                           getDriver().getTargetTriple());
 

 }
 
 void BareMetal::addClangTargetOptions(const ArgList &DriverArgs,
                                       ArgStringList &CC1Args,
                                       Action::OffloadKind) const {
   CC1Args.push_back("-nostdsysteminc");
 }
 
-void BareMetal::AddClangCXXStdlibIncludeArgs(
-    const ArgList &DriverArgs, ArgStringList &CC1Args) const {
+void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
+                                             ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc) ||
       DriverArgs.hasArg(options::OPT_nostdlibinc) ||
       DriverArgs.hasArg(options::OPT_nostdincxx))
     return;
 
   std::string SysRoot(computeSysRoot());
   if (SysRoot.empty())
     return;
case ToolChain::CST_Libstdcxx:
     break;
   }
   CmdArgs.push_back("-lunwind");
 }
 
 void BareMetal::AddLinkRuntimeLib(const ArgList &Args,
                                   ArgStringList &CmdArgs) const {
   ToolChain::RuntimeLibType RLT = GetRuntimeLibType(Args);
   switch (RLT) {
-  case ToolChain::RLT_CompilerRT:
-    CmdArgs.push_back(
-        Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
+  case ToolChain::RLT_CompilerRT: {
+    const std::string FileName = getCompilerRT(Args, "builtins");
+    llvm::StringRef BaseName = llvm::sys::path::filename(FileName);
+    BaseName.consume_front("lib");
+    BaseName.consume_back(".a");
+    CmdArgs.push_back(Args.MakeArgString("-l" + BaseName));
     return;
+  }
   case ToolChain::RLT_Libgcc:
     CmdArgs.push_back("-lgcc");
     return;
   }
   llvm_unreachable("Unhandled RuntimeLibType.");
 }
 
 void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                      const InputInfo &Output,
                                      const InputInfoList &Inputs,
                                      const ArgList &Args,
                                      const char *LinkingOutput) const {
   ArgStringList CmdArgs;
 
-  auto &TC = static_cast<const toolchains::BareMetal&>(getToolChain());
+  auto &TC = static_cast<const toolchains::BareMetal &>(getToolChain());
 
   AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
 
   CmdArgs.push_back("-Bstatic");
 
   Args.AddAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
                             options::OPT_e, options::OPT_s, options::OPT_t,
                             options::OPT_Z_Flag, options::OPT_r});
 
   TC.AddFilePathLibArgs(Args, CmdArgs);
 
-  CmdArgs.push_back(Args.MakeArgString("-L" + TC.getRuntimesDir()));
+  for (const auto &LibPath : TC.getLibraryPaths())
+    CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-L", LibPath)));
+
+  const std::string FileName = TC.getCompilerRT(Args, "builtins");
+  llvm::SmallString<128> PathBuf{FileName};
+  llvm::sys::path::remove_filename(PathBuf);
+  CmdArgs.push_back(Args.MakeArgString("-L" + PathBuf));
 
   if (TC.ShouldLinkCXXStdlib(Args))
     TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     CmdArgs.push_back("-lc");
     CmdArgs.push_back("-lm");
 
     TC.AddLinkRuntimeLib(Args, CmdArgs);
   }
 
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(),
                                          Args.MakeArgString(TC.GetLinkerPath()),
                                          CmdArgs, Inputs, Output));
 }

clang/test/Driver/baremetal.cpp

 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target armv6m-none-eabi \
 // RUN:     -nostdinc \
 // RUN:     --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
 // CHECK-V6M-LIBINC-NOT: "-internal-isystem"
 
+// RUN: %clang -no-canonical-prefixes -rtlib=compiler-rt %s -### -o %t.o 2>&1 \
+// RUN:     -target armv7m-vendor-none-eabi \
+// RUN:     --sysroot=%S/Inputs/baremetal_arm \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7M-PER-TARGET %s
+// CHECK-ARMV7M-PER-TARGET: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-ARMV7M-PER-TARGET: "-isysroot" "[[SYSROOT:[^"]*]]"
+// CHECK-ARMV7M-PER-TARGET: "-x" "c++" "{{.*}}baremetal.cpp"
+// CHECK-ARMV7M-PER-TARGET: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-ARMV7M-PER-TARGET: "-L[[SYSROOT:[^"]+]]{{[/\\]+}}lib"
+// CHECK-ARMV7M-PER-TARGET: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}armv7m-vendor-none-eabi
+// CHECK-ARMV7M-PER-TARGET: "-lc" "-lm" "-lclang_rt.builtins"
+// CHECK-ARMV7M-PER-TARGET: "-o" "{{.*}}.o"
+
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target armv6m-none-eabi \
 // RUN:     --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-DEFAULTCXX %s
 // CHECK-V6M-DEFAULTCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}Inputs{{[/\\]+}}baremetal_arm{{[/\\]+}}lib"
 // CHECK-V6M-DEFAULTCXX-SAME: "-L[[RESOURCE_DIR]]{{[/\\]+}}lib{{[/\\]+}}baremetal"

clang/test/Driver/print-libgcc-file-name-clangrt.c

 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARM-ABI %s
 // CHECK-CLANGRT-ARM-ABI: libclang_rt.builtins-armhf.a
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
 // RUN:     --target=armv7m-none-eabi \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARM-BAREMETAL %s
 // CHECK-CLANGRT-ARM-BAREMETAL: libclang_rt.builtins-armv7m.a
+
+// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
+// RUN:     --target=armv7m-vendor-none-eabi \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARM-BAREMETAL-PER-TARGET %s
+// CHECK-CLANGRT-ARM-BAREMETAL-PER-TARGET: libclang_rt.builtins.a