ADT: handle special case of ARM environment for SuSE

[compnerd]

Summary

SuSE treats "gnueabi" as "gnueabihf" so make sure that we normalise the environment.

[ismail]

This looks good but please fix the typo in comments: SuSE -> SUSE
Also please use the correct triple: armv7hl-suse-linux-gnueabi (at least in the second test I think.)

Also the vendor test in TripleTest.cpp should be updated.

[rengolin]

This could have side-effects, too, especially when going through textual IR, no?

[compnerd]

Yes, the idea is that the textual IR uses the correct triple, unless it is overidden at the command line.

[majnemer]

In D32460#740134, @compnerd wrote:

Yes, the idea is that the textual IR uses the correct triple, unless it is overidden at the command line.

Why not just handle this in clang?

[compnerd]

@majnemer thats the idea :-). Passing either triple to clang would give the right behaviour (clang will normalize the triple before passing it to cc1).

[rengolin]

In D32460#740135, @majnemer wrote:

In D32460#740134, @compnerd wrote:

Yes, the idea is that the textual IR uses the correct triple, unless it is overidden at the command line.

Why not just handle this in clang?

IIRC, the problem here is of consistency.

The Triple usage throughout LLVM is broken wrt float ABI, but "hf" at the end of the environment "makes it work". The reason being that "gnueabihf" is the mostly tested environment, so it converged to work by accident.

The target that clang passes to LLVM is, therefore, also broken. Making Clang change from "eabi" to "eabihf" would "work", in the sense that in some cases, the correct float ABI would be selected. But it would also serialise the *wrong* triple in IR.

Again, that solution has "converged" to work, because the IR triple doesn't have to make sense (for example "thumbv7a"), so it's possible that, for all intended purposes, just changing the environment in the clang driver should be enough.

If it wasn't for the fact, of course, that we already do that with Arch/CPU names, sometimes getting it terrible wrong, and most times "fixing" it with by "accidental convergence".

I'm not against continuing this pattern, as I certainly don't have time (and health) to try and fix the clang driver for the Nth time, but I just want to make clear to all parties involved that none of that is a "solution". :)

The real solution would be to continue Daniel's Triple refactoring [1] until all fields are uniquely represented on both Clang and LLVM.

</rant>

[1] Some references:

• https://reviews.llvm.org/D10969
• http://lists.llvm.org/pipermail/llvm-dev/2015-July/087700.html

[ismail]

Can we go ahead with this?

[compnerd]

@majnemer thats the idea - passing either triple to clang would give the right behaviour (clang will normalize the triple before passing it to cc1).
@ismail Im waiting on you to accept the change

[ismail]

In D32460#750331, @compnerd wrote:

@ismail Im waiting on you to accept the change

I requested two changes, does that makes sense?

[ismail]

@compnerd ping?

[compnerd]

@ismail the current version of the patch should have those requested changes :-).

[ismail]

Thank you!

[compnerd]

SVN r304670

lib/Support/Triple.cpp

     }
   }
 
+  // SuSE uses "gnueabi" to mean "gnueabihf"
+  if (Vendor == Triple::SUSE && Environment == llvm::Triple::GNUEABI)
+    Components[3] = "gnueabihf";
+
   if (OS == Triple::Win32) {
     Components.resize(4);
     Components[2] = "windows";

unittests/ADT/TripleTest.cpp

   EXPECT_EQ("armv6eb--netbsd-eabihf", Triple::normalize("armv6eb-netbsd-eabihf"));
   EXPECT_EQ("armv7eb--netbsd-eabihf", Triple::normalize("armv7eb-netbsd-eabihf"));
 
+  EXPECT_EQ("armv7-suse-linux-gnueabihf",
+            Triple::normalize("armv7-suse-linux-gnueabi"));
+
   Triple T;
   T = Triple("armv6--netbsd-eabi");
   EXPECT_EQ(Triple::arm, T.getArch());
   T = Triple("armv6eb--netbsd-eabi");
   EXPECT_EQ(Triple::armeb, T.getArch());
+  T = Triple("armv7-suse-linux-gnueabihf");
+  EXPECT_EQ(Triple::GNUEABIHF, T.getEnvironment());
 }
 
 TEST(TripleTest, ParseARMArch) {