BareMetal ToolChain multilib layering

[michaelplatings]

Summary

This enables layering baremetal multilibs on top of each other.
For example a multilib containing only a no-exceptions libc++ could be
layered on top of a multilib containing C libs. This avoids the need
to duplicate the C library for every libc++ variant.

[michaelplatings]

Rebase

[peter.smith]

No comments on the implementation.

[michaelplatings]

Rebase and remove some braces

[michaelplatings]

Rebase

[michaelplatings]

PrintArgs -> PrintOptions

[michaelplatings]

flags -> tags

[peter.smith]

I've set approved from the Arm side. Please leave some time for people in the US time zone to leave any final comments or ask for extensions.

[michaelplatings]

tags->flags

[michaelplatings]

Rebase

[michaelplatings]

Rebase

[michaelplatings]

Rebase

[phosek]

clang/lib/Driver/ToolChains/BareMetal.cpp

236

Can you use llvm::reverse instead of allocating an entire new collection?

[michaelplatings]

Rebase

clang/lib/Driver/ToolChains/BareMetal.cpp

236

Changed to return an iterator range.

[phosek]

clang/lib/Driver/ToolChains/BareMetal.cpp

246–250

What's the reason for returning an empty Multilib, could we instead return and empty range?

[michaelplatings]

Rebase

clang/lib/Driver/ToolChains/BareMetal.cpp

246–250

This is for consistency with the previous behaviour, which would fall back to using a default sysroot if no matching multilib was found.

[phosek]

LGTM

clang/lib/Driver/ToolChains/BareMetal.cpp

240–244

It should be possible to simplify this with llvm::reverse.

248–250

It should be possible to simplify this with llvm::reverse.

clang/lib/Driver/ToolChains/BareMetal.h

76

I'd define an alias for this type with using to avoid repeating it multiple times.

[michaelplatings]

Simplify code as suggested by @phosek

clang/lib/Driver/ToolChains/BareMetal.h

public:
   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;
+
+private:
+  using OrderedMultilibs =
+      llvm::iterator_range<llvm::SmallVector<Multilib>::const_reverse_iterator>;
+  OrderedMultilibs getOrderedMultilibs() const;
 };
 
 } // namespace toolchains
 
 namespace tools {
 namespace baremetal {
 
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {

clang/lib/Driver/ToolChains/BareMetal.cpp

BareMetal::BareMetal(const Driver &D, const llvm::Triple &Triple,
     : 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()) {
-    llvm::sys::path::append(SysRoot, "lib");
-    getFilePaths().push_back(std::string(SysRoot));
-    getLibraryPaths().push_back(std::string(SysRoot));
+    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));
+    }
   }
 }
 
 /// Is the triple {arm,thumb}-none-none-{eabi,eabihf} ?
 static bool isARMBareMetal(const llvm::Triple &Triple) {
   if (Triple.getArch() != llvm::Triple::arm &&
       Triple.getArch() != llvm::Triple::thumb)
     return false;
return isARMBareMetal(Triple) || isAArch64BareMetal(Triple) ||
          isRISCVBareMetal(Triple);
 }
 
 Tool *BareMetal::buildLinker() const {
   return new tools::baremetal::Linker(*this);
 }
 
 std::string BareMetal::computeSysRoot() const {
-  std::string Result = computeBaseSysRoot(getDriver(), getTriple());
+  return computeBaseSysRoot(getDriver(), getTriple());
+}
+
+BareMetal::OrderedMultilibs BareMetal::getOrderedMultilibs() const {
+  // Get multilibs in reverse order because they're ordered most-specific last.
   if (!SelectedMultilibs.empty())
-    Result += SelectedMultilibs.back().osSuffix();
-  return Result;
+    return llvm::reverse(SelectedMultilibs);
+
+  // No multilibs selected so return a single default multilib.
+  static const llvm::SmallVector<Multilib> Default = {Multilib()};
+  return llvm::reverse(Default);
 }
 
 void BareMetal::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
                                           ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc))
     return;
 
   if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
     SmallString<128> Dir(getDriver().ResourceDir);
     llvm::sys::path::append(Dir, "include");
     addSystemInclude(DriverArgs, CC1Args, Dir.str());
   }
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-    SmallString<128> Dir(computeSysRoot());
-    if (!Dir.empty()) {
+    const SmallString<128> SysRoot(computeSysRoot());
+    if (!SysRoot.empty()) {
+      for (const Multilib &M : getOrderedMultilibs()) {
+        SmallString<128> Dir(SysRoot);
+        llvm::sys::path::append(Dir, M.includeSuffix());
         llvm::sys::path::append(Dir, "include");
         addSystemInclude(DriverArgs, CC1Args, Dir.str());
       }
     }
   }
+}
 
 void BareMetal::addClangTargetOptions(const ArgList &DriverArgs,
                                       ArgStringList &CC1Args,
                                       Action::OffloadKind) const {
   CC1Args.push_back("-nostdsysteminc");
 }
 
 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;
 
   const Driver &D = getDriver();
   std::string SysRoot(computeSysRoot());
   if (SysRoot.empty())
     return;
 
+  for (const Multilib &M : getOrderedMultilibs()) {
+    SmallString<128> Dir(SysRoot);
+    llvm::sys::path::append(Dir, M.gccSuffix());
     switch (GetCXXStdlibType(DriverArgs)) {
     case ToolChain::CST_Libcxx: {
       // First check sysroot/usr/include/c++/v1 if it exists.
-    SmallString<128> TargetDir(SysRoot);
+      SmallString<128> TargetDir(Dir);
       llvm::sys::path::append(TargetDir, "usr", "include", "c++", "v1");
       if (D.getVFS().exists(TargetDir)) {
         addSystemInclude(DriverArgs, CC1Args, TargetDir.str());
         break;
       }
       // Add generic path if nothing else succeeded so far.
-    SmallString<128> Dir(SysRoot);
       llvm::sys::path::append(Dir, "include", "c++", "v1");
       addSystemInclude(DriverArgs, CC1Args, Dir.str());
       break;
     }
     case ToolChain::CST_Libstdcxx: {
-    SmallString<128> Dir(SysRoot);
       llvm::sys::path::append(Dir, "include", "c++");
       std::error_code EC;
       Generic_GCC::GCCVersion Version = {"", -1, -1, -1, "", "", ""};
       // Walk the subdirs, and find the one with the newest gcc version:
-    for (llvm::vfs::directory_iterator LI = D.getVFS().dir_begin(Dir.str(), EC),
-                                       LE;
+      for (llvm::vfs::directory_iterator
+               LI = D.getVFS().dir_begin(Dir.str(), EC),
            !EC && LI != LE; LI = LI.increment(EC)) {
         StringRef VersionText = llvm::sys::path::filename(LI->path());
         auto CandidateVersion = Generic_GCC::GCCVersion::Parse(VersionText);
         if (CandidateVersion.Major == -1)
           continue;
         if (CandidateVersion <= Version)
           continue;
         Version = CandidateVersion;
       }
-    if (Version.Major == -1)
-      return;
+      if (Version.Major != -1) {
         llvm::sys::path::append(Dir, Version.Text);
         addSystemInclude(DriverArgs, CC1Args, Dir.str());
+      }
       break;
     }
     }
   }
+}
 
 void BareMetal::AddCXXStdlibLibArgs(const ArgList &Args,
                                     ArgStringList &CmdArgs) const {
   switch (GetCXXStdlibType(Args)) {
   case ToolChain::CST_Libcxx:
     CmdArgs.push_back("-lc++");
     if (Args.hasArg(options::OPT_fexperimental_library))
       CmdArgs.push_back("-lc++experimental");

clang/test/Driver/baremetal-multilib-layered.yaml

+# REQUIRES: shell
+# UNSUPPORTED: system-windows
+
+# This test demonstrates "layered" multilib in which more than one
+# multilib is matched.
+# For example a multilib containing only a no-exceptions libc++ could
+# be layered on top of a multilib containing C libs. This avoids the
+# need to duplicate the C library for every libc++ variant.
+# However -fno-exceptions is not yet supported for multilib selection
+# so we use a more contrived -mfloat-abi example instead.
+
+# RUN: rm -rf %T/baremetal_multilib_layered
+# RUN: mkdir -p %T/baremetal_multilib_layered/bin
+# RUN: mkdir -p %T/baremetal_multilib_layered/lib/clang-runtimes
+# RUN: ln -s %clang %T/baremetal_multilib_layered/bin/clang
+# RUN: ln -s %s %T/baremetal_multilib_layered/lib/clang-runtimes/multilib.yaml
+
+# RUN: %T/baremetal_multilib_layered/bin/clang -no-canonical-prefixes -x c++ %s -### -o %t.out 2>&1 \
+# RUN:     --target=thumbv7m-none-eabi -mfloat-abi=softfp --sysroot= \
+# RUN:   | FileCheck -DSYSROOT=%T/baremetal_multilib_layered %s
+# CHECK:      "-cc1" "-triple" "thumbv7m-none-unknown-eabi"
+# CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/softfp/include/c++/v1"
+# CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/soft/include/c++/v1"
+# CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/softfp/include"
+# CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/soft/include"
+# CHECK-NEXT: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/softfp/lib"
+# CHECK-SAME: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/soft/lib"
+
+# RUN: %T/baremetal_multilib_layered/bin/clang -no-canonical-prefixes -print-multi-directory 2>&1 \
+# RUN:     --target=arm-none-eabi -mfloat-abi=softfp --sysroot= \
+# RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-DIRECTORY %s
+# CHECK-PRINT-MULTI-DIRECTORY:      soft
+# CHECK-PRINT-MULTI-DIRECTORY-NEXT: softfp
+
+---
+MultilibVersion: 1.0
+Variants:
+- Dir: soft
+  Flags: [-mfloat-abi=soft]
+- Dir: softfp
+  Flags: [-mfloat-abi=softfp]
+Mappings:
+- Match: -mfloat-abi=softfp
+  Flags: [-mfloat-abi=soft]
+...