[PATCH] Add driver support for Native Client SDK

[Derek Schuff]

================
Comment at: lib/Basic/Targets.cpp:3897
@@ -3896,1 +3896,3 @@
+      assert(!BigEndian && "NaCl on ARM does not support big endian");
+      DescriptionString = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S128";
     } else {
----------------
jvoung wrote:
> BTW, for the description string there was some test that did:
> 
> // RUN: %clang_cc1 -triple arm-nacl-gnueabi
> 
> wonder if it should just change to arm-nacl or arm-nacl-gnueabihf.
> 
> test/CodeGen/target-data.c
Just made it arm-nacl (the environment shouldn't affect the datalayout anyway)

================
Comment at: lib/Driver/ToolChains.cpp:2399
@@ +2398,3 @@
+void NaCl_TC::AddCXXStdlibLibArgs(const ArgList &Args,
+                                    ArgStringList &CmdArgs) const {
+  // Allow and ignore -stdlib=libc++ without warning, but not libstdc++
----------------
jvoung wrote:
> parameters don't seem lined up here and "AddClangCXXStdlibIncludeArgs"
Fixed

================
Comment at: lib/Driver/ToolChains.cpp:2400
@@ +2399,3 @@
+                                    ArgStringList &CmdArgs) const {
+  // Allow and ignore -stdlib=libc++ without warning, but not libstdc++
+  GetCXXStdlibType(Args);
----------------
jvoung wrote:
> The "warning" part is a bit confusing on first read. You have to look at the implementation of "GetCXXStdlibType" to really know that there's warning.
> 
made it clearer.

================
Comment at: lib/Driver/ToolChains.cpp:2447
@@ +2446,3 @@
+      TheTriple.getEnvironment() == llvm::Triple::UnknownEnvironment)
+    TheTriple.setEnvironment(llvm::Triple::GNUEABIHF);
+  return TheTriple.getTriple();
----------------
jvoung wrote:
> Is there anywhere that would warn if plain "gnueabi" was specified?
This just covers the case if it's not specified. If gnueabi is specified that should override this default and be honored. Since the behavior of the regular arm compiler is just to allow the user to do that, maybe we should too?

================
Comment at: lib/Driver/Tools.cpp:7853
@@ +7852,3 @@
+void nacltools::AssembleARM::ConstructJob(Compilation &C, const JobAction &JA,
+                                       const InputInfo &Output,
+                                       const InputInfoList &Inputs,
----------------
jvoung wrote:
> line up params
done.

================
Comment at: lib/Driver/Tools.cpp:7872
@@ +7871,3 @@
+// others. Eventually we can support more of that and hopefully migrate back
+// to gnutools::link
+void nacltools::Link::ConstructJob(Compilation &C, const JobAction &JA,
----------------
jvoung wrote:
> Add a period at the end of the sentence.
done.

================
Comment at: lib/Driver/Tools.cpp:7883
@@ +7882,3 @@
+  const bool IsStatic =
+    !Args.hasArg(options::OPT_dynamic) &&
+    !Args.hasArg(options::OPT_shared);
----------------
jvoung wrote:
> Technically there is OPT_pie too, comparing this to the gnutools version.
> 
> """
> OPT_dynamic looks like a darwin thing. Under the GCC man page:
>  -dynamic
>  -...
> 
> These options are passed to the Darwin linker.  The Darwin linker man page describes them in detail.
> """
Yeah... because we default to static, we wanted a way to sort of support doing dynamic, at least until we can make it the default. we don't support PIE at all yet.

================
Comment at: lib/Driver/Tools.cpp:7917
@@ +7916,3 @@
+    CmdArgs.push_back("armelf_nacl");
+  else
+    CmdArgs.push_back("elf_x86_64_nacl");
----------------
jvoung wrote:
> Should explicitly check for x86-64 and have an llvm_unreachable or error for unexpected arches.
done. made it a diag since putting an unexpected arch on the command line actually does make it all the way to link here.

================
Comment at: lib/Driver/Tools.cpp:7946
@@ +7945,3 @@
+
+  const ToolChain::path_list Paths = ToolChain.getFilePaths();
+
----------------
jvoung wrote:
> Does a ref work?
> 
> const ToolChain::path_list &Paths = ... 
yes done. im guessing that was probably the original intention since it's already const.

================
Comment at: lib/Driver/Tools.cpp:7974
@@ +7973,3 @@
+      CmdArgs.push_back("-lc");
+      // libc++ and PPAPI programs always require libpthread, so just always
+      // include it in the group for C++.
----------------
jvoung wrote:
> Might not need to mention PPAPI or phrase this differently, since it's not necessarily nacl-clang is not browser-specific.
done.

http://reviews.llvm.org/D8590

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/