clang: fix MIPS/N32 triple and paths

[wzssyqa]

Summary

Guess N32 ABI when no abi option is given based on llvm patch.
It now support mips64(el)-linux-gnuabin32 and mipsn32(el).

The include and library paths are also added based on
Debian/Gcc scheme.

[atanasyan]

Could you please include more context to patches sent for review?
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

lib/Driver/ToolChains/Arch/Mips.cpp

109

If target triple is mips-mti-linux-gnuabin32 the code above set (incorrectly) the ABIName to n64 and this statement will be false.

lib/Driver/ToolChains/Linux.cpp

47 ↗

(On Diff #163222)

• Do you need MipsCpu variable?
• Is it possible to use any lightweight type like StringRef for the Mips64Abi?

118 ↗

(On Diff #163222)

If a user has two toolchains installed into "/lib/mips64-linux-gnu" and "/lib/mips64-linux-gnuabin32", this code always selects mips64-linux-gnu even if N32 ABI is requested. Is it intended?

[wzssyqa]

Remove unused MipsCpu.

[wzssyqa]

remove mips64(el)-linux-gnu from path search.

[atanasyan]

This patch fails the following test cases:

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

lib/Basic/Targets/Mips.h

75 ↗

(On Diff #166220)

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

lib/Driver/ToolChains/Arch/Mips.cpp

112

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.

lib/Driver/ToolChains/Gnu.cpp

2426

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.

[atanasyan]

lib/Driver/ToolChains/Arch/Mips.cpp

88

Is possible to rewrite this piece of code (lines 85-114) as follows?

if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
  ABIName = "n32";

if (ABIName.empty() &&
    (Triple.getVendor() == llvm::Triple::MipsTechnologies ||
     Triple.getVendor() == llvm::Triple::ImaginationTechnologies)) {
  ABIName = llvm::StringSwitch<const char *>(CPUName)
                .Case("mips1", "o32")
                .Case("mips2", "o32")
                .Case("mips3", "n64")
                .Case("mips4", "n64")
                .Case("mips5", "n64")
                .Case("mips32", "o32")
                .Case("mips32r2", "o32")
                .Case("mips32r3", "o32")
                .Case("mips32r5", "o32")
                .Case("mips32r6", "o32")
                .Case("mips64", "n64")
                .Case("mips64r2", "n64")
                .Case("mips64r3", "n64")
                .Case("mips64r5", "n64")
                .Case("mips64r6", "n64")
                .Case("octeon", "n64")
                .Case("p5600", "o32")
                .Default("");
}

lib/Driver/ToolChains/Gnu.cpp

2081–2082

Why do you remove BiarchTripleAliases.append(begin(MIPSTriples), end(MIPSTriples)); in that case only? Is it intended?

[atanasyan]

Please run test suite before sending a patch to review. After applying this patch the following tests failed:

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

[wzssyqa]

ohhh. make check-all is needed, instead of make check

test/CodeGen/target-data.c

is due to duplicate line `MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"'

test/Driver/mips-cs.cpp

is due to this test use the hardcode path `/mips-linux-gnu/', so mipsel and mips64el also need
    BiarchTripleAliases.append(begin(MIPSTriples), end(MIPSTriples));
although MIPSTriples should in the last order.

lib/Driver/ToolChains/Arch/Mips.cpp

88

OK. I didn't do like this because I am not very full confident about whether there are some case that it is set to N32 even -gnuabin32 is used.

109

as I know mips-mti-linux-gnuabin32 is never used, at least for gcc.
Should we allow it?

lib/Driver/ToolChains/Gnu.cpp

2081–2082

ohhh, my fault. I should remove from mipsel also.

lib/Driver/ToolChains/Linux.cpp

47 ↗

(On Diff #163222)

oh, MipsCpu is not used here, while used in D50850.
In that patch. we need different CPU names:

mipsel vs mipsisa32r6el

etc. It is my fault to split the patches.

StringRef is not OK as, the return value of getMultiarchTriple is std::string.

118 ↗

(On Diff #163222)

Yes. It is intended.

I don't want my patch change the behavior of llvm/clang:

the previous of llvm/clang behavior is perfering /lib/mips64-linux-gnu.

And on Debian, /lib/mips64-linux-gnu should never exists, we use

/lib/mips64-linux-gnuabi64

and

/lib/mips64-linux-gnuabin32

118 ↗

(On Diff #163222)

If there is a SDK, which is using /lib/mips64-linux-gnu', and for Debian we use /lib/mips64-linux-gnuabi64':

there must at least one cannot work. So here, I choose to make sure the exists SDK working and leave the risk to Debian's official toolchain.
as the `/lib/mips64-linux-gnu' is previous than my patch by time.

By FHS and Debian's practice, cross toolchains should never be put under /lib directly, these manually installed software should exist in /usr/local or /opt.

[atanasyan]

I'm going to test current MIPS N32 ABI implementation. Maybe we are ready to enable integrated assembler for it. In that case both Generic_GCC::IsIntegratedAssemblerDefault() and MipsMCAsmInfo ctor can be simplified.

[atanasyan]

LGTM. Thanks for the patch.

lib/Driver/ToolChains/Arch/Mips.cpp

case llvm::Triple::mipsel:
       break;
     case llvm::Triple::mips64:
     case llvm::Triple::mips64el:
       CPUName = DefMips64CPU;
       break;
     }
   }
 
+  if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
+    ABIName = "n32";
+
   if (ABIName.empty() &&
       (Triple.getVendor() == llvm::Triple::MipsTechnologies ||
        Triple.getVendor() == llvm::Triple::ImaginationTechnologies)) {
     ABIName = llvm::StringSwitch<const char *>(CPUName)
                   .Case("mips1", "o32")
                   .Case("mips2", "o32")
                   .Case("mips3", "n64")
                   .Case("mips4", "n64")
                   .Case("mips5", "n64")
                   .Case("mips32", "o32")
                   .Case("mips32r2", "o32")
                   .Case("mips32r3", "o32")
                   .Case("mips32r5", "o32")
                   .Case("mips32r6", "o32")
                   .Case("mips64", "n64")
                   .Case("mips64r2", "n64")
                   .Case("mips64r3", "n64")
                   .Case("mips64r5", "n64")
                   .Case("mips64r6", "n64")
                   .Case("octeon", "n64")
                   .Case("p5600", "o32")
                   .Default("");
-    if (Triple.getEnvironment() == llvm::Triple::GNUABIN32)
-      ABIName = llvm::StringSwitch<llvm::StringRef>(ABIName)
-                    .Case("n64", "n32")
-                    .Default(ABIName);
   }
 
-  if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32))
-    ABIName = "n32";
-
   if (ABIName.empty()) {
     // Deduce ABI name from the target triple.
     ABIName = Triple.isMIPS32() ? "o32" : "n64";
   }
 
   if (CPUName.empty()) {
     // Deduce CPU name from ABI name.
     CPUName = llvm::StringSwitch<const char *>(ABIName)
                   .Case("o32", DefMips32CPU)

lib/Driver/ToolChains/Gnu.cpp

case llvm::Triple::mips:
     BiarchLibDirs.append(begin(MIPS64LibDirs), end(MIPS64LibDirs));
     BiarchTripleAliases.append(begin(MIPS64Triples), end(MIPS64Triples));
     BiarchLibDirs.append(begin(MIPSN32LibDirs), end(MIPSN32LibDirs));
     BiarchTripleAliases.append(begin(MIPSN32Triples), end(MIPSN32Triples));
     break;
   case llvm::Triple::mipsel:
     LibDirs.append(begin(MIPSELLibDirs), end(MIPSELLibDirs));
     TripleAliases.append(begin(MIPSELTriples), end(MIPSELTriples));
-    TripleAliases.append(begin(MIPSTriples), end(MIPSTriples));
     BiarchLibDirs.append(begin(MIPS64ELLibDirs), end(MIPS64ELLibDirs));
     BiarchTripleAliases.append(begin(MIPS64ELTriples), end(MIPS64ELTriples));
     BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs));
     BiarchTripleAliases.append(begin(MIPSN32ELTriples), end(MIPSN32ELTriples));
     break;
   case llvm::Triple::mips64:
     LibDirs.append(begin(MIPS64LibDirs), end(MIPS64LibDirs));
     TripleAliases.append(begin(MIPS64Triples), end(MIPS64Triples));
     BiarchLibDirs.append(begin(MIPSLibDirs), end(MIPSLibDirs));
     BiarchTripleAliases.append(begin(MIPSTriples), end(MIPSTriples));
     BiarchLibDirs.append(begin(MIPSN32LibDirs), end(MIPSN32LibDirs));
     BiarchTripleAliases.append(begin(MIPSN32Triples), end(MIPSN32Triples));
     break;
   case llvm::Triple::mips64el:
     LibDirs.append(begin(MIPS64ELLibDirs), end(MIPS64ELLibDirs));
     TripleAliases.append(begin(MIPS64ELTriples), end(MIPS64ELTriples));
     BiarchLibDirs.append(begin(MIPSELLibDirs), end(MIPSELLibDirs));
     BiarchTripleAliases.append(begin(MIPSELTriples), end(MIPSELTriples));
     BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs));
     BiarchTripleAliases.append(begin(MIPSN32ELTriples), end(MIPSN32ELTriples));
     break;
   case llvm::Triple::ppc:
     LibDirs.append(begin(PPCLibDirs), end(PPCLibDirs));
     TripleAliases.append(begin(PPCTriples), end(PPCTriples));
     BiarchLibDirs.append(begin(PPC64LibDirs), end(PPC64LibDirs));
     BiarchTripleAliases.append(begin(PPC64Triples), end(PPC64Triples));
     break;
   case llvm::Triple::ppc64:
case llvm::Triple::mipsel:
     return true;
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el:
     // Enabled for Debian, Android, FreeBSD and OpenBSD mips64/mipsel, as they
     // can precisely identify the ABI in use (Debian) or only use N64 for MIPS64
     // (Android). Other targets are unable to distinguish N32 from N64.
     if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 ||
         getTriple().isAndroid() ||
         getTriple().isOSFreeBSD() ||
         getTriple().isOSOpenBSD())
       return true;
     return false;
   default:
     return false;
   }
 }