skan accepted this revision.
skan added inline comments.
================
Comment at: clang/test/CodeGen/X86/avx512vlbw-reduceIntrin.c:115
+
+char test_mm_reduce_mul_epi8(__m128i __W){
+// CHECK-LABEL: @test_mm_reduce_mul_epi8(
----------------
FreddyYe wrote:
> skan wrote:
> > skan wrote:
> > > char has different meaning on different platforms. Should we use "signed
> > > char" or "unsigned char" explicitly?
> > > char has different meaning on different platforms. Should we use "signed
> > > char" or "unsigned char" explicitly?
> >
> > Sorry, I think you misuderstood. It's a question, not a request. I already
> > saw
> >
> > ```
> > _mm_mask_set1_epi8 (__m128i __O, __mmask16 __M, char __A)
> > _mm_maskz_set1_epi8 (__mmask16 __M, char __A)
> > _mm256_mask_set1_epi8 (__m256i __O, __mmask32 __M, char __A)
> > _mm256_maskz_set1_epi8 (__mmask32 __M, char __A)
> > ```
> >
> > in this file, but I am not if using "char" is appropriate.
> After investigating, I think we should use `signed char` and `_v\d+qs`
> instead of `_v\d+qi` for new intrinsic here for 2 reasons:
> 1) _epi8 means `signed i8` here.
> 2) implement is using LLVM builtins but not x86 builtins.
>
> And intrinsics not fitting two conditions above don't need to use explictly
> `signed char`.
Make sense
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140531/new/
https://reviews.llvm.org/D140531
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
