MIPS: Triple::setArch as R6 if MipsSubArch_r6

[wzssyqa]

Summary

Currently,

clang -target mipsisa64r6-linux-gnu -mabi=32

will generate a r2 32bit object instead expected r6 one.

The reason is that for some R6 triples, the ArchName and ArchTypeName are different:

ArchTypeName has only 4 choice: mips, mipsel, mips64, mips64el, 
ArchName may be something like mipsisa{32,64}r6{,el}

[atanasyan]

If we construct T = Triple("mipsisa64r6el-unknown-linux-gnuabi64") what is result of the T.getArchName() call (without your patch)? If getArchName() returns mipsisa64r6el we need to investigate what is the reason for redundant setArch() call somewhere in the clang driver code. If getArchName() does not return mipsisa64r6el we need to fix triple parsing. One more question - why other targets which has "sub architectures" does not need to extend the Triple::setArch method?

[wzssyqa]

In D110514#3026049, @atanasyan wrote:

If we construct T = Triple("mipsisa64r6el-unknown-linux-gnuabi64") what is result of the T.getArchName() call (without your patch)?

mipsisa64r6el

If getArchName() returns mipsisa64r6el we need to investigate what is the reason for redundant setArch() call somewhere in the clang driver code.

setArch is not call by clang directly, while from:
llvm/lib/Support/Triple.cpp:

Triple::get32BitArchVariant()
Triple::get64BitArchVariant()

If getArchName() does not return mipsisa64r6el we need to fix triple parsing.
One more question - why other targets which has "sub architectures" does not need to extend the Triple::setArch method?

x86_64 has similir problem, while since they they keep compatiable, it is not a big problem:

$ bin/clang -target i686-linux-gnu -march=x86-64-v3 -m32 -c xx.c

For MIPS r6, it is not full compatiable with r2, so the falldown is a problem here.

For ARMv8, they also have simliar bug/feature:

$ bin/clang -target aarch64-linux-gnu -m32 -c xx.c
$ file xx.o
xx.o: ELF 32-bit LSB relocatable, ARM, EABI5 version 1 (SYSV), not stripped

In this situation, if ARMv8 has a native 32bit mode, they have 2 choice:

armv7-32  /    aarchv8-32

Of course, ARMv8 doesn't have this mode, as I know.

[wzssyqa]

In D110514#3026049, @atanasyan wrote:

If we construct T = Triple("mipsisa64r6el-unknown-linux-gnuabi64") what is result of the T.getArchName() call (without your patch)? If getArchName() returns mipsisa64r6el we need to investigate what is the reason for redundant setArch() call somewhere in the clang driver code. If getArchName() does not return mipsisa64r6el we need to fix triple parsing. One more question - why other targets which has "sub architectures" does not need to extend the Triple::setArch method?

any idea?

[atanasyan]

• This patch needs test cases. Take a look at llvm/unittests/ADT/TripleTest.cpp
• IMHO the current fix is not the best solution. It solves the problem but looks a bit strange. For example, the following code (unusual but correct) stops working. Without the patch it produces a correct "arm-linux-gnu" target triple.

Triple T("mipsisa32r6-linux-gnu");
T.setArch(Triple::arm);

It's better to modify Triple::setArch() function so it takes two arguments ArchType and SubArchType. The second argument is NoSubARch by default. Then the setArch() passes both arguments to the getArchTypeName(). We need to teach this function to generate architecture's name in accordance with ArchType and SubArchType arguments. In this patch we can do it for MIPS only. @MaskRay - what do you think?

[wzssyqa]

In D110514#3057388, @atanasyan wrote:

• This patch needs test cases. Take a look at llvm/unittests/ADT/TripleTest.cpp
• IMHO the current fix is not the best solution. It solves the problem but looks a bit strange. For example, the following code (unusual but correct) stops working. Without the patch it produces a correct "arm-linux-gnu" target triple.

Triple T("mipsisa32r6-linux-gnu");
T.setArch(Triple::arm);

ohhh. strange code...

It's better to modify Triple::setArch() function so it takes two arguments ArchType and SubArchType. The second argument is NoSubARch by default. Then the setArch() passes both arguments to the getArchTypeName(). We need to teach this function to generate architecture's name in accordance with ArchType and SubArchType arguments. In this patch we can do it for MIPS only. @MaskRay - what do you think?

I don't think modify `getArchTypeName' is a good idea.
I prefer keep TypeName as it, and use ArchName to hold more info.

In fact it is almost same as my first shape of code: Add a new function

getArchName(ArchType Kind)

[atanasyan]

LGTM. But get one more acceptance from any other reviewer before commit.

[arichardson]

llvm/lib/Support/Triple.cpp

1421

Shouldn't all others also preserve the subarch?

[wzssyqa]

llvm/lib/Support/Triple.cpp

1421

As I know, there are no other architectures need subarch here.
So, I think that it is better to keep it as now.

[MaskRay]

The llvm/lib/Support/Triple.cpp change LGTM.

For clang -target mipsisa64r6-linux-gnu -mabi=32, what include and library paths do it use? It may be related to clang/lib/Driver/Driver.cpp:computeTargetTriple

[wzssyqa]

This comment was removed by wzssyqa.

[wzssyqa]

In D110514#3073329, @MaskRay wrote:

The llvm/lib/Support/Triple.cpp change LGTM.

For clang -target mipsisa64r6-linux-gnu -mabi=32, what include and library paths do it use? It may be related to clang/lib/Driver/Driver.cpp:computeTargetTriple

Yes. You are right, while it is another bug.
I will fix it.

[atanasyan]

Do you have commit access?

[wzssyqa]

In D110514#3077052, @atanasyan wrote:

Do you have commit access?

No. I have no commit access.

[atanasyan]

The patch committed at rG302a165

llvm/lib/Support/Triple.cpp

void Triple::getWatchOSVersion(unsigned &Major, unsigned &Minor,
   }
 }
 
 void Triple::setTriple(const Twine &Str) {
   *this = Triple(Str);
 }
 
 void Triple::setArch(ArchType Kind) {
+  if (getSubArch() == Triple::MipsSubArch_r6) {
+    switch (Kind) {
+    case Triple::mips:
+      setArchName("mipsisa32r6");
+      return;
+    case Triple::mipsel:
+      setArchName("mipsisa32r6el");
+      return;
+    case Triple::mips64:
+      setArchName("mipsisa64r6");
+      return;
+    case Triple::mips64el:
+      setArchName("mipsisa64r6el");
+      return;
+    default:
+      llvm_unreachable("unexpected Triple for MIPS r6");
+    }
+  }
   setArchName(getArchTypeName(Kind));
 }
 
 void Triple::setVendor(VendorType Kind) {
   setVendorName(getVendorTypeName(Kind));
 }
 
 void Triple::setOS(OSType Kind) {
Triple Triple::get32BitArchVariant() const {
 
   case Triple::aarch64:        T.setArch(Triple::arm);     break;
   case Triple::aarch64_be:     T.setArch(Triple::armeb);   break;
   case Triple::amdil64:        T.setArch(Triple::amdil);   break;
   case Triple::hsail64:        T.setArch(Triple::hsail);   break;
   case Triple::le64:           T.setArch(Triple::le32);    break;
   case Triple::mips64:         T.setArch(Triple::mips);    break;
   case Triple::mips64el:       T.setArch(Triple::mipsel);  break;
   case Triple::nvptx64:        T.setArch(Triple::nvptx);   break;
   case Triple::ppc64:          T.setArch(Triple::ppc);     break;
   case Triple::ppc64le:        T.setArch(Triple::ppcle);   break;
   case Triple::renderscript64: T.setArch(Triple::renderscript32); break;
   case Triple::riscv64:        T.setArch(Triple::riscv32); break;
   case Triple::sparcv9:        T.setArch(Triple::sparc);   break;
   case Triple::spir64:         T.setArch(Triple::spir);    break;
   case Triple::wasm64:         T.setArch(Triple::wasm32);  break;
   case Triple::x86_64:         T.setArch(Triple::x86);     break;