Enable fma and f16c features when avx512f is enabled.

[ab]

Summary

Since, according to the ISA extension manual, (EVEX versions of) the FMA3/F16C instructions are a core part of AVX512F, it seems to me we should enable the features when avx512f is enabled (e.g., -mavx512f). If not, users need to either specify a specific CPU, or add the redundant -mfma/-mf16c.

[ab]

Also, we should probably do the same for LLVM; if that sounds good I'll submit another patch.

Thanks!

[craig.topper]

Are the 512-bit intrinsics in clang's headers qualified with F16C and FMA
defines? They don't seem to be in gcc's headers. And gcc doesn't seem to
auto enable FMA or F16C when avx512 is specified.

Similar question for the instructions in llvm.

[ab]

In D10617#192429, @craig.topper wrote:

Are the 512-bit intrinsics in clang's headers qualified with F16C and FMA
defines? They don't seem to be in gcc's headers. And gcc doesn't seem to
auto enable FMA or F16C when avx512 is specified.

Similar question for the instructions in llvm.

They're not?

But, if that's what you're getting at: if the defines have a 1-to-1 mapping with the intrinsics/headers (I think they do?), then on second thought, this isn't the right fix: +avx512f says nothing about whether we support FMA/F16C intrinsics, only the AVX512F ones.

Then the actual issue here is that, for fma and f16c, LLVM codegen relies on +fma/+f16c only to mark say f32 ISD::FMA as legal, and generate X86ISD::FMADD. Both of which are necessary to generate even the AVX512F instruction, and this makes it impossible with +avx512f alone.
Does that make sense? If so I'll post a patch!

-Ahmed

[craig.topper]

I think that makes sense. We need to enable ISD::FMA for the smaller types
when AVX512 is enabled. Probably need to add AVX512 in PerformFMACombine
and isFMAFasterThanFMulAndFAdd as well.

[ab]

Yep, simple enough: r240616, let me know if I missed anything.

The 213->231 xform, I'll leave for another day, since we can't
currently disassemble the 231/132 variants: PR23937.
Also, F16C is a wee bit trickier, and not just about flags: PR23941.

-Ahmed

lib/Basic/Targets.cpp

case CK_Silvermont:
     setFeatureEnabledImpl(Features, "aes", true);
     setFeatureEnabledImpl(Features, "pclmul", true);
     // FALLTHROUGH
   case CK_Nehalem:
     setFeatureEnabledImpl(Features, "sse4.2", true);
     setFeatureEnabledImpl(Features, "cx16", true);
     break;
   case CK_KNL:
+    // avx512f implies fma and f16c.
     setFeatureEnabledImpl(Features, "avx512f", true);
     setFeatureEnabledImpl(Features, "avx512cd", true);
     setFeatureEnabledImpl(Features, "avx512er", true);
     setFeatureEnabledImpl(Features, "avx512pf", true);
     setFeatureEnabledImpl(Features, "rdseed", true);
     setFeatureEnabledImpl(Features, "adx", true);
     setFeatureEnabledImpl(Features, "lzcnt", true);
     setFeatureEnabledImpl(Features, "bmi", true);
     setFeatureEnabledImpl(Features, "bmi2", true);
     setFeatureEnabledImpl(Features, "rtm", true);
-    setFeatureEnabledImpl(Features, "fma", true);
     setFeatureEnabledImpl(Features, "rdrnd", true);
-    setFeatureEnabledImpl(Features, "f16c", true);
     setFeatureEnabledImpl(Features, "fsgsbase", true);
     setFeatureEnabledImpl(Features, "aes", true);
     setFeatureEnabledImpl(Features, "pclmul", true);
     setFeatureEnabledImpl(Features, "cx16", true);
     break;
   case CK_K6_2:
   case CK_K6_3:
   case CK_WinChip2:

 }
 
 void X86TargetInfo::setSSELevel(llvm::StringMap<bool> &Features,
                                 X86SSEEnum Level, bool Enabled) {
   if (Enabled) {
     switch (Level) {
     case AVX512F:
       Features["avx512f"] = true;
+      Features["fma"] = true;
+      Features["f16c"] = true;
     case AVX2:
       Features["avx2"] = true;
     case AVX:
       Features["avx"] = true;
     case SSE42:
       Features["sse4.2"] = true;
     case SSE41:
       Features["sse4.1"] = true;
void X86TargetInfo::setSSELevel(llvm::StringMap<bool> &Features,
   case AVX:
     Features["fma"] = Features["avx"] = Features["f16c"] = false;
     setXOPLevel(Features, FMA4, false);
   case AVX2:
     Features["avx2"] = false;
   case AVX512F:
     Features["avx512f"] = Features["avx512cd"] = Features["avx512er"] =
       Features["avx512pf"] = Features["avx512dq"] = Features["avx512bw"] =
-      Features["avx512vl"] = false;
+      Features["avx512vl"] = Features["fma"] = Features["f16c"] = false;
   }
 }
 
 void X86TargetInfo::setMMXLevel(llvm::StringMap<bool> &Features,
                                 MMX3DNowEnum Level, bool Enabled) {
   if (Enabled) {
     switch (Level) {
     case AMD3DNowAthlon:

test/Preprocessor/x86_target_features.c

 // SSE4A: #define __SSE__ 1
 // SSE4A: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512f -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512F %s
 
 // AVX512F: #define __AVX2__ 1
 // AVX512F: #define __AVX512F__ 1
 // AVX512F: #define __AVX__ 1
+// AVX512F: #define __F16C__ 1
+// AVX512F: #define __FMA__ 1
 // AVX512F: #define __SSE2_MATH__ 1
 // AVX512F: #define __SSE2__ 1
 // AVX512F: #define __SSE3__ 1
 // AVX512F: #define __SSE4_1__ 1
 // AVX512F: #define __SSE4_2__ 1
 // AVX512F: #define __SSE_MATH__ 1
 // AVX512F: #define __SSE__ 1
 // AVX512F: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512cd -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512CD %s
 
 // AVX512CD: #define __AVX2__ 1
 // AVX512CD: #define __AVX512CD__ 1
 // AVX512CD: #define __AVX512F__ 1
 // AVX512CD: #define __AVX__ 1
+// AVX512CD: #define __F16C__ 1
+// AVX512CD: #define __FMA__ 1
 // AVX512CD: #define __SSE2_MATH__ 1
 // AVX512CD: #define __SSE2__ 1
 // AVX512CD: #define __SSE3__ 1
 // AVX512CD: #define __SSE4_1__ 1
 // AVX512CD: #define __SSE4_2__ 1
 // AVX512CD: #define __SSE_MATH__ 1
 // AVX512CD: #define __SSE__ 1
 // AVX512CD: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512er -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512ER %s
 
 // AVX512ER: #define __AVX2__ 1
 // AVX512ER: #define __AVX512ER__ 1
 // AVX512ER: #define __AVX512F__ 1
 // AVX512ER: #define __AVX__ 1
+// AVX512ER: #define __F16C__ 1
+// AVX512ER: #define __FMA__ 1
 // AVX512ER: #define __SSE2_MATH__ 1
 // AVX512ER: #define __SSE2__ 1
 // AVX512ER: #define __SSE3__ 1
 // AVX512ER: #define __SSE4_1__ 1
 // AVX512ER: #define __SSE4_2__ 1
 // AVX512ER: #define __SSE_MATH__ 1
 // AVX512ER: #define __SSE__ 1
 // AVX512ER: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512pf -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512PF %s
 
 // AVX512PF: #define __AVX2__ 1
 // AVX512PF: #define __AVX512F__ 1
 // AVX512PF: #define __AVX512PF__ 1
 // AVX512PF: #define __AVX__ 1
+// AVX512PF: #define __F16C__ 1
+// AVX512PF: #define __FMA__ 1
 // AVX512PF: #define __SSE2_MATH__ 1
 // AVX512PF: #define __SSE2__ 1
 // AVX512PF: #define __SSE3__ 1
 // AVX512PF: #define __SSE4_1__ 1
 // AVX512PF: #define __SSE4_2__ 1
 // AVX512PF: #define __SSE_MATH__ 1
 // AVX512PF: #define __SSE__ 1
 // AVX512PF: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512dq -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512DQ %s
 
 // AVX512DQ: #define __AVX2__ 1
 // AVX512DQ: #define __AVX512DQ__ 1
 // AVX512DQ: #define __AVX512F__ 1
 // AVX512DQ: #define __AVX__ 1
+// AVX512DQ: #define __F16C__ 1
+// AVX512DQ: #define __FMA__ 1
 // AVX512DQ: #define __SSE2_MATH__ 1
 // AVX512DQ: #define __SSE2__ 1
 // AVX512DQ: #define __SSE3__ 1
 // AVX512DQ: #define __SSE4_1__ 1
 // AVX512DQ: #define __SSE4_2__ 1
 // AVX512DQ: #define __SSE_MATH__ 1
 // AVX512DQ: #define __SSE__ 1
 // AVX512DQ: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512bw -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512BW %s
 
 // AVX512BW: #define __AVX2__ 1
 // AVX512BW: #define __AVX512BW__ 1
 // AVX512BW: #define __AVX512F__ 1
 // AVX512BW: #define __AVX__ 1
+// AVX512BW: #define __F16C__ 1
+// AVX512BW: #define __FMA__ 1
 // AVX512BW: #define __SSE2_MATH__ 1
 // AVX512BW: #define __SSE2__ 1
 // AVX512BW: #define __SSE3__ 1
 // AVX512BW: #define __SSE4_1__ 1
 // AVX512BW: #define __SSE4_2__ 1
 // AVX512BW: #define __SSE_MATH__ 1
 // AVX512BW: #define __SSE__ 1
 // AVX512BW: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512vl -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512VL %s
 
 // AVX512VL: #define __AVX2__ 1
 // AVX512VL: #define __AVX512F__ 1
 // AVX512VL: #define __AVX512VL__ 1
 // AVX512VL: #define __AVX__ 1
+// AVX512VL: #define __F16C__ 1
+// AVX512VL: #define __FMA__ 1
 // AVX512VL: #define __SSE2_MATH__ 1
 // AVX512VL: #define __SSE2__ 1
 // AVX512VL: #define __SSE3__ 1
 // AVX512VL: #define __SSE4_1__ 1
 // AVX512VL: #define __SSE4_2__ 1
 // AVX512VL: #define __SSE_MATH__ 1
 // AVX512VL: #define __SSE__ 1
 // AVX512VL: #define __SSSE3__ 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mavx512pf -mno-avx512f -x c -E -dM -o - %s | FileCheck --check-prefix=AVX512F2 %s
 
 // AVX512F2: #define __AVX2__ 1
 // AVX512F2-NOT: #define __AVX512F__ 1
 // AVX512F2-NOT: #define __AVX512PF__ 1
+// AVX512F2-NOT: #define __F16C__ 1
+// AVX512F2-NOT: #define __FMA__ 1
 // AVX512F2: #define __AVX__ 1
 // AVX512F2: #define __SSE2_MATH__ 1
 // AVX512F2: #define __SSE2__ 1
 // AVX512F2: #define __SSE3__ 1
 // AVX512F2: #define __SSE4_1__ 1
 // AVX512F2: #define __SSE4_2__ 1
 // AVX512F2: #define __SSE_MATH__ 1
 // AVX512F2: #define __SSE__ 1