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

238

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

[michaelplatings]

Rebase

clang/lib/Driver/ToolChains/BareMetal.cpp

238

Changed to return an iterator range.

[phosek]

clang/lib/Driver/ToolChains/BareMetal.cpp

260–264

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

[michaelplatings]

Rebase

clang/lib/Driver/ToolChains/BareMetal.cpp

260–264

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

254–258

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

262–264

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

clang/lib/Driver/ToolChains/BareMetal.h

75

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:
+  llvm::SmallVector<Multilib> 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());
-  if (!SelectedMultilibs.empty()) {
-    Result += SelectedMultilibs.back().osSuffix();
+  return computeBaseSysRoot(getDriver(), getTriple());
+}
+
+llvm::SmallVector<Multilib> BareMetal::getOrderedMultilibs() const {
+  llvm::SmallVector<Multilib> Result;
+  // Get multilibs in reverse order because they're ordered most-specific last.
+  for (auto It = SelectedMultilibs.rbegin(), End = SelectedMultilibs.rend();
+       It != End; ++It) {
+    Result.push_back(*It);
+  }
+  if (Result.empty()) {
+    Result.push_back(Multilib());
   }
   return Result;
 }
 
 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/Inputs/baremetal_multilib/multilib.yaml

 - dir: arm-none-eabi/thumb/v8.1-m.main/fp
   flags: [target=thumbv8.1m.main-none-unknown-eabihf, hasfpu]
   printArgs: [--target=thumbv8.1m.main-none-eabihf]
 
 - dir: arm-none-eabi/thumb/v8.1-m.main/nofp/mve
   flags: [target=thumbv8.1m.main-none-unknown-eabihf, march=+mve]
   printArgs: [--target=arm-none-eabihf, -march=armv8.1m.main+nofp+mve]
 
+# A specialisation of v8.1-m.main/fp without exceptions.
+# This layers over the top of the regular v8.1-m.main/fp so it doesn't
+# need to have its own include directory or C library, thereby saving
+# disk space.
+- dir: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept
+  flags: [target=thumbv8.1m.main-none-unknown-eabihf, hasfpu, fno-exceptions]
+  printArgs: [--target=thumbv8.1m.main-none-eabihf, -fno-exceptions]
+
 
 # The second section of the file is a map from auto-detected flags
 # to custom flags. The auto-detected flags can be printed out
 # by running clang with `-print-multi-selection-flags`.
 # The regex must match a whole flag string.
 # "matchFlags" flags will be added if an argument matches, while
 # "noMatchFlags" flags will be added otherwise.
 flagMap:

clang/test/Driver/baremetal-multilib.cpp

 // CHECK-SAME: "-lc" "-lm" "-lclang_rt.builtins"
 // CHECK-SAME: "-o" "{{.*}}.o"
 
 // RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-directory 2>&1 \
 // RUN:     --target=thumbv8m.main-none-eabihf --sysroot= \
 // RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-DIRECTORY %s
 // CHECK-PRINT-MULTI-DIRECTORY: arm-none-eabi/thumb/v8-m.main/fp
 
+// RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN:     --target=thumbv8.1m.main-none-eabihf -fno-exceptions --sysroot= \
+// RUN:   | FileCheck --check-prefix=CHECK-LAYERED-MULTILIB %s
+// CHECK-LAYERED-MULTILIB: "{{.*}}clang{{.*}}" "-cc1" "-triple" "thumbv8.1m.main-none-unknown-eabihf"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/include/c++/v1"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/include/c++/v1"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/include"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/include"
+// CHECK-LAYERED-MULTILIB-NEXT: "-L{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/lib"
+// CHECK-LAYERED-MULTILIB-SAME: "-L{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/lib"
+
+// RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-directory 2>&1 \
+// RUN:     --target=thumbv8.1m.main-none-eabihf -fno-exceptions --sysroot= \
+// RUN:   | FileCheck --check-prefix=CHECK-LAYERED-PRINT-MULTI-DIRECTORY %s
+// CHECK-LAYERED-PRINT-MULTI-DIRECTORY: arm-none-eabi/thumb/v8.1-m.main/fp
+// CHECK-LAYERED-PRINT-MULTI-DIRECTORY-NEXT: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept
+
 // RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-lib 2>&1 \
 // RUN:     --target=arm-none-eabi --sysroot= \
 // RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-LIB %s
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v6-m/nofp;@-target=thumbv6m-none-eabi@mfloat-abi=soft
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v7-m/nofp;@-target=thumbv7m-none-eabi@mfloat-abi=soft
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v7e-m/nofp;@-target=thumbv7em-none-eabi@mfloat-abi=soft@mfpu=none
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8-m.main/nofp;@-target=arm-none-eabi@mfloat-abi=soft@march=armv8m.main+nofp
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/nofp/nomve;@-target=arm-none-eabi@mfloat-abi=soft@march=armv8.1m.main+nofp+nomve
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v7e-m/fpv4_sp_d16;@-target=thumbv7em-none-eabihf@mfpu=fpv4-sp-d16
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v7e-m/fpv5_d16;@-target=thumbv7em-none-eabihf@mfpu=fpv5-d16
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8-m.main/fp;@-target=thumbv8m.main-none-eabihf
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/fp;@-target=thumbv8.1m.main-none-eabihf
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/nofp/mve;@-target=arm-none-eabihf@march=armv8.1m.main+nofp+mve
+// CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept;@-target=thumbv8.1m.main-none-eabihf@fno-exceptions
 
 // RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -x assembler -mexecute-only \
 // RUN:     --target=arm-none-eabi --sysroot= %s -c -### 2>&1 \
 // RUN:    | FileCheck %s --check-prefix=CHECK-NO-EXECUTE-ONLY-ASM
 // CHECK-NO-EXECUTE-ONLY-ASM: warning: argument unused during compilation: '-mexecute-only'