Extending Baremetal toolchain's support for the rtlib option.

[mcarrasco]

Summary

The Baremetal toolchain currently has a fixed behavior regardless of the -rtlib option's value. It is always linking against compiler-rt unless -nodefaultlibs is enabled.

This proposal slightly changes the code in such a way that enabling -rtlib=libgcc implies linking against -libgcc.

Something that I'm unsure about this patch is that we are not enforcing the existence of such libgcc. By reading other toolchains, I understood that is not always enforced. Do you think this policy is acceptable? If it is not, how would you think it should be addressed?

Context:

So far I manually set an -L path to a valid libgcc on my system when clang is invoked. In this particular case, I use arm-none-eabi-gcc -mthumb -mcpu=cortex-m4 -mfloat-abi=hard -mfpu=fpv4-sp-d16 -print-libgcc-file-name which retrieves the path I want.

Assume the user forwards a gcc-toolchain installation through --gcc-toolchain. Would be a good idea to programmatically query arm-none-eabi-gcc for the libgcc as described above? If that is the case, how would be the most "llvm way" to do it? I'm not very related to all abstractions and concepts from the framework.

This is my very first contribution to LLVM so I'd really appreciate any feedback in order to improve this proposal :)

[jroelofs]

The Baremetal toolchain currently has a fixed behavior regardless of the -rtlib option's value. It is always linking against compiler-rt unless -nodefaultlibs is enabled.

This proposal slightly changes the code in such a way that enabling -rtlib=libgcc implies linking against -libgcc.

Something that I'm unsure about this patch is that we are not enforcing the existence of such libgcc. By reading other toolchains, I understood that is not always enforced. Do you think this policy is acceptable? If it is not, how would you think it should be addressed?

I think it's fine. You'll get linker errors if the library can't be found.

Context:

So far I manually set an -L path to a valid libgcc on my system when clang is invoked. In this particular case, I use arm-none-eabi-gcc -mthumb -mcpu=cortex-m4 -mfloat-abi=hard -mfpu=fpv4-sp-d16 -print-libgcc-file-name which retrieves the path I want.

Assume the user forwards a gcc-toolchain installation through --gcc-toolchain. Would be a good idea to programmatically query arm-none-eabi-gcc for the libgcc as described above?

I don't think it's appropriate to bake this into clang's driver. Instead, this should be done by the build script, and the resulting path should be fed in via the -resource-dir flag.

If that is the case, how would be the most "llvm way" to do it? I'm not very related to all abstractions and concepts from the framework.

This is my very first contribution to LLVM so I'd really appreciate any feedback in order to improve this proposal :)

Sorry it took me a few days to get around to reviewing this. Been a bit busy with a move.... thanks for the patch!

clang/lib/Driver/ToolChains/BareMetal.cpp

164

If you make these breaks into returns, and put an llvm_unreachable("unhandled RuntimeLibType"); after the switch, then whoever adds a new entry to that enum will get a nice warning showing them that they need to update this code.

[mcarrasco]

@jroelofs no worries, thanks a lot for your time.

Please tell me if I understood you correctly.

It is acceptable just to append the -lgcc if -rtlib=libgcc is used, although the user is responsible to ensure that correct paths are provided so libgcc is found.

I also agree with the above, although I just want to know if I got your words correctly. In addition, you suggest that the build system should invoke clang with a valid value in --resource-dir option in order find libgcc.

Could you point me where I can find more info regarding --resource-dir? So far I only found this:

clang --help-hidden | grep resource-dir
  -print-resource-dir     Print the resource directory pathname
  -resource-dir <value>   The directory which holds the compiler resource files

Sincerly, I'm a little bit confused about: -sysroot, -gcc-toolchain and now -resource-dir

Thanks for your feedback!
Manuel.

clang/lib/Driver/ToolChains/BareMetal.cpp

164

Thanks!

[jroelofs]

In D87164#2268366, @mcarrasco wrote:

@jroelofs no worries, thanks a lot for your time.

Please tell me if I understood you correctly.

It is acceptable just to append the -lgcc if -rtlib=libgcc is used, although the user is responsible to ensure that correct paths are provided so libgcc is found.

Kind of, yeah.

I also agree with the above, although I just want to know if I got your words correctly. In addition, you suggest that the build system should invoke clang with a valid value in --resource-dir option in order find libgcc.

If you're putting together your own toolchain, then the right thing to do is to install libgcc into the folder where clang expects to find the compiler-rt builtins in the resource-dir. This is usually in a folder that's sibling to the compiler itself, something like lib/clang/$version/lib/baremetal, assuming clang lives at bin/clang.

Could you point me where I can find more info regarding --resource-dir? So far I only found this:

clang --help-hidden | grep resource-dir
  -print-resource-dir     Print the resource directory pathname
  -resource-dir <value>   The directory which holds the compiler resource files

Sincerly, I'm a little bit confused about: -sysroot, -gcc-toolchain and now -resource-dir

I don't know of any docs for it off the top of my head, but roughly speaking:

• resource-dir is where the compiler is instructed to find the compiler-support libraries (i.e. compiler-rt or libgcc)
• sysroot is where libc, the stl, unwinder, c++ abi support library etc should live. The folder that this points at should have the same layout as root would (i.e. with $sysroot/usr/lib, $sysroot/usr/include, etc), were you building for the target from the target (not that that makes much sense for baremetal, but you get the picture).
• -gcc-toolchain is where to look for a gcc toolchain for your target. This is mainly for linux targets that rely on an installed gcc toolchain, and isn't hooked up for the bare metal side of things. This isn't necessary if you have a self-contained clang toolchain built that doesn't rely on those bits of gcc.

Thanks for your feedback!
Manuel.

[mcarrasco]

Great!

Thanks a lot for your answer @jroelofs !!

Then, do you think this current proposal to BareMetal.cpp is enough?

Best,
Manuel.

[jroelofs]

Yeah, this should do what you're after. LGTM

[jroelofs]

btw, do you need me to commit it for you?

[mcarrasco]

@jroelofs yes please :p

After you mentioned it, I rechecked the guidelines and read:

Once a patch has been reviewed and approved on Phabricator it can then be committed to trunk. If you do not have commit access, someone has to commit the change for you (with attribution). It is sufficient to add a comment to the approved review indicating you cannot commit the patch yourself.

Best,

clang/lib/Driver/ToolChains/BareMetal.cpp

case ToolChain::CST_Libstdcxx:
     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()));
+  ToolChain::RuntimeLibType RLT = GetRuntimeLibType(Args);
+  switch (RLT) {
+  case ToolChain::RLT_CompilerRT:
+    CmdArgs.push_back(
+        Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
+    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;

clang/test/Driver/baremetal.cpp

 
 // RUN: %clangxx -target arm-none-eabi -mthread-model single -v 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-THREAD-MODEL-SINGLE
 // CHECK-THREAD-MODEL-SINGLE: Thread model: single
 
 // RUN: %clangxx -target arm-none-eabi -mthread-model posix -v 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-THREAD-MODEL-POSIX
 // CHECK-THREAD-MODEL-POSIX: Thread model: posix
+
+// RUN: %clang -### -target arm-none-eabi -rtlib=libgcc -v %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-RTLIB-GCC
+// CHECK-RTLIB-GCC: -lgcc