Re: [PATCH 04/11] rs6000, Update comment for the __builtin_vsx_vper*, built-ins.

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

Hi,

on 2024/2/21 01:56, Carl Love wrote:
> GCC maintainers:
> 
> The patch expands an existing comment to document that the duplicates are covered by an overloaded built-in.  I am wondering if we should just go ahead and remove the duplicates?

As the below comments Bill placed before, I think we should remove them, since
users should use the standard interface vec_perm which is defined by PVIPR.

They are not undocumented at all, in case some users are still using such builtins
they should switch to use vec_perm instead, so even if it's stage 4 now, it looks
still fine to drop them IMHO.

Segher & Peter, what do you think of this?

BR,
Kewen

> 
> The patch has been tested on Power 10 with no regressions.
> 
> Please let me know if this patch is acceptable for mainline.  Thanks.
> 
>                       Carl 
> 
> -----------------------------------------------------
> rs6000, Update comment for the __builtin_vsx_vper* built-ins.
> 
> There is a comment about the __builtin_vsx_vper* built-ins being
> duplicates of the __builtin_altivec_* built-ins.  The note says we
> should consider deprecation/removeal of the __builtin_vsx_vper*.  Add a
> note that the _builtin_vsx_vper* built-ins are covered by the overloaded
> vec_perm built-ins which use the __builtin_altivec_* built-in definitions.
> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtins.def ( __builtin_vsx_vperm_*):
> 	Add comment to existing comment about the built-ins.
> ---
>  gcc/config/rs6000/rs6000-builtins.def | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 96d095da2cb..4c95429f137 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1556,6 +1556,14 @@
>  ; These are duplicates of __builtin_altivec_* counterparts, and are being
>  ; kept for backwards compatibility.  The reason for their existence is
>  ; unclear.  TODO: Consider deprecation/removal at some point.
> +; Note, __builtin_vsx_vperm_16qi, __builtin_vsx_vperm_16qi_uns,
> +; __builtin_vsx_vperm_1ti, __builtin_vsx_vperm_v1ti_uns,
> +; __builtin_vsx_vperm_2df, __builtin_vsx_vperm_2di, __builtin_vsx_vperm_2di,
> +; __builtin_vsx_vperm_2di_uns, __builtin_vsx_vperm_4sf,
> +; __builtin_vsx_vperm_4si, __builtin_vsx_vperm_4si_uns,
> +; __builtin_vsx_vperm_8hi, __builtin_altivec_vperm_8hi_uns
> +; are all covered by the overloaded vec_perm built-in which uses the
> +; __builtin_altivec_* built-in definitions.
>    const vsc __builtin_vsx_vperm_16qi (vsc, vsc, vuc);
>      VPERM_16QI_X altivec_vperm_v16qi {}
>