[PATCH] D11279: Initial patch for PS4 toolchain

[Eric Christopher via cfe-commits]

echristo added a comment.

Replying to the inline comments. I know we're waiting on the linker stuff, but Katya made me realize I hadn't replied to your comments.

-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>;
+
----------------
filcab wrote:
> echristo wrote:
> > Are the existing header warnings insufficient?
> Yes. We have an expected location and would like the whole "PS4 SDK headers" thing in there.
> But I can change it to be more general, like:
> 
>   def warn_drv_unable_find_directory_expected : Warning<
>     "unable to find %0 directory, expected to be in '%1'">,
>     InGroup<InvalidOrNonExistentDirectory>;
> 
> That might be preferable. That way other toolchains can use it (and we can merge the "libraries" and "headers" warnings.
Seems totally reasonable :)

I can see it being something that you'd want to enable to make sure that sysroot prefixes are paid attention to and, e.g. /usr/include is never touched.

================
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");
----------------
filcab wrote:
> echristo wrote:
> > Hmm?
> We have different defaults from other platforms.
*nod* disabling column info yes? I have "opinions" on this, but it's not my platform. Needs a comment.

================
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");
----------------
filcab wrote:
> echristo wrote:
> > Ditto.
> Ditto, different defaults.
> But I guess I can hoist out the Triple.isPS4CPU() on both cases, if you prefer, like it's done for other toolchains like:
>   bool IsWindowsMSVC = getToolChain().getTriple().isWindowsMSVCEnvironment();
Probably better to just handle it the same way as the column info I'd think. Also, this is all terrible and needs to change, that said, not your problem. :)

================
Comment at: lib/Driver/Tools.h:783
@@ -782,1 +782,3 @@
 
+namespace PS4cpu {
+class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
----------------
filcab wrote:
> echristo wrote:
> > Hrm. PS4 maybe? PS4cpu seems to say that it's a custom cpu target and not a custom environment?
> PS4cpu (or PS4CPU) is more consistent with how we've been naming things in open source.
Sure.

================
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);
+  }
----------------
kromanova wrote:
> echristo wrote:
> > This all seems odd, what's going on here?
> Our SDK layout structure is different from other platforms. Some of the PS4 specific headers (e.g. graphic headers, etc) are placed into target/include_common directory and we want to have them in the search path.
You also check for ps4cpu in here?


http://reviews.llvm.org/D11279