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@march=armv7-m
v7m-PIC;@mthumb@march=armv7-m@fPIC
```
or
```
v7a-neon;@march=armv7-a@mfloat-abi=softfp@mfpu=neon
v7a-vfpv3-d16;@march=armv7-a@mfloat-abi=softfp@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
