compnerd added inline comments.
================ Comment at: cmake/caches/BaremetalARM.cmake:1 +set(LLVM_TARGETS_TO_BUILD ARM CACHE STRING "") + ---------------- 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? ================ 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: > > 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` ? ================ Comment at: lib/Driver/ToolChains/BareMetal.cpp:107-108 + ArgStringList &CmdArgs) const { + CmdArgs.push_back("-lc++"); + CmdArgs.push_back("-lc++abi"); + CmdArgs.push_back("-lunwind"); ---------------- jroelofs wrote: > jroelofs wrote: > > compnerd wrote: > > > I think that this is a bit extreme. We already have `-stdlib=libc++` and > > > `-stdlib=libstdc++`. Why not just honor that? > > I wasn't going for "support every possible thing out of the tin", instead > > preferring incremental development :) > Added support for `-stdlib=`. > > I made the assumption that `-stdlib=libstdc++` implies the user wants > libsupc++ as their ABI library, though someone may want libstdc++ + > libc++abi, or other combinations. If that's the case, we can add `-abilib=`, > and likewise `-unwinder=`... but let's save that for another patch, on > another day. Yeah, switching between libc++/libc++abi and libstdc++/libsupc++ sounds reasonable. Anything beyond that is something which doesnt work well within the driver anyways. I should polish off my `-unwinder` patch. ================ Comment at: lib/Driver/ToolChains/BareMetal.h:42 + + const char *getDefaultLinker() const override { return "ld.lld"; } + ---------------- 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 https://reviews.llvm.org/D33259 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits