Use 32/64 SimdDefaultAlign when AVX/AVX512 is enabled, even on i386.

[ab]

Summary

Per discussion, let's use the features instead of the ABI string.

Original summary:
One problem that came up in D12389 is that i386 doesn't know about the "avx" ABIs. Judging by the commit that originally introduced the X86_64 check and the "avx" ABI (r145652), it was just unnecessary.

Because of that, we can't decide based on the ABI string only for i386.

The only effect this should have is that SimdDefaultAlign would previously always be 128 on i386, no matter the SSE level. We will now use a larger alignment, which seems desirable to me. I added i386 RUN lines to the OpenMP simd test.

I also moved the no-mmx check earlier, as I gather it's necessary for correctness with -mno-mmx (as opposed to the "avx" ABIs which make no difference other than SIMD/vector alignment on i386).

[rjmccall]

This gives no-MMX priority over turning on SSE, which sounds like a major change in behavior to me. There are definitely clients that specifically never want to use MMX but do care deeply about SSE; my understanding is that modern microarchitectures heavily punish casual use of MMX.

[ab]

Unless I'm misunderstanding, I believe this has much less impact than you're thinking; there are three cases:

• x86_64: no change (-mno-mmx is guarded by x86)
• x86, with -mno-mmx: no change (because previously, we'd only set avx/avx512 for x86_64)
• x86, without -mno-mmx: this will use an avx/avx512 ABI string where we'd have used "", letting us use the better alignment (only for OpenMP and vectors, I think).

[rjmccall]

In D12390#234458, @ab wrote:

Unless I'm misunderstanding, I believe this has much less impact than you're thinking; there are three cases:

• x86_64: no change (-mno-mmx is guarded by x86)
• x86, with -mno-mmx: no change (because previously, we'd only set avx/avx512 for x86_64)
• x86, without -mno-mmx: this will use an avx/avx512 ABI string where we'd have used "", letting us use the better alignment (only for OpenMP and vectors, I think).

Ok, I see.

The outcome we want is that things like the max vector alignment should be properly honoring whatever target features are actually enabled. Specifically, if AVX is enabled, we should be setting the max vector alignment to the maximum enabled AVX alignment, regardless of whether MMX is enabled.

This ABI string is an artifact of the interface between the AST and IRGen TargetInfos. We should probably just remove it in favor of some kind of openly-extensible query interface so that IRGen can just directly ask things like whether AVX is enabled instead of parsing some random string. But for this patch, let's just leave it as it is, and instead just set the max vector alignment directly from the target features.

[ab]

r246228 using the features

Thanks for the reviews!

lib/Basic/Targets.cpp

public:
   // initDefaultFeatures which calls this repeatedly.
   static void setFeatureEnabledImpl(llvm::StringMap<bool> &Features,
                                     StringRef Name, bool Enabled);
   void initDefaultFeatures(llvm::StringMap<bool> &Features) const override;
   bool hasFeature(StringRef Feature) const override;
   bool handleTargetFeatures(std::vector<std::string> &Features,
                             DiagnosticsEngine &Diags) override;
   StringRef getABI() const override {
-    if (getTriple().getArch() == llvm::Triple::x86_64 && SSELevel >= AVX512F)
-      return "avx512";
-    else if (getTriple().getArch() == llvm::Triple::x86_64 && SSELevel >= AVX)
-      return "avx";
-    else if (getTriple().getArch() == llvm::Triple::x86 &&
-             MMX3DNowLevel == NoMMX3DNow)
+    if (getTriple().getArch() == llvm::Triple::x86 &&
       return "no-mmx";
+    else if (SSELevel >= AVX512F)
+      return "avx512";
+    else if (SSELevel >= AVX)
+      return "avx";
     return "";
   }
   bool setCPU(const std::string &Name) override {
     CPU = llvm::StringSwitch<CPUKind>(Name)
       .Case("i386", CK_i386)
       .Case("i486", CK_i486)
       .Case("winchip-c6", CK_WinChipC6)
       .Case("winchip2", CK_WinChip2)

test/OpenMP/simd_metadata.c

 // RUN: %clang_cc1 -fopenmp -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86
 // RUN: %clang_cc1 -fopenmp -triple x86_64-unknown-unknown -target-feature +avx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX
 // RUN: %clang_cc1 -fopenmp -triple x86_64-unknown-unknown -target-feature +avx512f -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX512
+// RUN: %clang_cc1 -fopenmp -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86
+// RUN: %clang_cc1 -fopenmp -triple i386-unknown-unknown -target-feature +avx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX
+// RUN: %clang_cc1 -fopenmp -triple i386-unknown-unknown -target-feature +avx512f -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=X86-AVX512
 // RUN: %clang_cc1 -fopenmp -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=PPC
 // RUN: %clang_cc1 -fopenmp -triple powerpc64-unknown-unknown -target-abi elfv1-qpx -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=PPC-QPX
 
 void h1(float *c, float *a, double b[], int size)
 {
 // CHECK-LABEL: define void @h1
   int t = 0;
 #pragma omp simd safelen(16) linear(t) aligned(c:32) aligned(a,b)