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

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs marked 2 inline comments as done.
jroelofs added inline comments.


================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:110
+  SmallString<128> Dir(SysRoot);
+  llvm::sys::path::append(Dir, "include", "c++", "v1");
+  return Dir.str();
----------------
compnerd wrote:
> Is this layout consistent between libc++ and libstdc++?
Damn, no it's not.


================
Comment at: lib/Driver/ToolChains/BareMetal.cpp:130-133
+    if (Value == "libc++")
+      return ToolChain::CST_Libcxx;
+    else if (Value == "libstdc++")
+      return ToolChain::CST_Libstdcxx;
----------------
compnerd wrote:
> Use `StringSwitch`?
`StringSwitch` isn't great when you want to error out of the default case. That being said, this whole function is unnecessary: I can just defer to the base class' implementation, which does almost the same thing.


================
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; }
----------------
compnerd wrote:
> Is the profiler support in compiler-rt sufficiently standalone to build it for baremetal?
IIRC, there was a test case that wanted this on for one of the arm-none-eabi triples. This is when my patches were against 4.0. After rebasing to trunk, I think that particular test is gone (split up into separate ones, I guess?).

I'll leave it off for now until someone confirms it is actually supported.


================
Comment at: lib/Driver/ToolChains/Linux.cpp:379
-/*static*/
-Generic_GCC::GCCVersion Linux::GCCVersion::Parse(StringRef VersionText) {
-  const GCCVersion BadVersion = {VersionText.str(), -1, -1, -1, "", "", ""};
----------------
Noticed that this was in the wrong place, and also, surprisingly, that it was incorrectly qualified.


https://reviews.llvm.org/D33259