[PATCH] D50850: clang: Add triples support for MIPS r6

[YunQiang Su via Phabricator via cfe-commits]

wzssyqa added a comment.

This is really for Clang. I guess you mean that compiler-rt directory also need to be patched.



================
Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:115
+  if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+    ABIName = "n32";
+
----------------
atanasyan wrote:
> It looks like this change is unrelated to introducing new target triples and can be made by a separate commit.
These lines code about N32, is quite close tied with r6 staffs, as they shared lots.

Is it ok to update the descriptions?


================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2093
     BiarchTripleAliases.append(begin(MIPSELTriples), end(MIPSELTriples));
-    BiarchTripleAliases.append(begin(MIPSTriples), end(MIPSTriples));
+    BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs));
+    BiarchTripleAliases.append(begin(MIPSN32ELTriples), end(MIPSN32ELTriples));
----------------
atanasyan wrote:
> Ditto
As a question: why  MIPSTriples here?
the above mips64 lines don't include any EL Triples.


================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2437
     if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 ||
-        getTriple().isAndroid() ||
-        getTriple().isOSFreeBSD() ||
+        getTriple().getEnvironment() == llvm::Triple::GNUABIN32 ||
+        getTriple().isAndroid() || getTriple().isOSFreeBSD() ||
----------------
atanasyan wrote:
> Are you sure that integrated LLVM assembler is ready to support N32 code generation? Anyway this change is for  a separate patch.
I didn't have so many test, while helloworld programs really works.




================
Comment at: lib/Driver/ToolChains/Linux.cpp:126
       return "mips64-linux-gnu";
-    if (D.getVFS().exists(SysRoot + "/lib/mips64-linux-gnuabi64"))
-      return "mips64-linux-gnuabi64";
+    MipsCpu = (TargetSubArch == llvm::Triple::MipsSubArch_r6) ? "mipsisa64"
+                                                              : "mips64";
----------------
atanasyan wrote:
> Suppose there are both "/lib/mips64-linux-gnu" and "/lib/mipsisa64-linux-gnuabi64" paths on a disk and provided target triple is mipsisa64-linux-gnuabi64. Is it good that we return "mips64-linux-gnu" from this function anyway?
No, return `mips64-linux-gnu' is not good, I keep it just for to making sure I don't change the behavior of clang with my patch.

In fact, mips64-linux-gnu in gcc is N32, and Debian never use this triple, we use
    mips*64*-linux-gnuabin32
So, on Debian, mips64-linux-gnu should never appear.


https://reviews.llvm.org/D50850