Re: [PATCH 2/13] rs6000, Remove __builtin_vsx_xvcvspsxws built-in

[Carl Love <cel@linux.ibm.com>]

Kewen:

On 5/14/24 01:43, Kewen.Lin wrote:
> Hi,
> 
> on 2024/4/20 05:17, Carl Love wrote:
>> rs6000, Remove __builtin_vsx_xvcvspsxws built-in
>>
>> The built-in __builtin_vsx_xvcvspsxws is a duplicate of the vec_signed
>> built-in that is documented in the PVIPR.  The __builtin_vsx_xvcvspsxws
>> built-in is not documented and there are no test cases for it.
>>
>> This patch removes the redundant built-in.
> 
> By revisiting the comments on the previous version:
> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646723.html

The comments from the previous version:
-----------------------------------------------------------------
   I think we should recommend users to adopt the recommended built-ins in
   PVIPR, by checking the corresponding mnemonic in PVIPR, I got:

   __builtin_vsx_xvcvspsxws -> vec_signed
   __builtin_vsx_xvcvspsxds -> N/A
   __builtin_vsx_xvcvspuxds -> N/A
   __builtin_vsx_xvcvdpsxws -> vec_signed{e,o}
   __builtin_vsx_xvcvdpuxws -> vec_unsigned{e,o}
   __builtin_vsx_xvcvdpuxds_uns -> vec_unsigned
   __builtin_vsx_xvcvspdp   -> vec_double{e,o}
   __builtin_vsx_xvcvdpsp   -> vec_float{e,o}
   __builtin_vsx_xvcvspuxws -> vec_unsigned
   __builtin_vsx_xvcvsxwdp  -> vec_double{e,o}
   __builtin_vsx_xvcvuxddp_uns> vec_double

   For __builtin_vsx_xvcvspsxds and __builtin_vsx_xvcvspuxds which don't have
   the according PVIPR built-ins, we can extend the current vec_{un,}signed{e,o}
   to cover them and document them following the section mentioning PVIPR.

are handled by multiple patches in the new series.  The main comment on the previous patch series was to remove most of the built-ins as they were redundant.  So, basically most of the patches in the previous series were thrown out and a new series to remove the built-ins in the current series.
----------------------------------------------------------------------------

That all said, I distinctly remember addressing each of the above built-ins.  The work on the series got
interrupted a couple of times and it looks like some of the patches to address the above got lost.  My bad.
The following is a list of which patch takes care of removing the duplicate built-ins.

__builtin_vsx_xvcvspsxws                     patch 2 removes this built-in
__builtin_vsx_xvcvspsxds -> N/A              patch 4 extends vec_{un,}signede to cover this built-in,
					     Built-in used in rs6000-overload.def.  Built-in now for                           
                                             internal use only.
__builtin_vsx_xvcvspuxds -> N/A              patch 4 extends vec_{un,}signedo to cover this built-in.
					     Built-in used in rs6000-overload.def.  Built-in now for
                                             internal use only 


__builtin_vsx_xvcvdpsxws -> vec_signed{e,o}   removed in patch 4
__builtin_vsx_xvcvdpuxws -> vec_unsigned{e,o} removed in patch 4

__builtin_vsx_xvcvdpuxds_uns -> vec_unsigned  remove in patch 4
__builtin_vsx_xvcvspuxws -> vec_unsigned      remove in patch 4

The following will changes will be put into a new patch when the series is reposted.  It appears they
got lost in the current series.  My bad.

__builtin_vsx_xvcvspdp   -> vec_double{e,o}   remove in new patch number 5
__builtin_vsx_xvcvdpsp   -> vec_float{e,o}    remove in new patch number 5

__builtin_vsx_xvcvsxwdp  -> vec_double{e,o}   remove in new patch number 5
__builtin_vsx_xvcvuxddp_uns> vec_double       remove in new patch number 5

> 
> I wonder if it's intentional to keep the others, at least bifs
> __builtin_vsx_xvcvdpuxds_uns, __builtin_vsx_xvcvspuxws and
> __builtin_vsx_xvcvuxddp_uns looks removable, users can just uses the
> equivalent ones in PVIPR.  And for the others, users can still use
> the PVIPR ones by considering endianness (controlling with endianness
> macros).
> 

Hopefully that makes it clearer where the various changes are.   

The next series will add a new patch 5 in the series.  The remaining patches in this series, patches 5, 6, ... will get moved to patch 6, 7, ... in the next posting of the built-in cleanup patch series.

                            Carl