[x86] Fix bugs of some intrinsic functions in CLANG : _mm512_stream_ps, _mm512_stream_pd, _mm512_stream_si512

[yubing]

Summary

Fix bugs of some intrinsic functions in CLANG : _mm512_stream_ps, _mm512_stream_pd, _mm512_stream_si512

[RKSimon]

clang/test/CodeGen/avx512f-builtins.c

8570 ↗

(On Diff #217315)

Would it make sense to keep the __m512* variants as well for test coverage?

[craig.topper]

I think @RKSimon was asking to have tests with the original type and with the new void type. Just to verify that we accept code we used to accept and that we now accept void.

[RKSimon]

LGTM - thanks

[FreddyYe]

In D66786#1654183, @craig.topper wrote:

I think @RKSimon was asking to have tests with the original type and with the new void type. Just to verify that we accept code we used to accept and that we now accept void.

Sorry for ping this long long ago Phab... I just found we are missing alignment between these intrinsics' 512 variants and 128/256 ones. Does anyone remember the motivation here? The intrinsic guide also suffers such unaligning issue, but we can refine there, too if we could have a conclusion here. BTW gcc doesn't have such fix so they used aligned type* for these intrinsics.

[RKSimon]

In D66786#4627385, @FreddyYe wrote:

In D66786#1654183, @craig.topper wrote:

I think @RKSimon was asking to have tests with the original type and with the new void type. Just to verify that we accept code we used to accept and that we now accept void.

Sorry for ping this long long ago Phab... I just found we are missing alignment between these intrinsics' 512 variants and 128/256 ones. Does anyone remember the motivation here? The intrinsic guide also suffers such unaligning issue, but we can refine there, too if we could have a conclusion here. BTW gcc doesn't have such fix so they used aligned type* for these intrinsics.

The tests (and the doxygen descriptions for sse2/avxavx2 headers) all have vector-alignment requirements - where did you find it missing?

[FreddyYe]

In D66786#4628233, @RKSimon wrote:

In D66786#4627385, @FreddyYe wrote:

In D66786#1654183, @craig.topper wrote:

I think @RKSimon was asking to have tests with the original type and with the new void type. Just to verify that we accept code we used to accept and that we now accept void.

Sorry for ping this long long ago Phab... I just found we are missing alignment between these intrinsics' 512 variants and 128/256 ones. Does anyone remember the motivation here? The intrinsic guide also suffers such unaligning issue, but we can refine there, too if we could have a conclusion here. BTW gcc doesn't have such fix so they used aligned type* for these intrinsics.

The tests (and the doxygen descriptions for sse2/avxavx2 headers) all have vector-alignment requirements - where did you find it missing?

The issue is 128/256 variants both used type* but 512 variants here used void*. I mean from the intrinsic level, not builtin.

[FreddyYe]

ping...

[RKSimon]

In D66786#4636498, @FreddyYe wrote:

ping...

@yubing

[FreddyYe]

FYI I raised a PR https://github.com/llvm/llvm-project/pull/66310 to align 128/256 variants to use void*. We can modify/inform intirnsic guide/gcc to align this change if the PR can be landed. Welcome comments there!

cfe/trunk/lib/Headers/avx512fintrin.h

 }
 
 static __inline__ void __DEFAULT_FN_ATTRS
 _store_mask16(__mmask16 *__A, __mmask16 __B) {
   *(__mmask16 *)__A = __builtin_ia32_kmovw((__mmask16)__B);
 }
 
 static __inline__ void __DEFAULT_FN_ATTRS512
-_mm512_stream_si512 (__m512i * __P, __m512i __A)
+_mm512_stream_si512 (void * __P, __m512i __A)
 {
   typedef __v8di __v8di_aligned __attribute__((aligned(64)));
   __builtin_nontemporal_store((__v8di_aligned)__A, (__v8di_aligned*)__P);
 }
 
 static __inline__ __m512i __DEFAULT_FN_ATTRS512
 _mm512_stream_load_si512 (void const *__P)
 {
   typedef __v8di __v8di_aligned __attribute__((aligned(64)));
   return (__m512i) __builtin_nontemporal_load((const __v8di_aligned *)__P);
 }
 
 static __inline__ void __DEFAULT_FN_ATTRS512
-_mm512_stream_pd (double *__P, __m512d __A)
+_mm512_stream_pd (void *__P, __m512d __A)
 {
   typedef __v8df __v8df_aligned __attribute__((aligned(64)));
   __builtin_nontemporal_store((__v8df_aligned)__A, (__v8df_aligned*)__P);
 }
 
 static __inline__ void __DEFAULT_FN_ATTRS512
-_mm512_stream_ps (float *__P, __m512 __A)
+_mm512_stream_ps (void *__P, __m512 __A)
 {
   typedef __v16sf __v16sf_aligned __attribute__((aligned(64)));
   __builtin_nontemporal_store((__v16sf_aligned)__A, (__v16sf_aligned*)__P);
 }
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS512
 _mm512_mask_compress_pd (__m512d __W, __mmask8 __U, __m512d __A)
 {

cfe/trunk/test/CodeGen/avx512f-builtins.c

 }
 
 void test_mm512_stream_si512(__m512i * __P, __m512i __A) {
   // CHECK-LABEL: @test_mm512_stream_si512
   // CHECK: store <8 x i64> %{{.*}}, <8 x i64>* %{{.*}}, align 64, !nontemporal
   _mm512_stream_si512(__P, __A); 
 }
 
+void test_mm512_stream_si512_2(void * __P, __m512i __A) {
+  // CHECK-LABEL: @test_mm512_stream_si512
+  // CHECK: store <8 x i64> %{{.*}}, <8 x i64>* %{{.*}}, align 64, !nontemporal
+  _mm512_stream_si512(__P, __A); 
+}
+
 __m512i test_mm512_stream_load_si512(void *__P) {
   // CHECK-LABEL: @test_mm512_stream_load_si512
   // CHECK: load <8 x i64>, <8 x i64>* %{{.*}}, align 64, !nontemporal
   return _mm512_stream_load_si512(__P); 
 }
 
 __m512i test_mm512_stream_load_si512_const(void const *__P) {
   // CHECK-LABEL: @test_mm512_stream_load_si512_const
   // CHECK: load <8 x i64>, <8 x i64>* %{{.*}}, align 64, !nontemporal
   return _mm512_stream_load_si512(__P); 
 }
 
 void test_mm512_stream_pd(double *__P, __m512d __A) {
   // CHECK-LABEL: @test_mm512_stream_pd
   // CHECK: store <8 x double> %{{.*}}, <8 x double>* %{{.*}}, align 64, !nontemporal
   return _mm512_stream_pd(__P, __A); 
 }
 
+void test_mm512_stream_pd_2(void *__P, __m512d __A) {
+  // CHECK-LABEL: @test_mm512_stream_pd
+  // CHECK: store <8 x double> %{{.*}}, <8 x double>* %{{.*}}, align 64, !nontemporal
+  return _mm512_stream_pd(__P, __A); 
+}
+
 void test_mm512_stream_ps(float *__P, __m512 __A) {
   // CHECK-LABEL: @test_mm512_stream_ps
   // CHECK: store <16 x float> %{{.*}}, <16 x float>* %{{.*}}, align 64, !nontemporal
   _mm512_stream_ps(__P, __A); 
 }
 
+void test_mm512_stream_ps_2(void *__P, __m512 __A) {
+  // CHECK-LABEL: @test_mm512_stream_ps
+  // CHECK: store <16 x float> %{{.*}}, <16 x float>* %{{.*}}, align 64, !nontemporal
+  _mm512_stream_ps(__P, __A); 
+}
 __m512d test_mm512_mask_compress_pd(__m512d __W, __mmask8 __U, __m512d __A) {
   // CHECK-LABEL: @test_mm512_mask_compress_pd
   // CHECK: @llvm.x86.avx512.mask.compress
   return _mm512_mask_compress_pd(__W, __U, __A); 
 }
 
 __m512d test_mm512_maskz_compress_pd(__mmask8 __U, __m512d __A) {
   // CHECK-LABEL: @test_mm512_maskz_compress_pd