[PATCH] D51464: clang: fix MIPS/N32 triple and paths

[Simon Atanasyan via Phabricator via llvm-commits]

atanasyan requested changes to this revision.
atanasyan added a comment.
This revision now requires changes to proceed.

This patch fails the following test cases:

- tools/clang/test/CodeGen/target-data.c
- tools/clang/test/Driver/mips-cs.cpp



================
Comment at: lib/Basic/Targets/Mips.h:75
+                                                                  : "n64";
+    setABI(getTriple().isMIPS32() ? "o32" : Mips64Abi);
 
----------------
Let's write all cases in a uniform manner:
```
if (Triple.isMIPS32())
  setABI("o32");
else if (Triple.getEnvironment() == llvm::Triple::GNUABIN32)
  setABI("n32");
else
  setABI("n64");
```


================
Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:109
 
+  if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+    ABIName = "n32";
----------------
What about the following combination of a command line arguments?

  -target mips-mti-linux-gnuabin32 -mips64

In that case, ABIName is empty, Triple.getVendor() returns MipsTechnologies, CPUName is "mips64". So ABIName becomes "n64". And this new `if` statement doesn't work.


================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2426
     if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 ||
-        getTriple().isAndroid() ||
-        getTriple().isOSFreeBSD() ||
+        getTriple().getEnvironment() == llvm::Triple::GNUABIN32 ||
+        getTriple().isAndroid() || getTriple().isOSFreeBSD() ||
----------------
Before this change we enable integrated assembler for mips64/mips64el architectures only when we are sure that target ABI is N64. The problem is that there are some bugs which do not allow the integrated assembler generates correct N32 code in all cases. After this change we enable integrated assembler for N32 ABI. I do not think it's a good idea now.

If we can pass command line arguments to this routine, it probably would be possible to detect N32 ABI by checking both GNUABIN32  environment and `-mabi=n32` option. And disable integrated assembler for MIPS targets in that case only. But anyway this change is for another patch.


Repository:
  rC Clang

https://reviews.llvm.org/D51464