On 5/13/24 22:37, Kewen.Lin wrote:
> Hi,
>
> on 2024/4/20 05:18, Carl Love wrote:
>> rs6000, remove __builtin_vsx_xvcmpeqsp built-in
>>
>> The built-in __builtin_vsx_xvcmpeqsp is a duplicate of the overloaded
>> vec_cmpeq built-in. The built-in is undocumented. The built-in and
>> the test cases are removed.
>>
>> gcc/ChangeLog:
>> * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcmpeqsp):
>> Remove built-in definition.
>>
>
> Ah, you separated this __builtin_vsx_xvcmpeqsp from the one for
> __builtin_vsx_xvcmpeqsp_p, it's fine, please ignore the comments for
> considering this __builtin_vsx_xvcmpeqsp in my previous reply to 11/13.
>
>
>> gcc/testsuite/ChangeLog:
>> * vsx-builtin-3.c (do_cmp): Remove test case for
>> __builtin_vsx_xvcmpeqsp.
>> ---
>> gcc/config/rs6000/rs6000-builtins.def | 3 ---
>> gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c | 2 --
>> 2 files changed, 5 deletions(-)
>>
>> diff --git a/gcc/config/rs6000/rs6000-builtins.def
>> b/gcc/config/rs6000/rs6000-builtins.def
>> index 2f6149edd5f..19d05b8043a 100644
>> --- a/gcc/config/rs6000/rs6000-builtins.def
>> +++ b/gcc/config/rs6000/rs6000-builtins.def
>> @@ -1613,9 +1613,6 @@
>> const signed int __builtin_vsx_xvcmpeqdp_p (signed int, vd, vd);
>> XVCMPEQDP_P vector_eq_v2df_p {pred}
>>
>> - const vf __builtin_vsx_xvcmpeqsp (vf, vf);
>> - XVCMPEQSP vector_eqv4sf {}
>> -
>> const vd __builtin_vsx_xvcmpgedp (vd, vd);
>> XVCMPGEDP vector_gev2df {}
>>
>> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
>> b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
>> index 35ea31b2616..245893dc0e3 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
>> @@ -27,7 +27,6 @@
>> /* { dg-final { scan-assembler "xvcmpeqdp" } } */
>> /* { dg-final { scan-assembler "xvcmpgtdp" } } */
>> /* { dg-final { scan-assembler "xvcmpgedp" } } */
>> -/* { dg-final { scan-assembler "xvcmpeqsp" } } */
>> /* { dg-final { scan-assembler "xvcmpgtsp" } } */
>> /* { dg-final { scan-assembler "xvcmpgesp" } } */
>> /* { dg-final { scan-assembler "xxsldwi" } } */
>> @@ -112,7 +111,6 @@ int do_cmp (void)
>> d[i][0] = __builtin_vsx_xvcmpgtdp (d[i][1], d[i][2]); i++;
>> d[i][0] = __builtin_vsx_xvcmpgedp (d[i][1], d[i][2]); i++;
>>
>> - f[i][0] = __builtin_vsx_xvcmpeqsp (f[i][1], f[i][2]); i++;
>> f[i][0] = __builtin_vsx_xvcmpgtsp (f[i][1], f[i][2]); i++;
>> f[i][0] = __builtin_vsx_xvcmpgesp (f[i][1], f[i][2]); i++;
>> return i;
>
> As the other in this patch series, I prefer to change it with
> vec_cmpeq here, OK for trunk with this tweaked (also keep the
> scan there), thanks!
When I went to change the test case I noticed that __builtin_vsx_xvcmpeqsp and
vec_cmpeq both return a vector where the element is all ones if the comparison
is True and zeros if False. However, the return type for
__builtin_vsx_xvcmpeqsp is vector floats but vec_cmpeq returns vector bool.
The PVIPR says the vec_cmpeq built-in returns a value where each bit in the
vector element is a 1 if the comparison is equal and 0 otherwise. However, the
documented result is a vector bool int for the floating point comparison. The
return value for __builtin_vsx_xvcmpeqsp was vector float.
So, the "bit values" returned are the same but not of the same type. So
technically vec_cmpeq is not a drop in replacement for __builtin_vsx_xvcmpeqsp.
Given that, perhaps we should not be removing __builtin_vsx_xvcmpeqsp?
The testcase has to be changed from:
f[i][0] = __builtin_vsx_xvcmpeqsp (f[i][1], f[i][2]); i++;
bi[i][0] = vec_cmpeq (f[i][1], f[i][2]); i++;
I am thinking we should drop this patch from the series, i.e. don't remove
__builtin_vsx_xvcmpeqsp. Thoughts?
Carl
>
> BR,
> Kewen
>
