[PATCH] D11279: Initial patch for PS4 toolchain

[Filipe Cabecinhas]

filcab planned changes to this revision.
filcab added a comment.

Thank you, Eric.
We'll address the comments and post back an updated patch. Probably make the linker a separate patch and submit it before we update this one.

I would prefer to not split this into "add the toolchain, then fill it out more" (if possible), since we already have this in our internal branch. Having two bigger merges (especially if one is "non-working") or waiting until we have everything to do the merge wouldn't be ideal.


================
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() ||
----------------
echristo wrote:
> If you could rewrite and separate this out a bit more it'd be great.
I'm not the best at it, but I'll try :-)

================
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");
----------------
echristo wrote:
> Hmm?
We have different defaults from other platforms.

================
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");
----------------
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();

================
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));
----------------
echristo wrote:
> Using an external assembler rather than the integrated one?
We support both (but default to integrated, since we don't override `Generic_GCC::IsIntegratedAssemblerDefault()`).

================
Comment at: lib/Driver/Tools.h:783
@@ -782,1 +782,3 @@
 
+namespace PS4cpu {
+class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
----------------
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.


http://reviews.llvm.org/D11279