compnerd added a comment.
Looks generally pretty good. This is going to be a pretty cool addition!
================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:68
+ SmallString<128> Dir(getDriver().ResourceDir);
+ llvm::sys::path::append(Dir, "lib", "baremetal");
+ return Dir.str();
----------------
jroelofs wrote:
> compnerd wrote:
> > jroelofs wrote:
> > > 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'.
> > Because I can have a baremetal environment that uses a different
> > architecture. How do you differentiate between the MIPS and ARM bare metal
> > runtimes? The way that the compiler actually looks up the builtins is that
> > it uses `clang_rt.[component]-[arch][-variant]`
> Yes, and that's still how they're being looked up (and built/installed), even
> in this patch:
>
> `lib/clang/[version]/lib/[cmake_system_name]/libclangrt.[component]-[arch][-variant].a`
>
> Having arch+variant in the name means they won't intersect, just as they
> don't for any other system. The only difference here is that baremetal
> doesn't really have a "system" per se, and it's not appropriate to use the
> darwin/linux/whatever ones, hence the 'baremetal' folder.
Ah. I see, I was visualizing the tree incorrectly. Using `baremetal` this way
is fine by me.
================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:110
+ SmallString<128> Dir(SysRoot);
+ llvm::sys::path::append(Dir, "include", "c++", "v1");
+ return Dir.str();
----------------
Is this layout consistent between libc++ and libstdc++?
================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:130-133
+ if (Value == "libc++")
+ return ToolChain::CST_Libcxx;
+ else if (Value == "libstdc++")
+ return ToolChain::CST_Libstdcxx;
----------------
Use `StringSwitch`?
================
Comment at: lib/Driver/ToolChains/BareMetal.h:39
+ bool isPICDefaultForced() const override { return false; }
+ bool SupportsProfiling() const override { return true; }
+ bool SupportsObjCGC() const override { return false; }
----------------
Is the profiler support in compiler-rt sufficiently standalone to build it for
baremetal?
https://reviews.llvm.org/D33259
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
