[PATCH] D87164: Extending Baremetal toolchain's support for the rtlib option.

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

> 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!



================
Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:164
+        Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
+    break;
+  case ToolChain::RLT_Libgcc:
----------------
If you make these `break`s into `return`s, 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87164/new/

https://reviews.llvm.org/D87164