[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.


================
Comment at: cmake/caches/BaremetalARM.cmake:1
+set(LLVM_TARGETS_TO_BUILD ARM CACHE STRING "")
+
----------------
compnerd wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > Please rename this file to `BareMetalARMv6.cmake`.  (I'm interested in the suffix primarily).
> > My plan is to eventually add multilibs to this configuration, so while that makes sense short term, I don't think it makes sense long term.
> > 
> > With that in mind, do you still want me to rename it?
> I think it makes sense longer term still.  ARMv6 vs ARMv7.  Even if you do multilib, that wouldnt take care of that right?
It's not limited to just ARMv6m multilibs... imagine it had:


```
set(LLVM_BUILTIN_TARGETS "armv6m-none-eabi;armv7m-none-eabi" CACHE STRING "Builtin Targets")
```

Then naming the file `BaremetalARMv6m.cmake` will be inappropriately specific.


Granted, the way clangrt is built/used now isn't very conducive to multilibs that differ by more than just the arch, since the name currently has to be of the form: `libclang_rt-builtins-${triple's subarch}.a`, which for example doesn't allow for differentiating between:
```
v7m;@mthumb at march=armv7-m
v7m-PIC;@mthumb at march=armv7-m at fPIC
```

or

```
v7a-neon;@march=armv7-a at mfloat-abi=softfp at mfpu=neon
v7a-vfpv3-d16;@march=armv7-a at mfloat-abi=softfp at mfpu=vfpv3-d16
```


================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:68
+  SmallString<128> Dir(getDriver().ResourceDir);
+  llvm::sys::path::append(Dir, "lib", "baremetal");
+  return Dir.str();
----------------
compnerd wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > Why not just the standard `arm` directory?
> > There are a few differences between the stuff in the existing ones, and what is needed on baremetal. For example __enable_execute_stack, emutls, as well as anything else that assumes existence of pthreads support shouldn't be there.
> Well, I think that "baremetal" here is a bad idea.  How about using the android approach?  Use `clang_rt.builtins-arm-baremetal.a` ?
Why? Given the way the cmake goop works in lib/runtimes + compiler-rt, the folder name there has to be the same as the CMAKE_SYSTEM_NAME. The alternative, I guess, is to call it 'generic', but I'm not convinced that's better than 'baremetal'.


================
Comment at: lib/Driver/ToolChains/BareMetal.h:42
+
+  const char *getDefaultLinker() const override { return "ld.lld"; }
+
----------------
compnerd wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > I think that this really should be `ld` still, as that is the canonical name for the linker.
> > You mean `lld`?
> > 
> > ```
> > $ lld
> > lld is a generic driver.
> > Invoke ld.lld (Unix), ld (macOS) or lld-link (Windows) instead.
> > ```
> > 
> > Or are you saying: "make binutils ld the default, not llvm's lld"?
> Im saying use the name "ld".  ld is a symlink on most linux distributions these days.  ld -> ld.gold, ld.bfd, ld.lld
I don't think this makes sense. I don't care about "most linux distributions", but rather about putting together an LLVM baremetal distribution based as much as possible on LLVM components. If someone wants a different linker, they can use `-fuse-ld=` and/or `-B`.

Also, doesn't using a symlink named `ld` cause `lld` to behave like a mach-o linker? We really want the elf one here.


https://reviews.llvm.org/D33259