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

1432

Shouldn't all others also preserve the subarch?

[wzssyqa]

llvm/lib/Support/Triple.cpp

1432

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/include/llvm/ADT/Triple.h

public:
   const std::string &str() const { return Data; }
 
   const std::string &getTriple() const { return Data; }
 
   /// getArchName - Get the architecture (first) component of the
   /// triple.
   StringRef getArchName() const;
 
+  /// getArchName - Get the architecture name based on Kind and SubArch.
+  StringRef getArchName(ArchType Kind, SubArchType SubArch = NoSubArch) const;
+
   /// getVendorName - Get the vendor (second) component of the triple.
   StringRef getVendorName() const;
 
   /// getOSName - Get the operating system (third) component of the
   /// triple.
   StringRef getOSName() const;
 
   /// getEnvironmentName - Get the optional environment (fourth)
public:
   bool hasDLLImportExport() const { return isOSWindows() || isPS4CPU(); }
 
   /// @}
   /// @name Mutators
   /// @{
 
   /// setArch - Set the architecture (first) component of the triple
   /// to a known type.
-  void setArch(ArchType Kind);
+  void setArch(ArchType Kind, SubArchType SubArch = NoSubArch);
 
   /// setVendor - Set the vendor (second) component of the triple to a
   /// known type.
   void setVendor(VendorType Kind);
 
   /// setOS - Set the operating system (third) component of the triple
   /// to a known type.
   void setOS(OSType Kind);

llvm/lib/Support/Triple.cpp

std::string Triple::normalize(StringRef Str) {
   // Stick the corrected components back together to form the normalized string.
   return join(Components, "-");
 }
 
 StringRef Triple::getArchName() const {
   return StringRef(Data).split('-').first;           // Isolate first component
 }
 
+StringRef Triple::getArchName(ArchType Kind, SubArchType SubArch) const {
+  switch (Kind) {
+  case Triple::mips:
+    if (SubArch == MipsSubArch_r6)
+      return "mipsisa32r6";
+    break;
+  case Triple::mipsel:
+    if (SubArch == MipsSubArch_r6)
+      return "mipsisa32r6el";
+    break;
+  case Triple::mips64:
+    if (SubArch == MipsSubArch_r6)
+      return "mipsisa64r6";
+    break;
+  case Triple::mips64el:
+    if (SubArch == MipsSubArch_r6)
+      return "mipsisa64r6el";
+    break;
+  default:
+    break;
+  }
+  return getArchTypeName(Kind);
+}
+
 StringRef Triple::getVendorName() const {
   StringRef Tmp = StringRef(Data).split('-').second; // Strip first component
   return Tmp.split('-').first;                       // Isolate second component
 }
 
 StringRef Triple::getOSName() const {
   StringRef Tmp = StringRef(Data).split('-').second; // Strip first component
   Tmp = Tmp.split('-').second;                       // Strip second component
case IOS:
     llvm_unreachable("conflicting triple info");
   }
 }
 
 void Triple::setTriple(const Twine &Str) {
   *this = Triple(Str);
 }
 
-void Triple::setArch(ArchType Kind) {
-  setArchName(getArchTypeName(Kind));
+void Triple::setArch(ArchType Kind, SubArchType SubArch) {
+  setArchName(getArchName(Kind, SubArch));
 }
 
 void Triple::setVendor(VendorType Kind) {
   setVendorName(getVendorTypeName(Kind));
 }
 
 void Triple::setOS(OSType Kind) {
   setOSName(getOSTypeName(Kind));
case Triple::xcore:
     // Already 32-bit.
     break;
 
   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::mips64:
+    T.setArch(Triple::mips, getSubArch());
+    break;
+  case Triple::mips64el:
+    T.setArch(Triple::mipsel, getSubArch());
+    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:
     break;
 
   case Triple::aarch64_32:      T.setArch(Triple::aarch64);    break;
   case Triple::amdil:           T.setArch(Triple::amdil64);    break;
   case Triple::arm:             T.setArch(Triple::aarch64);    break;
   case Triple::armeb:           T.setArch(Triple::aarch64_be); break;
   case Triple::hsail:           T.setArch(Triple::hsail64);    break;
   case Triple::le32:            T.setArch(Triple::le64);       break;
-  case Triple::mips:            T.setArch(Triple::mips64);     break;
-  case Triple::mipsel:          T.setArch(Triple::mips64el);   break;
+  case Triple::mips:
+    T.setArch(Triple::mips64, getSubArch());
+    break;
+  case Triple::mipsel:
+    T.setArch(Triple::mips64el, getSubArch());
+    break;
   case Triple::nvptx:           T.setArch(Triple::nvptx64);    break;
   case Triple::ppc:             T.setArch(Triple::ppc64);      break;
   case Triple::ppcle:           T.setArch(Triple::ppc64le);    break;
   case Triple::renderscript32:  T.setArch(Triple::renderscript64);     break;
   case Triple::riscv32:         T.setArch(Triple::riscv64);    break;
   case Triple::sparc:           T.setArch(Triple::sparcv9);    break;
   case Triple::spir:            T.setArch(Triple::spir64);     break;
   case Triple::thumb:           T.setArch(Triple::aarch64);    break;
Triple Triple::getBigEndianArchVariant() const {
   // drop any arch suffixes.
   case Triple::arm:
   case Triple::thumb:
     T.setArch(UnknownArch);
     break;
 
   case Triple::aarch64: T.setArch(Triple::aarch64_be); break;
   case Triple::bpfel:   T.setArch(Triple::bpfeb);      break;
-  case Triple::mips64el:T.setArch(Triple::mips64);     break;
-  case Triple::mipsel:  T.setArch(Triple::mips);       break;
+  case Triple::mips64el:
+    T.setArch(Triple::mips64, getSubArch());
+    break;
+  case Triple::mipsel:
+    T.setArch(Triple::mips, getSubArch());
+    break;
   case Triple::ppcle:   T.setArch(Triple::ppc);        break;
   case Triple::ppc64le: T.setArch(Triple::ppc64);      break;
   case Triple::sparcel: T.setArch(Triple::sparc);      break;
   case Triple::tcele:   T.setArch(Triple::tce);        break;
   default:
     llvm_unreachable("getBigEndianArchVariant: unknown triple.");
   }
   return T;
Triple Triple::getLittleEndianArchVariant() const {
   // drop any arch suffixes.
   case Triple::armeb:
   case Triple::thumbeb:
     T.setArch(UnknownArch);
     break;
 
   case Triple::aarch64_be: T.setArch(Triple::aarch64);  break;
   case Triple::bpfeb:      T.setArch(Triple::bpfel);    break;
-  case Triple::mips64:     T.setArch(Triple::mips64el); break;
-  case Triple::mips:       T.setArch(Triple::mipsel);   break;
+  case Triple::mips64:
+    T.setArch(Triple::mips64el, getSubArch());
+    break;
+  case Triple::mips:
+    T.setArch(Triple::mipsel, getSubArch());
+    break;
   case Triple::ppc:        T.setArch(Triple::ppcle);    break;
   case Triple::ppc64:      T.setArch(Triple::ppc64le);  break;
   case Triple::sparc:      T.setArch(Triple::sparcel);  break;
   case Triple::tce:        T.setArch(Triple::tcele);    break;
   default:
     llvm_unreachable("getLittleEndianArchVariant: unknown triple.");
   }
   return T;

llvm/unittests/ADT/TripleTest.cpp

TEST(TripleTest, BitWidthArchVariants) {
   EXPECT_EQ(Triple::UnknownArch, T.get64BitArchVariant().getArch());
 
   T.setArch(Triple::UnknownArch);
   EXPECT_EQ(Triple::UnknownArch, T.get32BitArchVariant().getArch());
   EXPECT_EQ(Triple::UnknownArch, T.get64BitArchVariant().getArch());
 
   T.setArch(Triple::mips);
   EXPECT_EQ(Triple::mips, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get32BitArchVariant().getSubArch());
   EXPECT_EQ(Triple::mips64, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get64BitArchVariant().getSubArch());
+
+  T.setArch(Triple::mips, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mips, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get32BitArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mips64, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get64BitArchVariant().getSubArch());
 
   T.setArch(Triple::mipsel);
   EXPECT_EQ(Triple::mipsel, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get32BitArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mips64el, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get64BitArchVariant().getSubArch());
+
+  T.setArch(Triple::mipsel, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mipsel, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get32BitArchVariant().getSubArch());
   EXPECT_EQ(Triple::mips64el, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get64BitArchVariant().getSubArch());
 
   T.setArch(Triple::ppc);
   EXPECT_EQ(Triple::ppc, T.get32BitArchVariant().getArch());
   EXPECT_EQ(Triple::ppc64, T.get64BitArchVariant().getArch());
 
   T.setArch(Triple::nvptx);
   EXPECT_EQ(Triple::nvptx, T.get32BitArchVariant().getArch());
   EXPECT_EQ(Triple::nvptx64, T.get64BitArchVariant().getArch());
 
   T.setArch(Triple::sparc);
   EXPECT_EQ(Triple::sparc, T.get32BitArchVariant().getArch());
   EXPECT_EQ(Triple::sparcv9, T.get64BitArchVariant().getArch());
 
   T.setArch(Triple::x86);
   EXPECT_EQ(Triple::x86, T.get32BitArchVariant().getArch());
   EXPECT_EQ(Triple::x86_64, T.get64BitArchVariant().getArch());
 
   T.setArch(Triple::mips64);
   EXPECT_EQ(Triple::mips, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get32BitArchVariant().getSubArch());
   EXPECT_EQ(Triple::mips64, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get64BitArchVariant().getSubArch());
+
+  T.setArch(Triple::mips64, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mips, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get32BitArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mips64, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get64BitArchVariant().getSubArch());
 
   T.setArch(Triple::mips64el);
   EXPECT_EQ(Triple::mipsel, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get32BitArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mips64el, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.get64BitArchVariant().getSubArch());
+
+  T.setArch(Triple::mips64el, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mipsel, T.get32BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get32BitArchVariant().getSubArch());
   EXPECT_EQ(Triple::mips64el, T.get64BitArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.get64BitArchVariant().getSubArch());
 
   T.setArch(Triple::ppc64);
   EXPECT_EQ(Triple::ppc, T.get32BitArchVariant().getArch());
   EXPECT_EQ(Triple::ppc64, T.get64BitArchVariant().getArch());
 
   T.setArch(Triple::nvptx64);
   EXPECT_EQ(Triple::nvptx, T.get32BitArchVariant().getArch());
   EXPECT_EQ(Triple::nvptx64, T.get64BitArchVariant().getArch());
TEST(TripleTest, EndianArchVariants) {
   EXPECT_EQ(Triple::bpfel, T.getLittleEndianArchVariant().getArch());
 
   T.setArch(Triple::bpfel);
   EXPECT_EQ(Triple::bpfeb, T.getBigEndianArchVariant().getArch());
   EXPECT_EQ(Triple::bpfel, T.getLittleEndianArchVariant().getArch());
 
   T.setArch(Triple::mips64);
   EXPECT_EQ(Triple::mips64, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getBigEndianArchVariant().getSubArch());
   EXPECT_EQ(Triple::mips64el, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getLittleEndianArchVariant().getSubArch());
+
+  T.setArch(Triple::mips64, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mips64, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.getBigEndianArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mips64el, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6,
+            T.getLittleEndianArchVariant().getSubArch());
 
   T.setArch(Triple::mips64el);
   EXPECT_EQ(Triple::mips64, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getBigEndianArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mips64el, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getLittleEndianArchVariant().getSubArch());
+
+  T.setArch(Triple::mips64el, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mips64, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.getBigEndianArchVariant().getSubArch());
   EXPECT_EQ(Triple::mips64el, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6,
+            T.getLittleEndianArchVariant().getSubArch());
 
   T.setArch(Triple::mips);
   EXPECT_EQ(Triple::mips, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getBigEndianArchVariant().getSubArch());
   EXPECT_EQ(Triple::mipsel, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getLittleEndianArchVariant().getSubArch());
+
+  T.setArch(Triple::mips, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mips, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.getBigEndianArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mipsel, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6,
+            T.getLittleEndianArchVariant().getSubArch());
 
   T.setArch(Triple::mipsel);
   EXPECT_EQ(Triple::mips, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getBigEndianArchVariant().getSubArch());
+  EXPECT_EQ(Triple::mipsel, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::NoSubArch, T.getLittleEndianArchVariant().getSubArch());
+
+  T.setArch(Triple::mipsel, Triple::MipsSubArch_r6);
+  EXPECT_EQ(Triple::mips, T.getBigEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6, T.getBigEndianArchVariant().getSubArch());
   EXPECT_EQ(Triple::mipsel, T.getLittleEndianArchVariant().getArch());
+  EXPECT_EQ(Triple::MipsSubArch_r6,
+            T.getLittleEndianArchVariant().getSubArch());
 
   T.setArch(Triple::ppc);
   EXPECT_EQ(Triple::ppc, T.getBigEndianArchVariant().getArch());
   EXPECT_EQ(Triple::ppcle, T.getLittleEndianArchVariant().getArch());
 
   T.setArch(Triple::ppc64);
   EXPECT_EQ(Triple::ppc64, T.getBigEndianArchVariant().getArch());
   EXPECT_EQ(Triple::ppc64le, T.getLittleEndianArchVariant().getArch());