[Clang][Driver] Use full path to builtins in bare-metal toolchain

[tambre]

Summary

The bare-metal toolchain ought to use full paths to link the builtins
bringing it in line with everything else.
Getting the path is now done through the getCompilerRT() function, which was
intended to be overriden in this manner.

Notably per-target runtime directories don't make sense for this toolchain,
so they aren't handled.

Related tests are updated to reference RESOURCE_DIR for their checks.
This patch is a revival of D59425.

[tambre]

Add missing dash.

[phosek]

This was discussed when LLVM_ENABLE_PER_TARGET_RUNTIME_DIR was introduced and there was a pushback against changing the driver behavior depending on the value of that option, so if we're going to reverse that decision for BareMetal, I think that deserves a broader discussion.

clang/lib/Driver/ToolChains/BareMetal.cpp

161

Is there a reason why BareMetal doesn't just use https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChain.cpp#L462 like all other drivers, and instead re-implements the runtime library lookup logic?

[tambre]

Remove define to change driver behaviour, use ToolChain::getCompilerRTArgString() instead.

[phosek]

LGTM

[tambre]

In D86877#2248064, @phosek wrote:

This was discussed when LLVM_ENABLE_PER_TARGET_RUNTIME_DIR was introduced and there was a pushback against changing the driver behavior depending on the value of that option, so if we're going to reverse that decision for BareMetal, I think that deserves a broader discussion.

Thanks for the background. I'd rather not go for a compile-time behaviour change in that case and have gone with using getCompilerRTArgString(). I'll do an internal toolchain build and do some testing before landing this.

clang/lib/Driver/ToolChains/BareMetal.cpp

161

Was unaware of that function before, thanks!
I can't see a good reason not to use it, so I've switched it over.

[tambre]

Rework patch to simply cleanup runtime handling in the bare-metal toolchain.

[tambre]

@phosek Please review again.
I've overhauled the patch as I realized that per-target runtime directories don't make sense for the bare-metal target, since the runtime is only distinguished by the specific architecture and nothing else.
As a result I've simply changed this into a cleanup doing the same thing as D59425, which seems to have languished.

[tambre]

phosek: ping

[phosek]

It's not clear why couldn't we support per-target runtime directory? It uses the standard multiarch layout, so in you'd end up using a directory like [path/to/resource-dir]/lib/armv6m-unknown-eabi/libclang_rt.builtins.a. I'd find this more preferable for consistency with other targets, it'd also enable the use of runtimes build for baremetal.

[tambre]

In D86877#2262448, @phosek wrote:

It's not clear why couldn't we support per-target runtime directory? It uses the standard multiarch layout, so in you'd end up using a directory like [path/to/resource-dir]/lib/armv6m-unknown-eabi/libclang_rt.builtins.a. I'd find this more preferable for consistency with other targets, it'd also enable the use of runtimes build for baremetal.

Agreed. Unfortunately I'm unable to actually get the builtins built for baremetal and I originally encountered this issue under circumstances I can no longer reproduce.
Abandoning this as a result. Hopefully someone else will be able to take it up.

clang/lib/Driver/ToolChains/BareMetal.h

public:
   void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                              llvm::opt::ArgStringList &CC1Args,
                              Action::OffloadKind DeviceOffloadKind) const override;
   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
+  getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
+                FileType Type = ToolChain::FT_Static) const override;
 };
 
 } // namespace toolchains
 
 namespace tools {
 namespace baremetal {
 
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {

clang/lib/Driver/ToolChains/BareMetal.cpp

void BareMetal::AddCXXStdlibLibArgs(const ArgList &Args,
   case ToolChain::CST_Libstdcxx:
     CmdArgs.push_back("-lstdc++");
     CmdArgs.push_back("-lsupc++");
     break;
   }
   CmdArgs.push_back("-lunwind");
 }
 
-void BareMetal::AddLinkRuntimeLib(const ArgList &Args,
-                                  ArgStringList &CmdArgs) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-                                       getTriple().getArchName()));
+std::string BareMetal::getCompilerRT(const llvm::opt::ArgList &Args,
+                                     StringRef Component, FileType) const {
+  SmallString<128> Path(getDriver().ResourceDir);
+
+  // Builds of compiler-rt on bare-metal targets are specialized by specific
+  // architecture rather than architecture classes. We grab the triple's
+  // architecture explicitly so we don't just link the generic thumb-mode
+  // versions of these libraries. Static library is implied by bare-metal.
+  // Note that this also means per-target runtime directories don't apply.
+  llvm::sys::path::append(Path, "lib",
+                          Twine("libclang_rt." + Component + "-" +
+                                getTriple().getArchName() + ".a"));
+
+  return std::string(Path.str());
 }
 
 void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                      const InputInfo &Output,
                                      const InputInfoList &Inputs,
                                      const ArgList &Args,
                                      const char *LinkingOutput) const {
   ArgStringList CmdArgs;
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});
 
   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(TC.getCompilerRTArgString(Args, "builtins"));
   }
 
   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));
 }

clang/test/Driver/arm-compiler-rt.c

 // RUN: %clang -target arm-none-eabi \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-EABI
-// ARM-EABI: "-L{{.*[/\\]}}Inputs/resource_dir_with_arch_subdir{{/|\\\\}}lib{{/|\\\\}}baremetal"
-// ARM-EABI: "-lclang_rt.builtins-arm"
+// ARM-EABI: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-EABI: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// ARM-EABI: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}libclang_rt.builtins-arm.a"
 
 // RUN: %clang -target arm-linux-gnueabi \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-GNUEABI
-// ARM-GNUEABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a"
+// ARM-GNUEABI: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-GNUEABI: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-arm.a"
 
 // RUN: %clang -target arm-linux-gnueabi \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -mfloat-abi=hard -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-GNUEABI-ABI
-// ARM-GNUEABI-ABI: "{{.*[/\\]}}libclang_rt.builtins-armhf.a"
+// ARM-GNUEABI-ABI: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-GNUEABI-ABI: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-armhf.a"
 
 // RUN: %clang -target arm-linux-gnueabihf \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-GNUEABIHF
-// ARM-GNUEABIHF: "{{.*[/\\]}}libclang_rt.builtins-armhf.a"
+// ARM-GNUEABIHF: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-GNUEABIHF: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-armhf.a"
 
 // RUN: %clang -target arm-linux-gnueabihf \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -mfloat-abi=soft -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-GNUEABIHF-ABI
-// ARM-GNUEABIHF-ABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a"
+// ARM-GNUEABIHF-ABI: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-GNUEABIHF-ABI: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-arm.a"
 
 // RUN: %clang -target arm-windows-itanium \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-WINDOWS
-// ARM-WINDOWS: "{{.*[/\\]}}clang_rt.builtins-arm.lib"
+// ARM-WINDOWS: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-WINDOWS: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}windows{{/|\\\\}}clang_rt.builtins-arm.lib"
 
 // RUN: %clang -target arm-linux-androideabi \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-ANDROID
-// ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins-arm-android.a"
+// ARM-ANDROID: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-ANDROID: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-arm-android.a"
 
 // RUN: %clang -target arm-linux-androideabi \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:     -rtlib=compiler-rt -mfloat-abi=hard -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix ARM-ANDROIDHF
-// ARM-ANDROIDHF: "{{.*[/\\]}}libclang_rt.builtins-armhf-android.a"
-
+// ARM-ANDROIDHF: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// ARM-ANDROIDHF: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-armhf-android.a"

clang/test/Driver/baremetal.cpp

 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target armv6m-none-eabi \
 // RUN:     -T semihosted.lds \
 // RUN:     -L some/directory/user/asked/for \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN:     --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
 // CHECK-V6M-C: "[[PREFIX_DIR:.*]]{{[/\\]+}}{{[^/^\\]+}}{{[/\\]+}}clang{{.*}}" "-cc1" "-triple" "thumbv6m-none-unknown-eabi"
 // CHECK-V6M-C-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-V6M-C-SAME: "-isysroot" "[[SYSROOT:[^"]*]]"
 // CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECk-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]{{[/\\]+}}include"
 // CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal.cpp"
 // CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-C-SAME: "-lc" "-lm"
+// CHECK-V6M-C-SAME: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target armv6m-none-eabi \
 // RUN:     -nostdlibinc -nobuiltininc \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN:     --sysroot=%S/Inputs/baremetal_arm \
 // 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: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target armv6m-none-eabi \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_baremetal \
 // 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{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-V6M-DEFAULTCXX-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm"
+// CHECK-V6M-DEFAULTCXX-SAME: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target armv6m-none-eabi \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN:     --sysroot=%S/Inputs/baremetal_arm \
 // RUN:     -stdlib=libc++ \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBCXX %s
 // CHECK-V6M-LIBCXX-NOT: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}{{[^v].*}}"
-// CHECK-V6M-LIBCXX: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-V6M-LIBCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-V6M-LIBCXX-SAME: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECK-V6M-LIBCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
-// CHECK-V6M-LIBCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-V6M-LIBCXX-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm"
+// CHECK-V6M-LIBCXX-SAME: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target armv6m-none-eabi \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN:     --sysroot=%S/Inputs/baremetal_arm \
 // RUN:     -stdlib=libstdc++ \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBSTDCXX %s
 // CHECK-V6M-LIBSTDCXX-NOT: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
-// CHECK-V6M-LIBSTDCXX: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
+// CHECK-V6M-LIBSTDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-V6M-LIBSTDCXX-SAME: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
 // CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
-// CHECK-V6M-LIBSTDCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-V6M-LIBSTDCXX-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBSTDCXX-SAME: "-lstdc++" "-lsupc++" "-lunwind"
-// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm"
+// CHECK-V6M-LIBSTDCXX-SAME: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-LIBSTDCXX-SAME: "-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:     -nodefaultlibs \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-NDL %s
 // CHECK-V6M-NDL: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"