[PATCH] Add driver support for Native Client SDK

Jan Voung jvoung at chromium.org
Wed Mar 25 14:41:02 PDT 2015


================
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 {
----------------
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

================
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++
----------------
parameters don't seem lined up here and "AddClangCXXStdlibIncludeArgs"

================
Comment at: lib/Driver/ToolChains.cpp:2400
@@ +2399,3 @@
+                                    ArgStringList &CmdArgs) const {
+  // Allow and ignore -stdlib=libc++ without warning, but not libstdc++
+  GetCXXStdlibType(Args);
----------------
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.


================
Comment at: lib/Driver/ToolChains.cpp:2447
@@ +2446,3 @@
+      TheTriple.getEnvironment() == llvm::Triple::UnknownEnvironment)
+    TheTriple.setEnvironment(llvm::Triple::GNUEABIHF);
+  return TheTriple.getTriple();
----------------
Is there anywhere that would warn if plain "gnueabi" was specified?

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

================
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,
----------------
Add a period at the end of the sentence.

================
Comment at: lib/Driver/Tools.cpp:7883
@@ +7882,3 @@
+  const bool IsStatic =
+    !Args.hasArg(options::OPT_dynamic) &&
+    !Args.hasArg(options::OPT_shared);
----------------
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.
"""

================
Comment at: lib/Driver/Tools.cpp:7917
@@ +7916,3 @@
+    CmdArgs.push_back("armelf_nacl");
+  else
+    CmdArgs.push_back("elf_x86_64_nacl");
----------------
Should explicitly check for x86-64 and have an llvm_unreachable or error for unexpected arches.

================
Comment at: lib/Driver/Tools.cpp:7946
@@ +7945,3 @@
+
+  const ToolChain::path_list Paths = ToolChain.getFilePaths();
+
----------------
Does a ref work?

const ToolChain::path_list &Paths = ... 

================
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++.
----------------
Might not need to mention PPAPI or phrase this differently, since it's not necessarily nacl-clang is not browser-specific.

http://reviews.llvm.org/D8590

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






More information about the cfe-commits mailing list