Re: [PATCH 4/13] rs6000, extend the current vec_{un,}signed{e,o} built-ins

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Carl,

on 2024/5/18 04:20, Carl Love wrote:
> Kewen:
> 
> I am working thru the patches.  I made the changes as requested for this patch but have a question about 
> one of your comments.
> 
> On 5/14/24 00:53, Kewen.Lin wrote:
>> Hi,
>>
>> on 2024/4/20 05:17, Carl Love wrote:
>>> rs6000, extend the current vec_{un,}signed{e,o} built-ins
>>>
>>> The built-ins __builtin_vsx_xvcvspsxds and __builtin_vsx_xvcvspuxds
>>> convert a vector of floats to signed/unsigned long long ints.  Extend the
>>> existing vec_{un,}signed{e,o} built-ins to handle the argument
>>> vector of floats to return the even/odd signed/unsigned integers.
>>>
>>> Add testcases and update documentation.
>>>
>>> gcc/ChangeLog:
>>>         * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcvspsxds_low,
>>>         __builtin_vsx_xvcvspuxds_low): New built-in definitions.
>>>         * config/rs6000/rs6000-overload.def (vec_signede, vec_signedo):
>>>         Add new overloaded specifications.
>>>         * config/rs6000/vsx.md (vsx_xvcvsp<su>xds_low): New define_expand.
>>>         * doc/extend.texi (vec_signedo, vec_signede): Add documentation.
>>>
>>> gcc/testsuite/ChangeLog:
>>>         * gcc.target/powerpc/builtins-3-runnable: New tests for the added
> 
> 
> <snip>
>>
>> As the existing instances for vec_signed and vec_unsigned are with
>> names like VEC_V{UN,}SIGNED{O,E}_V2DF, I prefer these are updated
>> with similar style, maybe something like:
>>
>> VEC_V{UN,}SIGNED{E,O}_V4SF v{un,}signed{e,o}_v4sf
> 
> Yes, sounds reasonable.  Changed XVCVSPUXDS -> VEC_VUNSIGNEDE_V4SF
>                                  XVCVSPUXDSO -> VEC_VUNSIGNEDO_V4SF
> 				 XVCVSPSXDS  -> VEC_VSIGNEDE_V4SF
> 				 XVCVSPSXDSO  -> VEC_VSIGNEDO_V4SF
> 
> QUESTION:
> I am not sure what you want changed to v{un,}signed{e,o}_v4sf??  The overloaded instance entry names

It's about the expander name, just like the existing *vunsignede_v2df* and *vunsignedo_v2df*:

  const vsi __builtin_vsx_vunsignede_v2df (vd);
    VEC_VUNSIGNEDE_V2DF vunsignede_v2df {}    

  const vsi __builtin_vsx_vunsignedo_v2df (vd);
    VEC_VUNSIGNEDO_V2DF vunsignedo_v2df {}

, not for the actual builtin names, sorry for the confusion.

BR,
Kewen

> for vd, vf have to match the first line of the definition. The name can't be type specific, i.e. v4sf.  
> So not sure where you want the v{un,}signed{e,o}_v4sf name used?
> 
> For example, file rs6000-overloaded.def now looks like:
> 
> [VEC_SIGNEDE, vec_signede, __builtin_vec_vsignede]
>    vsi __builtin_vec_vsignede (vd);
>      VEC_VSIGNEDE_V2DF
> +  vsll __builtin_vec_vsignede (vf);
> +    VEC_VSIGNEDE_V4SF
>  
>  [VEC_SIGNEDO, vec_signedo, __builtin_vec_vsignedo]
>    vsi __builtin_vec_vsignedo (vd);
>      VEC_VSIGNEDO_V2DF
> +  vsll __builtin_vec_vsignedo (vf);
> +    VEC_VSIGNEDO_V4SF
>  
> 
> 
> <snip>
> 
>                          Carl