[X86] use macros to split GFNI intrinsics into different kinds

[FreddyYe]

Summary

Tremont microarchitecture only has GFNI(SSE) version, not AVX and
AVX512 version. This patch is to avoid compiling fail on Windows when
using -march=tremont to invoke one of GFNI(SSE) intrinsic.

[craig.topper]

clang/lib/Headers/gfniintrin.h

35

This needs to be more like

#if !(defined(_MSC_VER) || defined(SCE)) || __has_feature(modules) || \

(defined(__AVXVL__)  && define(__AVX512BW__))

We want all intrinsics to be defined for when not compiling for MSVC compatibility. Otherwise using attribute(target) doesn't work.

[craig.topper]

What compilation failure do you get without this? The feature checks are compile time optimization for MSVC because windows.h includes intrin.h.

[FreddyYe]

The fails are all unknown type errors on Windows, since those typedefs are declared in other header files.
The error message goes like:

$ clang -march=tremont gfni.c
......
...\lib\clang\12.0.0\include\gfniintrin.h:129:37: error:
      unknown type name '__mmask16'
_mm_mask_gf2p8mul_epi8(__m128i __S, __mmask16 __U, __m128i __A, __m128i __B)
                                    ^
...\lib\clang\12.0.0\include\gfniintrin.h:137:25: error:
      unknown type name '__mmask16'
_mm_maskz_gf2p8mul_epi8(__mmask16 __U, __m128i __A, __m128i __B)
                        ^
......
...\lib\clang\12.0.0\include\gfniintrin.h:159:43: error:
      unknown type name '__m256i'
_mm256_maskz_gf2p8mul_epi8(__mmask32 __U, __m256i __A, __m256i __B)
                                          ^
...
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.

[craig.topper]

In D90822#2375372, @FreddyYe wrote:

The fails are all unknown type errors on Windows, since those typedefs are declared in other header files.
The error message goes like:

$ clang -march=tremont gfni.c
......
...\lib\clang\12.0.0\include\gfniintrin.h:129:37: error:
      unknown type name '__mmask16'
_mm_mask_gf2p8mul_epi8(__m128i __S, __mmask16 __U, __m128i __A, __m128i __B)
                                    ^
...\lib\clang\12.0.0\include\gfniintrin.h:137:25: error:
      unknown type name '__mmask16'
_mm_maskz_gf2p8mul_epi8(__mmask16 __U, __m128i __A, __m128i __B)
                        ^
......
...\lib\clang\12.0.0\include\gfniintrin.h:159:43: error:
      unknown type name '__m256i'
_mm256_maskz_gf2p8mul_epi8(__mmask32 __U, __m256i __A, __m256i __B)
                                          ^
...
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.

Ok that makes sense. Maybe the easiest fix is just check that AVXINTRIN_H or AVX512BWINTRIN_H or __AVX512BWVLINTRIN_H is defined where there's a dependency.

[pengfei]

Craig's method sounds good.
@FreddyYe , Why we check AVX512BW instead of AVX512F. I saw SDM says it depends on AVX512F.

[pengfei]

In D90822#2375423, @pengfei wrote:

Craig's method sounds good.

I think it may result in potential problem once we change the order of them in immintrin.h, it should be better to use the first method.

#if !(defined(_MSC_VER) || defined(SCE)) || __has_feature(modules) || \
    (defined(__AVX512VL__)  && define(__AVX512BW__))

[craig.topper]

In D90822#2375425, @pengfei wrote:

In D90822#2375423, @pengfei wrote:

Craig's method sounds good.

I think it may result in potential problem once we change the order of them in immintrin.h, it should be better to use the first method.

#if !(defined(_MSC_VER) || defined(SCE)) || __has_feature(modules) || \
    (defined(__AVX512VL__)  && define(__AVX512BW__))

But if the order changes then the code won't compile because _m512i or some other type won't be defined.

[FreddyYe]

In D90822#2375423, @pengfei wrote:

Craig's method sounds good.
@FreddyYe , Why we check AVX512BW instead of AVX512F. I saw SDM says it depends on AVX512F.

I was referring to the old implement and test case. Seems like a error introduced before.

[pengfei]

In D90822#2375445, @craig.topper wrote:

In D90822#2375425, @pengfei wrote:

In D90822#2375423, @pengfei wrote:

Craig's method sounds good.

I think it may result in potential problem once we change the order of them in immintrin.h, it should be better to use the first method.

#if !(defined(_MSC_VER) || defined(SCE)) || __has_feature(modules) || \
    (defined(__AVX512VL__)  && define(__AVX512BW__))

But if the order changes then the code won't compile because _m512i or some other type won't be defined.

I was thinking we may lose the chance to know we change them in wrong order. But it should be OK since we have intrinsic tests. So I agree with you.

[craig.topper]

In D90822#2375463, @FreddyYe wrote:

In D90822#2375423, @pengfei wrote:

Craig's method sounds good.
@FreddyYe , Why we check AVX512BW instead of AVX512F. I saw SDM says it depends on AVX512F.

I was referring to the old implement and test case. Seems like a error introduced before.

mmask64 requires avx512bw. And 32xi16 and 64xi8 aren’t well supported without avx512bw. We used to split them always, but we only split specific instructions now.

We also need avx512bw for the selectb and selectw builtins used for masking.

[pengfei]

In D90822#2375469, @craig.topper wrote:

In D90822#2375463, @FreddyYe wrote:

In D90822#2375423, @pengfei wrote:

Craig's method sounds good.
@FreddyYe , Why we check AVX512BW instead of AVX512F. I saw SDM says it depends on AVX512F.

I was referring to the old implement and test case. Seems like a error introduced before.

mmask64 requires avx512bw. And 32xi16 and 64xi8 aren’t well supported without avx512bw. We used to split them always, but we only split specific instructions now.

We also need avx512bw for the selectb and selectw builtins used for masking.

I see. Thank you!

[FreddyYe]

Use header file macros instead.

[pengfei]

clang/lib/Headers/gfniintrin.h

36

__AVX512VLBWINTRIN_H is better.

57–64

missing comments

184

comments

[FreddyYe]

Reorganize intrinsic orders to avoid using nested macros.

[pengfei]

clang/lib/Headers/gfniintrin.h

72–79

These 2 functions need to move to __AVX512BWINTRIN_H

154–155

These 2 functions need to move to __AVX512BWINTRIN_H

183–184

These 2 functions need to move to __AVX512BWINTRIN_H

[FreddyYe]

clang/lib/Headers/gfniintrin.h

154–155

Thanks for review!

[FreddyYe]

Refine

[craig.topper]

LGTM

[pengfei]

LGTM. Thanks.

clang/lib/Headers/gfniintrin.h

 /*===----------------- gfniintrin.h - GFNI intrinsics ----------------------===
  *
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  *
  *===-----------------------------------------------------------------------===
  */
 #ifndef __IMMINTRIN_H
 #error "Never use <gfniintrin.h> directly; include <immintrin.h> instead."
 #endif
 
 #ifndef __GFNIINTRIN_H
 #define __GFNIINTRIN_H
 
+/* Default attributes for simple form (no masking). */
+#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("gfni"), __min_vector_width__(128)))
+
+/* Default attributes for YMM unmasked form. */
+#define __DEFAULT_FN_ATTRS_Y __attribute__((__always_inline__, __nodebug__, __target__("avx,gfni"), __min_vector_width__(256)))
+
+/* Default attributes for ZMM forms. */
+#define __DEFAULT_FN_ATTRS_Z __attribute__((__always_inline__, __nodebug__, __target__("avx512bw,gfni"), __min_vector_width__(512)))
+
+/* Default attributes for VLX forms. */
+#define __DEFAULT_FN_ATTRS_VL128 __attribute__((__always_inline__, __nodebug__, __target__("avx512bw,avx512vl,gfni"), __min_vector_width__(128)))
+#define __DEFAULT_FN_ATTRS_VL256 __attribute__((__always_inline__, __nodebug__, __target__("avx512bw,avx512vl,gfni"), __min_vector_width__(256)))
 
 #define _mm_gf2p8affineinv_epi64_epi8(A, B, I) \
   (__m128i)__builtin_ia32_vgf2p8affineinvqb_v16qi((__v16qi)(__m128i)(A),          \
                                                   (__v16qi)(__m128i)(B),          \
                                                   (char)(I))
 
-#define _mm_mask_gf2p8affineinv_epi64_epi8(S, U, A, B, I) \
-  (__m128i)__builtin_ia32_selectb_128((__mmask16)(U),                             \
-        (__v16qi)_mm_gf2p8affineinv_epi64_epi8(A, B, I),                          \
-        (__v16qi)(__m128i)(S))
-
-
-#define _mm_maskz_gf2p8affineinv_epi64_epi8(U, A, B, I) \
-  (__m128i)_mm_mask_gf2p8affineinv_epi64_epi8((__m128i)_mm_setzero_si128(),       \
-        U, A, B, I)
+#define _mm_gf2p8affine_epi64_epi8(A, B, I) \
+  (__m128i)__builtin_ia32_vgf2p8affineqb_v16qi((__v16qi)(__m128i)(A),             \
+                                                  (__v16qi)(__m128i)(B),          \
+                                                  (char)(I))
 
+static __inline__ __m128i __DEFAULT_FN_ATTRS
+_mm_gf2p8mul_epi8(__m128i __A, __m128i __B)
+{
+  return (__m128i) __builtin_ia32_vgf2p8mulb_v16qi((__v16qi) __A,
+              (__v16qi) __B);
+}
 
+#ifdef __AVXINTRIN_H
 #define _mm256_gf2p8affineinv_epi64_epi8(A, B, I) \
   (__m256i)__builtin_ia32_vgf2p8affineinvqb_v32qi((__v32qi)(__m256i)(A),          \
                                                   (__v32qi)(__m256i)(B),          \
                                                   (char)(I))
 
-#define _mm256_mask_gf2p8affineinv_epi64_epi8(S, U, A, B, I) \
-   (__m256i)__builtin_ia32_selectb_256((__mmask32)(U),                            \
-        (__v32qi)_mm256_gf2p8affineinv_epi64_epi8(A, B, I),                       \
-        (__v32qi)(__m256i)(S))
-
-#define _mm256_maskz_gf2p8affineinv_epi64_epi8(U, A, B, I) \
-  (__m256i)_mm256_mask_gf2p8affineinv_epi64_epi8((__m256i)_mm256_setzero_si256(), \
-        U, A, B, I)
+#define _mm256_gf2p8affine_epi64_epi8(A, B, I) \
+  (__m256i)__builtin_ia32_vgf2p8affineqb_v32qi((__v32qi)(__m256i)(A),             \
+                                                  (__v32qi)(__m256i)(B),          \
+                                                  (char)(I))
 
+static __inline__ __m256i __DEFAULT_FN_ATTRS_Y
+_mm256_gf2p8mul_epi8(__m256i __A, __m256i __B)
+{
+  return (__m256i) __builtin_ia32_vgf2p8mulb_v32qi((__v32qi) __A,
+              (__v32qi) __B);
+}
+#endif /* __AVXINTRIN_H */
 
+#ifdef __AVX512BWINTRIN_H
 #define _mm512_gf2p8affineinv_epi64_epi8(A, B, I) \
   (__m512i)__builtin_ia32_vgf2p8affineinvqb_v64qi((__v64qi)(__m512i)(A),          \
                                                   (__v64qi)(__m512i)(B),          \
                                                   (char)(I))
 
 #define _mm512_mask_gf2p8affineinv_epi64_epi8(S, U, A, B, I) \
    (__m512i)__builtin_ia32_selectb_512((__mmask64)(U),                            \
         (__v64qi)_mm512_gf2p8affineinv_epi64_epi8(A, B, I),                       \
         (__v64qi)(__m512i)(S))
 
 #define _mm512_maskz_gf2p8affineinv_epi64_epi8(U, A, B, I) \
   (__m512i)_mm512_mask_gf2p8affineinv_epi64_epi8((__m512i)_mm512_setzero_si512(),    \
         U, A, B, I)
 
-#define _mm_gf2p8affine_epi64_epi8(A, B, I) \
-  (__m128i)__builtin_ia32_vgf2p8affineqb_v16qi((__v16qi)(__m128i)(A),             \
-                                                  (__v16qi)(__m128i)(B),          \
+#define _mm512_gf2p8affine_epi64_epi8(A, B, I) \
+  (__m512i)__builtin_ia32_vgf2p8affineqb_v64qi((__v64qi)(__m512i)(A),             \
+                                                  (__v64qi)(__m512i)(B),          \
                                                   (char)(I))
 
+#define _mm512_mask_gf2p8affine_epi64_epi8(S, U, A, B, I) \
+   (__m512i)__builtin_ia32_selectb_512((__mmask64)(U),                            \
+        (__v64qi)_mm512_gf2p8affine_epi64_epi8(A, B, I),                          \
+        (__v64qi)(__m512i)(S))
+
+#define _mm512_maskz_gf2p8affine_epi64_epi8(U, A, B, I) \
+  (__m512i)_mm512_mask_gf2p8affine_epi64_epi8((__m512i)_mm512_setzero_si512(),       \
+        U, A, B, I)
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS_Z
+_mm512_gf2p8mul_epi8(__m512i __A, __m512i __B)
+{
+  return (__m512i) __builtin_ia32_vgf2p8mulb_v64qi((__v64qi) __A,
+              (__v64qi) __B);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS_Z
+_mm512_mask_gf2p8mul_epi8(__m512i __S, __mmask64 __U, __m512i __A, __m512i __B)
+{
+  return (__m512i) __builtin_ia32_selectb_512(__U,
+              (__v64qi) _mm512_gf2p8mul_epi8(__A, __B),
+              (__v64qi) __S);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS_Z
+_mm512_maskz_gf2p8mul_epi8(__mmask64 __U, __m512i __A, __m512i __B)
+{
+  return _mm512_mask_gf2p8mul_epi8((__m512i)_mm512_setzero_si512(),
+              __U, __A, __B);
+}
+#endif /* __AVX512BWINTRIN_H */
+
+#ifdef __AVX512VLBWINTRIN_H
+#define _mm_mask_gf2p8affineinv_epi64_epi8(S, U, A, B, I) \
+  (__m128i)__builtin_ia32_selectb_128((__mmask16)(U),                             \
+        (__v16qi)_mm_gf2p8affineinv_epi64_epi8(A, B, I),                          \
+        (__v16qi)(__m128i)(S))
+
+#define _mm_maskz_gf2p8affineinv_epi64_epi8(U, A, B, I) \
+  (__m128i)_mm_mask_gf2p8affineinv_epi64_epi8((__m128i)_mm_setzero_si128(),       \
+        U, A, B, I)
+
+#define _mm256_mask_gf2p8affineinv_epi64_epi8(S, U, A, B, I) \
+   (__m256i)__builtin_ia32_selectb_256((__mmask32)(U),                            \
+        (__v32qi)_mm256_gf2p8affineinv_epi64_epi8(A, B, I),                       \
+        (__v32qi)(__m256i)(S))
+
+#define _mm256_maskz_gf2p8affineinv_epi64_epi8(U, A, B, I) \
+  (__m256i)_mm256_mask_gf2p8affineinv_epi64_epi8((__m256i)_mm256_setzero_si256(), \
+        U, A, B, I)
+
 #define _mm_mask_gf2p8affine_epi64_epi8(S, U, A, B, I) \
   (__m128i)__builtin_ia32_selectb_128((__mmask16)(U),                             \
         (__v16qi)_mm_gf2p8affine_epi64_epi8(A, B, I),                             \
         (__v16qi)(__m128i)(S))
 
-
 #define _mm_maskz_gf2p8affine_epi64_epi8(U, A, B, I) \
   (__m128i)_mm_mask_gf2p8affine_epi64_epi8((__m128i)_mm_setzero_si128(),          \
         U, A, B, I)
 
-
-#define _mm256_gf2p8affine_epi64_epi8(A, B, I) \
-  (__m256i)__builtin_ia32_vgf2p8affineqb_v32qi((__v32qi)(__m256i)(A),             \
-                                                  (__v32qi)(__m256i)(B),          \
-                                                  (char)(I))
-
 #define _mm256_mask_gf2p8affine_epi64_epi8(S, U, A, B, I) \
    (__m256i)__builtin_ia32_selectb_256((__mmask32)(U),                            \
         (__v32qi)_mm256_gf2p8affine_epi64_epi8(A, B, I),                          \
         (__v32qi)(__m256i)(S))
 
 #define _mm256_maskz_gf2p8affine_epi64_epi8(U, A, B, I) \
   (__m256i)_mm256_mask_gf2p8affine_epi64_epi8((__m256i)_mm256_setzero_si256(),    \
         U, A, B, I)
 
-
-#define _mm512_gf2p8affine_epi64_epi8(A, B, I) \
-  (__m512i)__builtin_ia32_vgf2p8affineqb_v64qi((__v64qi)(__m512i)(A),             \
-                                                  (__v64qi)(__m512i)(B),          \
-                                                  (char)(I))
-
-#define _mm512_mask_gf2p8affine_epi64_epi8(S, U, A, B, I) \
-   (__m512i)__builtin_ia32_selectb_512((__mmask64)(U),                            \
-        (__v64qi)_mm512_gf2p8affine_epi64_epi8(A, B, I),                          \
-        (__v64qi)(__m512i)(S))
-
-#define _mm512_maskz_gf2p8affine_epi64_epi8(U, A, B, I) \
-  (__m512i)_mm512_mask_gf2p8affine_epi64_epi8((__m512i)_mm512_setzero_si512(),       \
-        U, A, B, I)
-
-/* Default attributes for simple form (no masking). */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("gfni"), __min_vector_width__(128)))
-
-/* Default attributes for YMM unmasked form. */
-#define __DEFAULT_FN_ATTRS_Y __attribute__((__always_inline__, __nodebug__, __target__("avx,gfni"), __min_vector_width__(256)))
-
-/* Default attributes for ZMM forms. */
-#define __DEFAULT_FN_ATTRS_Z __attribute__((__always_inline__, __nodebug__, __target__("avx512bw,gfni"), __min_vector_width__(512)))
-
-/* Default attributes for VLX forms. */
-#define __DEFAULT_FN_ATTRS_VL128 __attribute__((__always_inline__, __nodebug__, __target__("avx512bw,avx512vl,gfni"), __min_vector_width__(128)))
-#define __DEFAULT_FN_ATTRS_VL256 __attribute__((__always_inline__, __nodebug__, __target__("avx512bw,avx512vl,gfni"), __min_vector_width__(256)))
-
-static __inline__ __m128i __DEFAULT_FN_ATTRS
-_mm_gf2p8mul_epi8(__m128i __A, __m128i __B)
-{
-  return (__m128i) __builtin_ia32_vgf2p8mulb_v16qi((__v16qi) __A,
-              (__v16qi) __B);
-}
-
 static __inline__ __m128i __DEFAULT_FN_ATTRS_VL128
 _mm_mask_gf2p8mul_epi8(__m128i __S, __mmask16 __U, __m128i __A, __m128i __B)
 {
   return (__m128i) __builtin_ia32_selectb_128(__U,
               (__v16qi) _mm_gf2p8mul_epi8(__A, __B),
               (__v16qi) __S);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS_VL128
 _mm_maskz_gf2p8mul_epi8(__mmask16 __U, __m128i __A, __m128i __B)
 {
   return _mm_mask_gf2p8mul_epi8((__m128i)_mm_setzero_si128(),
               __U, __A, __B);
 }
 
-static __inline__ __m256i __DEFAULT_FN_ATTRS_Y
-_mm256_gf2p8mul_epi8(__m256i __A, __m256i __B)
-{
-  return (__m256i) __builtin_ia32_vgf2p8mulb_v32qi((__v32qi) __A,
-              (__v32qi) __B);
-}
-
 static __inline__ __m256i __DEFAULT_FN_ATTRS_VL256
 _mm256_mask_gf2p8mul_epi8(__m256i __S, __mmask32 __U, __m256i __A, __m256i __B)
 {
   return (__m256i) __builtin_ia32_selectb_256(__U,
               (__v32qi) _mm256_gf2p8mul_epi8(__A, __B),
               (__v32qi) __S);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS_VL256
 _mm256_maskz_gf2p8mul_epi8(__mmask32 __U, __m256i __A, __m256i __B)
 {
   return _mm256_mask_gf2p8mul_epi8((__m256i)_mm256_setzero_si256(),
               __U, __A, __B);
 }
-
-static __inline__ __m512i __DEFAULT_FN_ATTRS_Z
-_mm512_gf2p8mul_epi8(__m512i __A, __m512i __B)
-{
-  return (__m512i) __builtin_ia32_vgf2p8mulb_v64qi((__v64qi) __A,
-              (__v64qi) __B);
-}
-
-static __inline__ __m512i __DEFAULT_FN_ATTRS_Z
-_mm512_mask_gf2p8mul_epi8(__m512i __S, __mmask64 __U, __m512i __A, __m512i __B)
-{
-  return (__m512i) __builtin_ia32_selectb_512(__U,
-              (__v64qi) _mm512_gf2p8mul_epi8(__A, __B),
-              (__v64qi) __S);
-}
-
-static __inline__ __m512i __DEFAULT_FN_ATTRS_Z
-_mm512_maskz_gf2p8mul_epi8(__mmask64 __U, __m512i __A, __m512i __B)
-{
-  return _mm512_mask_gf2p8mul_epi8((__m512i)_mm512_setzero_si512(),
-              __U, __A, __B);
-}
+#endif /* __AVX512VLBWINTRIN_H */
 
 #undef __DEFAULT_FN_ATTRS
 #undef __DEFAULT_FN_ATTRS_Y
 #undef __DEFAULT_FN_ATTRS_Z
 #undef __DEFAULT_FN_ATTRS_VL128
 #undef __DEFAULT_FN_ATTRS_VL256
 
 #endif /* __GFNIINTRIN_H */