compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.
================
Comment at: cmake/caches/BaremetalARM.cmake:1
+set(LLVM_TARGETS_TO_BUILD ARM CACHE STRING "")
+
----------------
Please rename this file to `BareMetalARMv6.cmake`. (I'm interested in the
suffix primarily).
================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:68
+ SmallString<128> Dir(getDriver().ResourceDir);
+ llvm::sys::path::append(Dir, "lib", "baremetal");
+ return Dir.str();
----------------
Why not just the standard `arm` directory?
================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:74
+ SmallString<128> Dir(getDriver().ResourceDir);
+ llvm::sys::path::append(Dir, "include", "c++", "v1");
+ return Dir.str();
----------------
This seems wrong. The libc++ headers should *not* be in the resource dir.
This should be based off of the sysroot IMO.
================
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");
----------------
I think that this is a bit extreme. We already have `-stdlib=libc++` and
`-stdlib=libstdc++`. Why not just honor that?
================
Comment at: lib/Driver/ToolChains/BareMetal.h:42
+
+ const char *getDefaultLinker() const override { return "ld.lld"; }
+
----------------
I think that this really should be `ld` still, as that is the canonical name
for the linker.
================
Comment at: lib/Driver/ToolChains/BareMetal.h:57
+ llvm::opt::ArgStringList &CmdArgs) const;
+protected:
+};
----------------
Unnecessary?
================
Comment at: lib/Driver/ToolChains/BareMetal.h:67
+public:
+ Linker(const ToolChain &TC) : Tool("baremetal::Linker", "ld.lld", TC) {}
+ bool isLinkJob() const override { return true; }
----------------
Update this accordingly.
https://reviews.llvm.org/D33259
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
