[PATCH] D11279: Initial patch for PS4 toolchain

[Eric Christopher]

echristo added a comment.

Added a bunch of inline comments. The easiest way for this is probably going to be to split this out into a few separate patches where we get the skeleton in and then start filling it out.

Thanks!

-eric


================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:203-208
@@ +202,8 @@
+  InGroup<InvalidOrNonExistentDirectory>;
+def warn_drv_ps4_sdk_include : Warning<
+  "unable to find PS4 system headers directory, expected to be in '%0'">,
+  InGroup<InvalidOrNonExistentDirectory>;
+def warn_drv_ps4_sdk_lib : Warning<
+  "unable to find PS4 system libraries directory, expected to be in '%0'">,
+  InGroup<InvalidOrNonExistentDirectory>;
+
----------------
Are the existing header warnings insufficient?

================
Comment at: include/clang/Driver/Options.td:1159-1160
@@ -1158,2 +1158,4 @@
 def lazy__library : Separate<["-"], "lazy_library">, Flags<[LinkerInput]>;
+def linker : Separate<["-"], "linker">, HelpText<"Use linker <name>">, MetaVarName<"<name>">;
+def linker_EQ : Joined<["-"], "linker=">, Alias<linker>;
 def mlittle_endian : Flag<["-"], "mlittle-endian">, Flags<[DriverOption]>;
----------------
Can this be done separately?

================
Comment at: lib/Driver/Tools.cpp:3115-3118
@@ -3106,6 +3114,6 @@
 
-  // Introduce a Darwin-specific hack. If the default is PIC but the flags
-  // specified while enabling PIC enabled level 1 PIC, just force it back to
-  // level 2 PIC instead. This matches the behavior of Darwin GCC (based on my
-  // informal testing).
-  if (PIC && getToolChain().getTriple().isOSDarwin())
+  // Introduce a Darwin and PS4-specific hack. If the default is PIC but
+  // the flags specified while enabling PIC enabled level 1 PIC, just
+  // force it back to level 2 PIC instead. This matches the behavior of
+  // Darwin GCC (based on my informal testing).
+  if (PIC && (getToolChain().getTriple().isOSDarwin() ||
----------------
If you could rewrite and separate this out a bit more it'd be great.

================
Comment at: lib/Driver/Tools.cpp:3590-3591
@@ -3580,4 +3589,4 @@
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-                   /*Default*/ true))
+                   /*Default*/ !Triple.isPS4CPU()))
     CmdArgs.push_back("-dwarf-column-info");
----------------
Hmm?

================
Comment at: lib/Driver/Tools.cpp:3613
@@ -3603,3 +3612,3 @@
   // backend.
-  if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+  if (Args.hasArg(options::OPT_gdwarf_aranges) || Triple.isPS4CPU()) {
     CmdArgs.push_back("-backend-option");
----------------
Ditto.

================
Comment at: lib/Driver/Tools.cpp:9435-9436
@@ +9434,4 @@
+
+  const char *Exec =
+      Args.MakeArgString(getToolChain().GetProgramPath("ps4-as"));
+  C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
----------------
Using an external assembler rather than the integrated one?

================
Comment at: lib/Driver/Tools.cpp:9733-9736
@@ +9732,6 @@
+
+  if (PS4Linker)
+    ConstructPS4LinkJob(*this, C, JA, Output, Inputs, Args, LinkingOutput);
+  else
+    ConstructGoldLinkJob(*this, C, JA, Output, Inputs, Args, LinkingOutput);
+}
----------------
Based on this (and the above code) might we want an enum set of options to the linker bit? How about we split up this patch and leave the linker option separate and we can talk about and add that bit after?

================
Comment at: lib/Driver/Tools.h:783
@@ -782,1 +782,3 @@
 
+namespace PS4cpu {
+class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
----------------
Hrm. PS4 maybe? PS4cpu seems to say that it's a custom cpu target and not a custom environment?

================
Comment at: lib/Frontend/InitHeaderSearch.cpp:325-344
@@ +324,22 @@
+  case llvm::Triple::PS4: {
+    // <isysroot> gets prepended later in AddPath().
+    std::string BaseSDKPath = "";
+    if (!HasSysroot) {
+      const char *envValue = getenv("SCE_PS4_SDK_DIR");
+      if (envValue)
+        BaseSDKPath = envValue;
+      else {
+        // HSOpts.ResourceDir variable contains the location of Clang's
+        // resource files.
+        // Assuming that Clang is configured for PS4 without
+        // --with-clang-resource-dir option, the location of Clang's resource
+        // files is <SDK_DIR>/host_tools/lib/clang
+        SmallString<128> P = StringRef(HSOpts.ResourceDir);
+        llvm::sys::path::append(P, "../../..");
+        BaseSDKPath = P.str();
+      }
+    }
+    AddPath(BaseSDKPath + "/target/include", System, false);
+    if (triple.isPS4CPU())
+      AddPath(BaseSDKPath + "/target/include_common", System, false);
+  }
----------------
This all seems odd, what's going on here?


http://reviews.llvm.org/D11279