[PATCH 0/13] rs6000, built-in cleanup patch series

[PATCH 0/13] rs6000, built-in cleanup patch series

[Carl Love]

GCC maintainers:

The following patch series removes duplicate built-ins.  There are patches to extend an existing overloaded built-in to cover additional input types.  The final patch removes built-ins to set and initialize vectors.  The code generated by these built-ins with the default optimization is efficient than the code generated by using straight C code.  The assembly code for the built-in and straight C code is the same with -O3
optimizations.  In this case, the built-ins are removed as they add no additional value.

The patches have all been tested on Power 10 LE.  The last patch was also tested on Power 8 BE.

No regression tests were seen.

Please let me know if the patches are acceptable for mainline.  Thanks.

                       Carl 

[PATCH 1/13] rs6000, Remove __builtin_vsx_cmple* builtins

[Carl Love]

rs6000, Remove __builtin_vsx_cmple* builtins

The built-ins __builtin_vsx_cmple_u16qi, __builtin_vsx_cmple_u2di,
__builtin_vsx_cmple_u4si and __builtin_vsx_cmple_u8hi should take
unsigned arguments and return an unsigned result.  The current definitions
take signed arguments and return signed results which is incorrect.

The signed and unsigned versions of __builtin_vsx_cmple* are not
documented in extend.texi.  Also there are no test cases for the
built-ins.

Users can use the existing vec_cmple as PVIPR defines instead of
__builtin_vsx_cmple_u16qi, __builtin_vsx_cmple_u2di,
__builtin_vsx_cmple_u4si and __builtin_vsx_cmple_u8hi,
__builtin_vsx_cmple_16qi, __builtin_vsx_cmple_2di,
__builtin_vsx_cmple_4si and __builtin_vsx_cmple_8hi,
__builtin_altivec_cmple_1ti, __builtin_altivec_cmple_u1ti.

Hence these built-ins are redundant and are removed by this patch.

gcc/ChangeLog:
	* config/rs6000/rs6000-builtin.cc (RS6000_BIF_CMPLE_16QI,
	RS6000_BIF_CMPLE_U16QI, RS6000_BIF_CMPLE_8HI,
	RS6000_BIF_CMPLE_U8HI, RS6000_BIF_CMPLE_4SI, RS6000_BIF_CMPLE_U4SI,
	RS6000_BIF_CMPLE_2DI, RS6000_BIF_CMPLE_U2DI, RS6000_BIF_CMPLE_1TI,
	RS6000_BIF_CMPLE_U1TI): Remove case statements.
	config/rs6000/rs6000-builtins.def (__builtin_vsx_cmple_16qi,
	__builtin_vsx_cmple_2di, __builtin_vsx_cmple_4si,
	__builtin_vsx_cmple_8hi, __builtin_vsx_cmple_u16qi,
	__builtin_vsx_cmple_u2di, __builtin_vsx_cmple_u4si,
	__builtin_vsx_cmple_u8hi): Remove buit-in definitions.
---
 gcc/config/rs6000/rs6000-builtin.cc   | 13 ------------
 gcc/config/rs6000/rs6000-builtins.def | 30 ---------------------------
 2 files changed, 43 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
index 320affd79e3..ac9f16fe51a 100644
--- a/gcc/config/rs6000/rs6000-builtin.cc
+++ b/gcc/config/rs6000/rs6000-builtin.cc
@@ -2027,19 +2027,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
       fold_compare_helper (gsi, GT_EXPR, stmt);
       return true;
 
-    case RS6000_BIF_CMPLE_16QI:
-    case RS6000_BIF_CMPLE_U16QI:
-    case RS6000_BIF_CMPLE_8HI:
-    case RS6000_BIF_CMPLE_U8HI:
-    case RS6000_BIF_CMPLE_4SI:
-    case RS6000_BIF_CMPLE_U4SI:
-    case RS6000_BIF_CMPLE_2DI:
-    case RS6000_BIF_CMPLE_U2DI:
-    case RS6000_BIF_CMPLE_1TI:
-    case RS6000_BIF_CMPLE_U1TI:
-      fold_compare_helper (gsi, LE_EXPR, stmt);
-      return true;
-
     /* flavors of vec_splat_[us]{8,16,32}.  */
     case RS6000_BIF_VSPLTISB:
     case RS6000_BIF_VSPLTISH:
diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index 3bc7fed6956..7c36976a089 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1337,30 +1337,6 @@
   const vss __builtin_vsx_cmpge_u8hi (vus, vus);
     CMPGE_U8HI vector_nltuv8hi {}
 
-  const vsc __builtin_vsx_cmple_16qi (vsc, vsc);
-    CMPLE_16QI vector_ngtv16qi {}
-
-  const vsll __builtin_vsx_cmple_2di (vsll, vsll);
-    CMPLE_2DI vector_ngtv2di {}
-
-  const vsi __builtin_vsx_cmple_4si (vsi, vsi);
-    CMPLE_4SI vector_ngtv4si {}
-
-  const vss __builtin_vsx_cmple_8hi (vss, vss);
-    CMPLE_8HI vector_ngtv8hi {}
-
-  const vsc __builtin_vsx_cmple_u16qi (vsc, vsc);
-    CMPLE_U16QI vector_ngtuv16qi {}
-
-  const vsll __builtin_vsx_cmple_u2di (vsll, vsll);
-    CMPLE_U2DI vector_ngtuv2di {}
-
-  const vsi __builtin_vsx_cmple_u4si (vsi, vsi);
-    CMPLE_U4SI vector_ngtuv4si {}
-
-  const vss __builtin_vsx_cmple_u8hi (vss, vss);
-    CMPLE_U8HI vector_ngtuv8hi {}
-
   const vd __builtin_vsx_concat_2df (double, double);
     CONCAT_2DF vsx_concat_v2df {}
 
@@ -3117,12 +3093,6 @@
   const vbq __builtin_altivec_cmpge_u1ti (vuq, vuq);
     CMPGE_U1TI vector_nltuv1ti {}
 
-  const vbq __builtin_altivec_cmple_1ti (vsq, vsq);
-    CMPLE_1TI vector_ngtv1ti {}
-
-  const vbq __builtin_altivec_cmple_u1ti (vuq, vuq);
-    CMPLE_U1TI vector_ngtuv1ti {}
-
   const unsigned long long __builtin_altivec_cntmbb (vuc, const int<1>);
     VCNTMBB vec_cntmb_v16qi {}
 
-- 
2.44.0

Re: [PATCH 1/13] rs6000, Remove __builtin_vsx_cmple* builtins

[Kewen.Lin]

Hi,

on 2024/4/20 05:16, Carl Love wrote:
> 
> rs6000, Remove __builtin_vsx_cmple* builtins
> 
> The built-ins __builtin_vsx_cmple_u16qi, __builtin_vsx_cmple_u2di,
> __builtin_vsx_cmple_u4si and __builtin_vsx_cmple_u8hi should take
> unsigned arguments and return an unsigned result.  The current definitions
> take signed arguments and return signed results which is incorrect.
> 
> The signed and unsigned versions of __builtin_vsx_cmple* are not
> documented in extend.texi.  Also there are no test cases for the
> built-ins.
> 
> Users can use the existing vec_cmple as PVIPR defines instead of
> __builtin_vsx_cmple_u16qi, __builtin_vsx_cmple_u2di,
> __builtin_vsx_cmple_u4si and __builtin_vsx_cmple_u8hi,
> __builtin_vsx_cmple_16qi, __builtin_vsx_cmple_2di,
> __builtin_vsx_cmple_4si and __builtin_vsx_cmple_8hi,
> __builtin_altivec_cmple_1ti, __builtin_altivec_cmple_u1ti.
> 
> Hence these built-ins are redundant and are removed by this patch.

OK for trunk, thanks.

BR,
Kewen

> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtin.cc (RS6000_BIF_CMPLE_16QI,
> 	RS6000_BIF_CMPLE_U16QI, RS6000_BIF_CMPLE_8HI,
> 	RS6000_BIF_CMPLE_U8HI, RS6000_BIF_CMPLE_4SI, RS6000_BIF_CMPLE_U4SI,
> 	RS6000_BIF_CMPLE_2DI, RS6000_BIF_CMPLE_U2DI, RS6000_BIF_CMPLE_1TI,
> 	RS6000_BIF_CMPLE_U1TI): Remove case statements.
> 	config/rs6000/rs6000-builtins.def (__builtin_vsx_cmple_16qi,
> 	__builtin_vsx_cmple_2di, __builtin_vsx_cmple_4si,
> 	__builtin_vsx_cmple_8hi, __builtin_vsx_cmple_u16qi,
> 	__builtin_vsx_cmple_u2di, __builtin_vsx_cmple_u4si,
> 	__builtin_vsx_cmple_u8hi): Remove buit-in definitions.
> ---
>  gcc/config/rs6000/rs6000-builtin.cc   | 13 ------------
>  gcc/config/rs6000/rs6000-builtins.def | 30 ---------------------------
>  2 files changed, 43 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
> index 320affd79e3..ac9f16fe51a 100644
> --- a/gcc/config/rs6000/rs6000-builtin.cc
> +++ b/gcc/config/rs6000/rs6000-builtin.cc
> @@ -2027,19 +2027,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
>        fold_compare_helper (gsi, GT_EXPR, stmt);
>        return true;
>  
> -    case RS6000_BIF_CMPLE_16QI:
> -    case RS6000_BIF_CMPLE_U16QI:
> -    case RS6000_BIF_CMPLE_8HI:
> -    case RS6000_BIF_CMPLE_U8HI:
> -    case RS6000_BIF_CMPLE_4SI:
> -    case RS6000_BIF_CMPLE_U4SI:
> -    case RS6000_BIF_CMPLE_2DI:
> -    case RS6000_BIF_CMPLE_U2DI:
> -    case RS6000_BIF_CMPLE_1TI:
> -    case RS6000_BIF_CMPLE_U1TI:
> -      fold_compare_helper (gsi, LE_EXPR, stmt);
> -      return true;
> -
>      /* flavors of vec_splat_[us]{8,16,32}.  */
>      case RS6000_BIF_VSPLTISB:
>      case RS6000_BIF_VSPLTISH:
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 3bc7fed6956..7c36976a089 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1337,30 +1337,6 @@
>    const vss __builtin_vsx_cmpge_u8hi (vus, vus);
>      CMPGE_U8HI vector_nltuv8hi {}
>  
> -  const vsc __builtin_vsx_cmple_16qi (vsc, vsc);
> -    CMPLE_16QI vector_ngtv16qi {}
> -
> -  const vsll __builtin_vsx_cmple_2di (vsll, vsll);
> -    CMPLE_2DI vector_ngtv2di {}
> -
> -  const vsi __builtin_vsx_cmple_4si (vsi, vsi);
> -    CMPLE_4SI vector_ngtv4si {}
> -
> -  const vss __builtin_vsx_cmple_8hi (vss, vss);
> -    CMPLE_8HI vector_ngtv8hi {}
> -
> -  const vsc __builtin_vsx_cmple_u16qi (vsc, vsc);
> -    CMPLE_U16QI vector_ngtuv16qi {}
> -
> -  const vsll __builtin_vsx_cmple_u2di (vsll, vsll);
> -    CMPLE_U2DI vector_ngtuv2di {}
> -
> -  const vsi __builtin_vsx_cmple_u4si (vsi, vsi);
> -    CMPLE_U4SI vector_ngtuv4si {}
> -
> -  const vss __builtin_vsx_cmple_u8hi (vss, vss);
> -    CMPLE_U8HI vector_ngtuv8hi {}
> -
>    const vd __builtin_vsx_concat_2df (double, double);
>      CONCAT_2DF vsx_concat_v2df {}
>  
> @@ -3117,12 +3093,6 @@
>    const vbq __builtin_altivec_cmpge_u1ti (vuq, vuq);
>      CMPGE_U1TI vector_nltuv1ti {}
>  
> -  const vbq __builtin_altivec_cmple_1ti (vsq, vsq);
> -    CMPLE_1TI vector_ngtv1ti {}
> -
> -  const vbq __builtin_altivec_cmple_u1ti (vuq, vuq);
> -    CMPLE_U1TI vector_ngtuv1ti {}
> -
>    const unsigned long long __builtin_altivec_cntmbb (vuc, const int<1>);
>      VCNTMBB vec_cntmb_v16qi {}
>  

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

[Carl Love]

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.

gcc/ChangeLog:
        * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcvspsxws):
	Remove built-in definition.
---
 gcc/config/rs6000/rs6000-builtins.def | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index 7c36976a089..c6d2ea1bc39 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1709,9 +1709,6 @@
   const vsll __builtin_vsx_xvcvspsxds (vf);
     XVCVSPSXDS vsx_xvcvspsxds {}
 
-  const vsi __builtin_vsx_xvcvspsxws (vf);
-    XVCVSPSXWS vsx_fix_truncv4sfv4si2 {}
-
   const vsll __builtin_vsx_xvcvspuxds (vf);
     XVCVSPUXDS vsx_xvcvspuxds {}
 
-- 
2.44.0

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

[Kewen.Lin]

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

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).

BR,
Kewen

> 
> gcc/ChangeLog:
>         * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcvspsxws):
> 	Remove built-in definition.
> ---
>  gcc/config/rs6000/rs6000-builtins.def | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 7c36976a089..c6d2ea1bc39 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1709,9 +1709,6 @@
>    const vsll __builtin_vsx_xvcvspsxds (vf);
>      XVCVSPSXDS vsx_xvcvspsxds {}
>  
> -  const vsi __builtin_vsx_xvcvspsxws (vf);
> -    XVCVSPSXWS vsx_fix_truncv4sfv4si2 {}
> -
>    const vsll __builtin_vsx_xvcvspuxds (vf);
>      XVCVSPUXDS vsx_xvcvspuxds {}
>  

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

[Carl Love]

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 

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

[Kewen.Lin]

Hi Carl,

on 2024/5/25 04:18, Carl Love wrote:
> 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.

No problem, thanks for working on this!!

> 
> __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

Just to avoid some misunderstanding, I guess you meant the new patch 4?
As the current patch 4 doesn't remove these.

> 
> 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.
> 

OK, thanks!

BR,
Kewen

[PATCH 3/13] rs6000, fix error in unsigned vector float to unsigned int built-in definitions

[Carl Love]

rs6000, fix error in unsigned vector float to unsigned  int built-in definitions

The built-ins __builtin_vsx_vunsigned_v2df and__builtin_vsx_vunsigned_v4sf
are supposed to take a vector of floats and return a vector of unsigned
long long ints.  The definitions are using the signed version of the
instructions not the unsigned version of the instruction.  The results
should also be unsigned.  The builtins are used by the overloaded
vec_unsigned builtin which has an unsigned result.

Similarly the built-ins __builtin_vsx_vunsignede_v2df and
__builtin_vsx_vunsignedo_v2df are supposed to retun an unsigned result.
If the floating point argument is negative, the unsigned result is zero.
The built-ins are used in the overloaded built-in vec_unsignede and
vec_unsignedo respectively.

Add a test cases for a negative floating point arguments for each of the
above built-ins.

gcc/ChangeLog:
	* config/rs6000/rs6000-builtins.def (__builtin_vsx_vunsigned_v2df,
	__builtin_vsx_vunsigned_v4sf, __builtin_vsx_vunsignede_v2df,
	__builtin_vsx_vunsignedo_v2df): Change the result type to unsigned.

gcc/testsuite/ChangeLog:
	* gcc.target/powerpc/builtins-3-runnable.c: Add tests for
	vec_unsignede and vec_unsignedo with negative arguments.
---
 gcc/config/rs6000/rs6000-builtins.def         | 12 +++++-----
 .../gcc.target/powerpc/builtins-3-runnable.c  | 23 ++++++++++++++++---
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index c6d2ea1bc39..bf9a0ae22fc 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1580,16 +1580,16 @@
   const vsi __builtin_vsx_vsignedo_v2df (vd);
     VEC_VSIGNEDO_V2DF vsignedo_v2df {}
 
-  const vsll __builtin_vsx_vunsigned_v2df (vd);
-    VEC_VUNSIGNED_V2DF vsx_xvcvdpsxds {}
+  const vull __builtin_vsx_vunsigned_v2df (vd);
+    VEC_VUNSIGNED_V2DF vsx_xvcvdpuxds {}
 
-  const vsi __builtin_vsx_vunsigned_v4sf (vf);
-    VEC_VUNSIGNED_V4SF vsx_xvcvspsxws {}
+  const vui __builtin_vsx_vunsigned_v4sf (vf);
+    VEC_VUNSIGNED_V4SF vsx_xvcvspuxws {}
 
-  const vsi __builtin_vsx_vunsignede_v2df (vd);
+  const vui __builtin_vsx_vunsignede_v2df (vd);
     VEC_VUNSIGNEDE_V2DF vunsignede_v2df {}
 
-  const vsi __builtin_vsx_vunsignedo_v2df (vd);
+  const vui __builtin_vsx_vunsignedo_v2df (vd);
     VEC_VUNSIGNEDO_V2DF vunsignedo_v2df {}
 
   const vf __builtin_vsx_xscvdpsp (double);
diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
index 0231a1fd086..6d4fe84c8a1 100644
--- a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
@@ -313,6 +313,15 @@ int main()
 	test_unsigned_int_result (ALL, vec_uns_int_result,
 				  vec_uns_int_expected);
 
+	/* Convert single precision float to  unsigned int.  Negative
+	   arguments
+	 */
+	vec_flt0 = (vector float){-14.930, -834.49, -3.3, -5.4};
+	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
+	vec_uns_int_result = vec_unsigned (vec_flt0);
+	test_unsigned_int_result (ALL, vec_uns_int_result,
+				  vec_uns_int_expected);
+
 	/* Convert double precision float to long long unsigned int */
 	vec_dble0 = (vector double){124.930, 8134.49};
 	vec_ll_uns_int_expected = (vector long long unsigned int){124, 8134};
@@ -321,9 +330,9 @@ int main()
 				     vec_ll_uns_int_expected);
 
 	/* Convert double precision vector float to vector unsigned int,
-	   even words */
-	vec_dble0 = (vector double){3124.930, 8234.49};
-	vec_uns_int_expected = (vector unsigned int){3124, 0, 8234, 0};
+	   even words.  Negative arguments */
+	vec_dble0 = (vector double){-124.930, -234.49};
+	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
 	vec_uns_int_result = vec_unsignede (vec_dble0);
 	test_unsigned_int_result (EVEN, vec_uns_int_result,
 				  vec_uns_int_expected);
@@ -335,5 +344,13 @@ int main()
 	vec_uns_int_result = vec_unsignedo (vec_dble0);
 	test_unsigned_int_result (ODD, vec_uns_int_result,
 				  vec_uns_int_expected);
+
+	/* Convert double precision vector float to vector unsigned int,
+	   odd words.  Negative arguments.  */
+	vec_dble0 = (vector double){-924.930, -1234.49};
+	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
+	vec_uns_int_result = vec_unsignedo (vec_dble0);
+	test_unsigned_int_result (ODD, vec_uns_int_result,
+				  vec_uns_int_expected);
 }
 
-- 
2.44.0

Re: [PATCH 3/13] rs6000, fix error in unsigned vector float to unsigned int built-in definitions

[Kewen.Lin]

Hi,

on 2024/4/20 05:17, Carl Love wrote:
> rs6000, fix error in unsigned vector float to unsigned  int built-in definitions
> 
> The built-ins __builtin_vsx_vunsigned_v2df and__builtin_vsx_vunsigned_v4sf
> are supposed to take a vector of floats and return a vector of unsigned
> long long ints.  The definitions are using the signed version of the

Sorry for nitpicking, here __builtin_vsx_vunsigned_v2df takes vector of doubles
and returns vector of unsigned long long ints while __builtin_vsx_vunsigned_v4sf
takes vector of floats and returns vector of unsigned ints.

> instructions not the unsigned version of the instruction.  The results
> should also be unsigned.  The builtins are used by the overloaded
> vec_unsigned builtin which has an unsigned result.
> 
> Similarly the built-ins __builtin_vsx_vunsignede_v2df and
> __builtin_vsx_vunsignedo_v2df are supposed to retun an unsigned result.

Nit: s/retun/return/

> If the floating point argument is negative, the unsigned result is zero.
> The built-ins are used in the overloaded built-in vec_unsignede and
> vec_unsignedo respectively.
> 
> Add a test cases for a negative floating point arguments for each of the
> above built-ins.
> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_vunsigned_v2df,
> 	__builtin_vsx_vunsigned_v4sf, __builtin_vsx_vunsignede_v2df,
> 	__builtin_vsx_vunsignedo_v2df): Change the result type to unsigned.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.target/powerpc/builtins-3-runnable.c: Add tests for
> 	vec_unsignede and vec_unsignedo with negative arguments.
> ---
>  gcc/config/rs6000/rs6000-builtins.def         | 12 +++++-----
>  .../gcc.target/powerpc/builtins-3-runnable.c  | 23 ++++++++++++++++---
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index c6d2ea1bc39..bf9a0ae22fc 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1580,16 +1580,16 @@
>    const vsi __builtin_vsx_vsignedo_v2df (vd);
>      VEC_VSIGNEDO_V2DF vsignedo_v2df {}
>  
> -  const vsll __builtin_vsx_vunsigned_v2df (vd);
> -    VEC_VUNSIGNED_V2DF vsx_xvcvdpsxds {}
> +  const vull __builtin_vsx_vunsigned_v2df (vd);
> +    VEC_VUNSIGNED_V2DF vsx_xvcvdpuxds {}
>  
> -  const vsi __builtin_vsx_vunsigned_v4sf (vf);
> -    VEC_VUNSIGNED_V4SF vsx_xvcvspsxws {}
> +  const vui __builtin_vsx_vunsigned_v4sf (vf);
> +    VEC_VUNSIGNED_V4SF vsx_xvcvspuxws {}
>  
> -  const vsi __builtin_vsx_vunsignede_v2df (vd);
> +  const vui __builtin_vsx_vunsignede_v2df (vd);
>      VEC_VUNSIGNEDE_V2DF vunsignede_v2df {}
>  
> -  const vsi __builtin_vsx_vunsignedo_v2df (vd);
> +  const vui __builtin_vsx_vunsignedo_v2df (vd);
>      VEC_VUNSIGNEDO_V2DF vunsignedo_v2df {}
>  
>    const vf __builtin_vsx_xscvdpsp (double);
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
> index 0231a1fd086..6d4fe84c8a1 100644
> --- a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
> @@ -313,6 +313,15 @@ int main()
>  	test_unsigned_int_result (ALL, vec_uns_int_result,
>  				  vec_uns_int_expected);
>  
> +	/* Convert single precision float to  unsigned int.  Negative
> +	   arguments
> +	 */
> +	vec_flt0 = (vector float){-14.930, -834.49, -3.3, -5.4};
> +	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
> +	vec_uns_int_result = vec_unsigned (vec_flt0);
> +	test_unsigned_int_result (ALL, vec_uns_int_result,
> +				  vec_uns_int_expected);
> +
>  	/* Convert double precision float to long long unsigned int */
>  	vec_dble0 = (vector double){124.930, 8134.49};
>  	vec_ll_uns_int_expected = (vector long long unsigned int){124, 8134};
> @@ -321,9 +330,9 @@ int main()
>  				     vec_ll_uns_int_expected);

Nit: Similar coverage on negative for vector double can be added here.

BR,
Kewen

>  
>  	/* Convert double precision vector float to vector unsigned int,
> -	   even words */
> -	vec_dble0 = (vector double){3124.930, 8234.49};
> -	vec_uns_int_expected = (vector unsigned int){3124, 0, 8234, 0};
> +	   even words.  Negative arguments */
> +	vec_dble0 = (vector double){-124.930, -234.49};
> +	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
>  	vec_uns_int_result = vec_unsignede (vec_dble0);
>  	test_unsigned_int_result (EVEN, vec_uns_int_result,
>  				  vec_uns_int_expected);
> @@ -335,5 +344,13 @@ int main()
>  	vec_uns_int_result = vec_unsignedo (vec_dble0);
>  	test_unsigned_int_result (ODD, vec_uns_int_result,
>  				  vec_uns_int_expected);
> +
> +	/* Convert double precision vector float to vector unsigned int,
> +	   odd words.  Negative arguments.  */
> +	vec_dble0 = (vector double){-924.930, -1234.49};
> +	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
> +	vec_uns_int_result = vec_unsignedo (vec_dble0);
> +	test_unsigned_int_result (ODD, vec_uns_int_result,
> +				  vec_uns_int_expected);
>  }
>  

Re: [PATCH 3/13] rs6000, fix error in unsigned vector float to unsigned int built-in definitions

[Carl Love]

Keewn:

On 5/14/24 00:00, Kewen.Lin wrote:
> Hi,
> 
> on 2024/4/20 05:17, Carl Love wrote:
>> rs6000, fix error in unsigned vector float to unsigned  int built-in definitions
>>
>> The built-ins __builtin_vsx_vunsigned_v2df and__builtin_vsx_vunsigned_v4sf
>> are supposed to take a vector of floats and return a vector of unsigned
>> long long ints.  The definitions are using the signed version of the
> 
> Sorry for nitpicking, here __builtin_vsx_vunsigned_v2df takes vector of doubles
> and returns vector of unsigned long long ints while __builtin_vsx_vunsigned_v4sf
> takes vector of floats and returns vector of unsigned ints.

That is not nitpicking, the description is wrong.  Changed float to double.
> 
>> instructions not the unsigned version of the instruction.  The results
>> should also be unsigned.  The builtins are used by the overloaded
>> vec_unsigned builtin which has an unsigned result.
>>
>> Similarly the built-ins __builtin_vsx_vunsignede_v2df and
>> __builtin_vsx_vunsignedo_v2df are supposed to retun an unsigned result.
> 
> Nit: s/retun/return/

Fixed.

> 
>> If the floating point argument is negative, the unsigned result is zero.
>> The built-ins are used in the overloaded built-in vec_unsignede and
>> vec_unsignedo respectively.
>>
>> Add a test cases for a negative floating point arguments for each of the
>> above built-ins.
>>
>> gcc/ChangeLog:
>> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_vunsigned_v2df,
>> 	__builtin_vsx_vunsigned_v4sf, __builtin_vsx_vunsignede_v2df,
>> 	__builtin_vsx_vunsignedo_v2df): Change the result type to unsigned.
>>
>> gcc/testsuite/ChangeLog:
>> 	* gcc.target/powerpc/builtins-3-runnable.c: Add tests for
>> 	vec_unsignede and vec_unsignedo with negative arguments.
>> ---
>>  gcc/config/rs6000/rs6000-builtins.def         | 12 +++++-----
>>  .../gcc.target/powerpc/builtins-3-runnable.c  | 23 ++++++++++++++++---
>>  2 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
>> index c6d2ea1bc39..bf9a0ae22fc 100644
>> --- a/gcc/config/rs6000/rs6000-builtins.def
>> +++ b/gcc/config/rs6000/rs6000-builtins.def
>> @@ -1580,16 +1580,16 @@
>>    const vsi __builtin_vsx_vsignedo_v2df (vd);
>>      VEC_VSIGNEDO_V2DF vsignedo_v2df {}
>>  
>> -  const vsll __builtin_vsx_vunsigned_v2df (vd);
>> -    VEC_VUNSIGNED_V2DF vsx_xvcvdpsxds {}
>> +  const vull __builtin_vsx_vunsigned_v2df (vd);
>> +    VEC_VUNSIGNED_V2DF vsx_xvcvdpuxds {}
>>  
>> -  const vsi __builtin_vsx_vunsigned_v4sf (vf);
>> -    VEC_VUNSIGNED_V4SF vsx_xvcvspsxws {}
>> +  const vui __builtin_vsx_vunsigned_v4sf (vf);
>> +    VEC_VUNSIGNED_V4SF vsx_xvcvspuxws {}
>>  
>> -  const vsi __builtin_vsx_vunsignede_v2df (vd);
>> +  const vui __builtin_vsx_vunsignede_v2df (vd);
>>      VEC_VUNSIGNEDE_V2DF vunsignede_v2df {}
>>  
>> -  const vsi __builtin_vsx_vunsignedo_v2df (vd);
>> +  const vui __builtin_vsx_vunsignedo_v2df (vd);
>>      VEC_VUNSIGNEDO_V2DF vunsignedo_v2df {}
>>  
>>    const vf __builtin_vsx_xscvdpsp (double);
>> diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
>> index 0231a1fd086..6d4fe84c8a1 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
>> @@ -313,6 +313,15 @@ int main()
>>  	test_unsigned_int_result (ALL, vec_uns_int_result,
>>  				  vec_uns_int_expected);
>>  
>> +	/* Convert single precision float to  unsigned int.  Negative
>> +	   arguments
>> +	 */
>> +	vec_flt0 = (vector float){-14.930, -834.49, -3.3, -5.4};
>> +	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
>> +	vec_uns_int_result = vec_unsigned (vec_flt0);
>> +	test_unsigned_int_result (ALL, vec_uns_int_result,
>> +				  vec_uns_int_expected);
>> +
>>  	/* Convert double precision float to long long unsigned int */
>>  	vec_dble0 = (vector double){124.930, 8134.49};
>>  	vec_ll_uns_int_expected = (vector long long unsigned int){124, 8134};
>> @@ -321,9 +330,9 @@ int main()
>>  				     vec_ll_uns_int_expected);
> 
> Nit: Similar coverage on negative for vector double can be added here.

Added.

                  Carl

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

[Carl Love]

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
        overloaded built-ins.
---
 gcc/config/rs6000/rs6000-builtins.def |  6 ++++++
 gcc/config/rs6000/rs6000-overload.def |  8 ++++++++
 gcc/config/rs6000/vsx.md              | 23 +++++++++++++++++++++++
 gcc/doc/extend.texi                   | 13 +++++++++++++
 4 files changed, 50 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index bf9a0ae22fc..5b7237a2327 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1709,9 +1709,15 @@
   const vsll __builtin_vsx_xvcvspsxds (vf);
     XVCVSPSXDS vsx_xvcvspsxds {}
 
+  const vsll __builtin_vsx_xvcvspsxds_low (vf);
+    XVCVSPSXDSO vsx_xvcvspsxds_low {}
+
   const vsll __builtin_vsx_xvcvspuxds (vf);
     XVCVSPUXDS vsx_xvcvspuxds {}
 
+  const vsll __builtin_vsx_xvcvspuxds_low (vf);
+    XVCVSPUXDSO vsx_xvcvspuxds_low {}
+
   const vsi __builtin_vsx_xvcvspuxws (vf);
     XVCVSPUXWS vsx_fixuns_truncv4sfv4si2 {}
 
diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
index 84bd9ae6554..68501c05289 100644
--- a/gcc/config/rs6000/rs6000-overload.def
+++ b/gcc/config/rs6000/rs6000-overload.def
@@ -3307,10 +3307,14 @@
 [VEC_SIGNEDE, vec_signede, __builtin_vec_vsignede]
   vsi __builtin_vec_vsignede (vd);
     VEC_VSIGNEDE_V2DF
+  vsll __builtin_vec_vsignede (vf);
+    XVCVSPSXDS
 
 [VEC_SIGNEDO, vec_signedo, __builtin_vec_vsignedo]
   vsi __builtin_vec_vsignedo (vd);
     VEC_VSIGNEDO_V2DF
+  vsll __builtin_vec_vsignedo (vf);
+    XVCVSPSXDSO
 
 [VEC_SIGNEXTI, vec_signexti, __builtin_vec_signexti]
   vsi __builtin_vec_signexti (vsc);
@@ -4433,10 +4437,14 @@
 [VEC_UNSIGNEDE, vec_unsignede, __builtin_vec_vunsignede]
   vui __builtin_vec_vunsignede (vd);
     VEC_VUNSIGNEDE_V2DF
+  vull __builtin_vec_vunsignede (vf);
+    XVCVSPUXDS
 
 [VEC_UNSIGNEDO, vec_unsignedo, __builtin_vec_vunsignedo]
   vui __builtin_vec_vunsignedo (vd);
     VEC_VUNSIGNEDO_V2DF
+  vull __builtin_vec_vunsignedo (vf);
+    XVCVSPUXDSO
 
 [VEC_VEE, vec_extract_exp, __builtin_vec_extract_exp]
   vui __builtin_vec_extract_exp (vf);
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index f135fa079bd..3d39ae7995f 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -2704,6 +2704,29 @@
   DONE;
 })
 
+;; Convert low vector elements of 32-bit floating point numbers to vector of
+;; 64-bit signed/unsigned integers.
+(define_expand "vsx_xvcvsp<su>xds_low"
+  [(match_operand:V2DI 0 "vsx_register_operand")
+   (match_operand:V4SF 1 "vsx_register_operand")
+   (any_fix (pc))]
+  "VECTOR_UNIT_VSX_P (V2DFmode)"
+{
+  /* Shift left one word to put even word in correct location */
+  rtx rtx_tmp;
+  rtx rtx_val = GEN_INT (4);
+  rtx_tmp = gen_reg_rtx (V4SFmode);
+  emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
+                                      rtx_val));
+
+  if (BYTES_BIG_ENDIAN)
+    emit_insn (gen_vsx_xvcvsp<su>xds_be (operands[0], rtx_tmp));
+  else
+    emit_insn (gen_vsx_xvcvsp<su>xds_le (operands[0], rtx_tmp));
+
+  DONE;
+})
+
 ;; Generate float2 double
 ;; convert two double to float
 (define_expand "float2_v2df"
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 7b54a241a7b..64a43b55e2d 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -22552,6 +22552,19 @@ can use @var{vector long} instead of @var{vector long long},
 @var{vector bool long} instead of @var{vector bool long long}, and
 @var{vector unsigned long} instead of @var{vector unsigned long long}.
 
+@smallexample
+vector signed signed long long vec_signedo (vector float);
+vector signed signed long long vec_signede (vector float);
+vector unsigned signed long long vec_signedo (vector float);
+vector unsigned signed long long vec_signede (vector float);
+@end smallexample
+
+The overloaded built-ins @code{vec_signedo} and @code{vec_signede} convert the
+even/odd input vector elements to signed/unsigned long long integer values in
+addition to the supported arguments and return types documented in the PVIPR.
+Negative input values are returned as zero for the unsigned long long return
+values.
+
 Only functions excluded from the PVIPR are listed here.
 
 @smallexample
-- 
2.44.0

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

[Kewen.Lin]

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
>         overloaded built-ins.

This part is missing, there are no test case changes in this patch.

> ---
>  gcc/config/rs6000/rs6000-builtins.def |  6 ++++++
>  gcc/config/rs6000/rs6000-overload.def |  8 ++++++++
>  gcc/config/rs6000/vsx.md              | 23 +++++++++++++++++++++++
>  gcc/doc/extend.texi                   | 13 +++++++++++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index bf9a0ae22fc..5b7237a2327 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1709,9 +1709,15 @@
>    const vsll __builtin_vsx_xvcvspsxds (vf);
>      XVCVSPSXDS vsx_xvcvspsxds {}
>  
> +  const vsll __builtin_vsx_xvcvspsxds_low (vf);
> +    XVCVSPSXDSO vsx_xvcvspsxds_low {}
> +
>    const vsll __builtin_vsx_xvcvspuxds (vf);
>      XVCVSPUXDS vsx_xvcvspuxds {}

This existing should return with type vull, ...

>  
> +  const vsll __builtin_vsx_xvcvspuxds_low (vf);
> +    XVCVSPUXDSO vsx_xvcvspuxds_low {}

... so this copied one should be vull too.

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

>    const vsi __builtin_vsx_xvcvspuxws (vf);
>      XVCVSPUXWS vsx_fixuns_truncv4sfv4si2 {}
>  > diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
> index 84bd9ae6554..68501c05289 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -3307,10 +3307,14 @@
>  [VEC_SIGNEDE, vec_signede, __builtin_vec_vsignede]
>    vsi __builtin_vec_vsignede (vd);
>      VEC_VSIGNEDE_V2DF
> +  vsll __builtin_vec_vsignede (vf);
> +    XVCVSPSXDS
>  
>  [VEC_SIGNEDO, vec_signedo, __builtin_vec_vsignedo]
>    vsi __builtin_vec_vsignedo (vd);
>      VEC_VSIGNEDO_V2DF
> +  vsll __builtin_vec_vsignedo (vf);
> +    XVCVSPSXDSO
>  
>  [VEC_SIGNEXTI, vec_signexti, __builtin_vec_signexti]
>    vsi __builtin_vec_signexti (vsc);
> @@ -4433,10 +4437,14 @@
>  [VEC_UNSIGNEDE, vec_unsignede, __builtin_vec_vunsignede]
>    vui __builtin_vec_vunsignede (vd);
>      VEC_VUNSIGNEDE_V2DF
> +  vull __builtin_vec_vunsignede (vf);
> +    XVCVSPUXDS
>  
>  [VEC_UNSIGNEDO, vec_unsignedo, __builtin_vec_vunsignedo]
>    vui __builtin_vec_vunsignedo (vd);
>      VEC_VUNSIGNEDO_V2DF
> +  vull __builtin_vec_vunsignedo (vf);
> +    XVCVSPUXDSO
>  
As above, the name can be tweaked.

>  [VEC_VEE, vec_extract_exp, __builtin_vec_extract_exp]
>    vui __builtin_vec_extract_exp (vf);
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index f135fa079bd..3d39ae7995f 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -2704,6 +2704,29 @@
>    DONE;
>  })
>  
> +;; Convert low vector elements of 32-bit floating point numbers to vector of
> +;; 64-bit signed/unsigned integers.
> +(define_expand "vsx_xvcvsp<su>xds_low"
> +  [(match_operand:V2DI 0 "vsx_register_operand")
> +   (match_operand:V4SF 1 "vsx_register_operand")
> +   (any_fix (pc))]
> +  "VECTOR_UNIT_VSX_P (V2DFmode)"
> +{
> +  /* Shift left one word to put even word in correct location */
> +  rtx rtx_tmp;
> +  rtx rtx_val = GEN_INT (4);
> +  rtx_tmp = gen_reg_rtx (V4SFmode);
> +  emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
> +                                      rtx_val));
> +

I think this shift is only needed for LE, see the existing handlings on 
float/signed int to double conversions, like:

(define_expand "doublee<mode>2"
(define_expand "doubleo<mode>2"

> +  if (BYTES_BIG_ENDIAN)
> +    emit_insn (gen_vsx_xvcvsp<su>xds_be (operands[0], rtx_tmp));
> +  else
> +    emit_insn (gen_vsx_xvcvsp<su>xds_le (operands[0], rtx_tmp));
> +
> +  DONE;
> +})
> +
>  ;; Generate float2 double
>  ;; convert two double to float
>  (define_expand "float2_v2df"
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 7b54a241a7b..64a43b55e2d 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -22552,6 +22552,19 @@ can use @var{vector long} instead of @var{vector long long},
>  @var{vector bool long} instead of @var{vector bool long long}, and
>  @var{vector unsigned long} instead of @var{vector unsigned long long}.
>  
> +@smallexample
> +vector signed signed long long vec_signedo (vector float);
> +vector signed signed long long vec_signede (vector float);
> +vector unsigned signed long long vec_signedo (vector float);
> +vector unsigned signed long long vec_signede (vector float);
> +@end smallexample
> +
> +The overloaded built-ins @code{vec_signedo} and @code{vec_signede} convert the
> +even/odd input vector elements to signed/unsigned long long integer values in
> +addition to the supported arguments and return types documented in the PVIPR.
> +Negative input values are returned as zero for the unsigned long long return
> +values.

These functions are placed in stanza [vsx] instead of [power8-vector], so it should
be in the section "PowerPC AltiVec Built-in Functions Available on ISA 2.06".

As mentioned in the replies to other patches, since it's an extension from the
existing PVIPR bifs and there is nothing special from them, I prefer to just mention
them but omit the description on them.

BR,
Kewen

> +
>  Only functions excluded from the PVIPR are listed here.
>  
>  @smallexample

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

[Carl Love]

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
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 

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

[Kewen.Lin]

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 

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

[Carl Love]

Kewen:

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
>>         overloaded built-ins.
> 
> This part is missing, there are no test case changes in this patch.

Yes, the new tests are missing.  Not sure what happened to them.  Fixed.

> 
>> ---
>>  gcc/config/rs6000/rs6000-builtins.def |  6 ++++++
>>  gcc/config/rs6000/rs6000-overload.def |  8 ++++++++
>>  gcc/config/rs6000/vsx.md              | 23 +++++++++++++++++++++++
>>  gcc/doc/extend.texi                   | 13 +++++++++++++
>>  4 files changed, 50 insertions(+)
>>
>> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
>> index bf9a0ae22fc..5b7237a2327 100644
>> --- a/gcc/config/rs6000/rs6000-builtins.def
>> +++ b/gcc/config/rs6000/rs6000-builtins.def
>> @@ -1709,9 +1709,15 @@
>>    const vsll __builtin_vsx_xvcvspsxds (vf);
>>      XVCVSPSXDS vsx_xvcvspsxds {}
>>  
>> +  const vsll __builtin_vsx_xvcvspsxds_low (vf);
>> +    XVCVSPSXDSO vsx_xvcvspsxds_low {}
>> +
>>    const vsll __builtin_vsx_xvcvspuxds (vf);
>>      XVCVSPUXDS vsx_xvcvspuxds {}
> 
> This existing should return with type vull, ...

Fixed.

> 
>>  
>> +  const vsll __builtin_vsx_xvcvspuxds_low (vf);
>> +    XVCVSPUXDSO vsx_xvcvspuxds_low {}
> 
> ... so this copied one should be vull too.

Fixed.

> 
> 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

NEED TO ADDRESS RESPONSE TO QUESTION I ASKED.

> 
>>    const vsi __builtin_vsx_xvcvspuxws (vf);
>>      XVCVSPUXWS vsx_fixuns_truncv4sfv4si2 {}
>>  > diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
>> index 84bd9ae6554..68501c05289 100644
>> --- a/gcc/config/rs6000/rs6000-overload.def
>> +++ b/gcc/config/rs6000/rs6000-overload.def
>> @@ -3307,10 +3307,14 @@
>>  [VEC_SIGNEDE, vec_signede, __builtin_vec_vsignede]
>>    vsi __builtin_vec_vsignede (vd);
>>      VEC_VSIGNEDE_V2DF
>> +  vsll __builtin_vec_vsignede (vf);
>> +    XVCVSPSXDS
>>  
>>  [VEC_SIGNEDO, vec_signedo, __builtin_vec_vsignedo]
>>    vsi __builtin_vec_vsignedo (vd);
>>      VEC_VSIGNEDO_V2DF
>> +  vsll __builtin_vec_vsignedo (vf);
>> +    XVCVSPSXDSO
>>  
>>  [VEC_SIGNEXTI, vec_signexti, __builtin_vec_signexti]
>>    vsi __builtin_vec_signexti (vsc);
>> @@ -4433,10 +4437,14 @@
>>  [VEC_UNSIGNEDE, vec_unsignede, __builtin_vec_vunsignede]
>>    vui __builtin_vec_vunsignede (vd);
>>      VEC_VUNSIGNEDE_V2DF
>> +  vull __builtin_vec_vunsignede (vf);
>> +    XVCVSPUXDS
>>  
>>  [VEC_UNSIGNEDO, vec_unsignedo, __builtin_vec_vunsignedo]
>>    vui __builtin_vec_vunsignedo (vd);
>>      VEC_VUNSIGNEDO_V2DF
>> +  vull __builtin_vec_vunsignedo (vf);
>> +    XVCVSPUXDSO
>>  
> As above, the name can be tweaked.

Fixed.

> 
>>  [VEC_VEE, vec_extract_exp, __builtin_vec_extract_exp]
>>    vui __builtin_vec_extract_exp (vf);
>> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
>> index f135fa079bd..3d39ae7995f 100644
>> --- a/gcc/config/rs6000/vsx.md
>> +++ b/gcc/config/rs6000/vsx.md
>> @@ -2704,6 +2704,29 @@
>>    DONE;
>>  })
>>  
>> +;; Convert low vector elements of 32-bit floating point numbers to vector of
>> +;; 64-bit signed/unsigned integers.
>> +(define_expand "vsx_xvcvsp<su>xds_low"
>> +  [(match_operand:V2DI 0 "vsx_register_operand")
>> +   (match_operand:V4SF 1 "vsx_register_operand")
>> +   (any_fix (pc))]
>> +  "VECTOR_UNIT_VSX_P (V2DFmode)"
>> +{
>> +  /* Shift left one word to put even word in correct location */
>> +  rtx rtx_tmp;
>> +  rtx rtx_val = GEN_INT (4);
>> +  rtx_tmp = gen_reg_rtx (V4SFmode);
>> +  emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
>> +                                      rtx_val));
>> +
> 
> I think this shift is only needed for LE, see the existing handlings on 
> float/signed int to double conversions, like:
> 
> (define_expand "doublee<mode>2"
> (define_expand "doubleo<mode>2
Yes, I tested builtins-3-runnable.c on a BE machine and it fails.  I moved the shift to the LE stanza only.  The patch now passes on LE and BE.  I think the fact that the tests cases got lost is why this didnt' get caught the first time.

> 
>> +  if (BYTES_BIG_ENDIAN)
>> +    emit_insn (gen_vsx_xvcvsp<su>xds_be (operands[0], rtx_tmp));
>> +  else
>> +    emit_insn (gen_vsx_xvcvsp<su>xds_le (operands[0], rtx_tmp));
>> +
>> +  DONE;
>> +})
>> +
>>  ;; Generate float2 double
>>  ;; convert two double to float
>>  (define_expand "float2_v2df"
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 7b54a241a7b..64a43b55e2d 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -22552,6 +22552,19 @@ can use @var{vector long} instead of @var{vector long long},
>>  @var{vector bool long} instead of @var{vector bool long long}, and
>>  @var{vector unsigned long} instead of @var{vector unsigned long long}.
>>  
>> +@smallexample
>> +vector signed signed long long vec_signedo (vector float);
>> +vector signed signed long long vec_signede (vector float);
>> +vector unsigned signed long long vec_signedo (vector float);
>> +vector unsigned signed long long vec_signede (vector float);
>> +@end smallexample
>> +
>> +The overloaded built-ins @code{vec_signedo} and @code{vec_signede} convert the
>> +even/odd input vector elements to signed/unsigned long long integer values in
>> +addition to the supported arguments and return types documented in the PVIPR.
>> +Negative input values are returned as zero for the unsigned long long return
>> +values.
> 
> These functions are placed in stanza [vsx] instead of [power8-vector], so it should
> be in the section "PowerPC AltiVec Built-in Functions Available on ISA 2.06".
> 
> As mentioned in the replies to other patches, since it's an extension from the
> existing PVIPR bifs and there is nothing special from them, I prefer to just mention
> them but omit the description on them.

Moved, description now just says they are new extensions of the exiting built-ins documented in the
PVIPR.

             Carl 
> 
> BR,
> Kewen
> 
>> +
>>  Only functions excluded from the PVIPR are listed here.
>>  
>>  @smallexample

[PATCH 5/13] rs6000, remove duplicated built-ins of vecmergl and vec_mergeh

[Carl Love]

rs6000, remove duplicated built-ins of vecmergl and vec_mergeh

The following undocumented built-ins are same as existing documented
overloaded builtins.

  const vf __builtin_vsx_xxmrghw (vf, vf);
same as  vf __builtin_vec_mergeh (vf, vf);      (overloaded vec_mergeh)

  const vsi __builtin_vsx_xxmrghw_4si (vsi, vsi);
same as vsi __builtin_vec_mergeh (vsi, vsi);   (overloaded vec_mergeh)

  const vf __builtin_vsx_xxmrglw (vf, vf);
same as vf __builtin_vec_mergel (vf, vf);      (overloaded vec_mergel)

  const vsi __builtin_vsx_xxmrglw_4si (vsi, vsi);
same as vsi __builtin_vec_mergel (vsi, vsi);   (overloaded vec_mergel)

This patch removes the duplicate built-in definitions so only the
documented built-ins will be available for use.  The case statements in
rs6000_gimple_fold_builtin are removed as they are no longer needed.  The
patch removes the now unused define_expands for vsx_xxmrghw_<mode> and
vsx_xxmrglw_<mode>.

gcc/ChangeLog:
	* config/rs6000/rs6000-builtins.def (__builtin_vsx_xxmrghw,
	__builtin_vsx_xxmrghw_4si, __builtin_vsx_xxmrglw,
	__builtin_vsx_xxmrglw_4si, __builtin_vsx_xxsel_16qi): Remove
	built-in definition.
	* config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin):
	remove case entries RS6000_BIF_XXMRGLW_4SI,
	RS6000_BIF_XXMRGLW_4SF, RS6000_BIF_XXMRGHW_4SI,
	RS6000_BIF_XXMRGHW_4SF.
	* config/rs6000/vsx.md (vsx_xxmrghw_<mode>, vsx_xxmrglw_<mode>):
	Remove unused define_expands.
---
 gcc/config/rs6000/rs6000-builtin.cc   |  4 ---
 gcc/config/rs6000/rs6000-builtins.def | 12 --------
 gcc/config/rs6000/vsx.md              | 41 ---------------------------
 3 files changed, 57 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
index ac9f16fe51a..f83d65b06ef 100644
--- a/gcc/config/rs6000/rs6000-builtin.cc
+++ b/gcc/config/rs6000/rs6000-builtin.cc
@@ -2097,20 +2097,16 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
     /* vec_mergel (integrals).  */
     case RS6000_BIF_VMRGLH:
     case RS6000_BIF_VMRGLW:
-    case RS6000_BIF_XXMRGLW_4SI:
     case RS6000_BIF_VMRGLB:
     case RS6000_BIF_VEC_MERGEL_V2DI:
-    case RS6000_BIF_XXMRGLW_4SF:
     case RS6000_BIF_VEC_MERGEL_V2DF:
       fold_mergehl_helper (gsi, stmt, 1);
       return true;
     /* vec_mergeh (integrals).  */
     case RS6000_BIF_VMRGHH:
     case RS6000_BIF_VMRGHW:
-    case RS6000_BIF_XXMRGHW_4SI:
     case RS6000_BIF_VMRGHB:
     case RS6000_BIF_VEC_MERGEH_V2DI:
-    case RS6000_BIF_XXMRGHW_4SF:
     case RS6000_BIF_VEC_MERGEH_V2DF:
       fold_mergehl_helper (gsi, stmt, 0);
       return true;
diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index 5b7237a2327..d09e21a9151 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1904,18 +1904,6 @@
   const signed int __builtin_vsx_xvtsqrtsp_fg (vf);
     XVTSQRTSP_FG vsx_tsqrtv4sf2_fg {}
 
-  const vf __builtin_vsx_xxmrghw (vf, vf);
-    XXMRGHW_4SF vsx_xxmrghw_v4sf {}
-
-  const vsi __builtin_vsx_xxmrghw_4si (vsi, vsi);
-    XXMRGHW_4SI vsx_xxmrghw_v4si {}
-
-  const vf __builtin_vsx_xxmrglw (vf, vf);
-    XXMRGLW_4SF vsx_xxmrglw_v4sf {}
-
-  const vsi __builtin_vsx_xxmrglw_4si (vsi, vsi);
-    XXMRGLW_4SI vsx_xxmrglw_v4si {}
-
   const vsc __builtin_vsx_xxpermdi_16qi (vsc, vsc, const int<2>);
     XXPERMDI_16QI vsx_xxpermdi_v16qi {}
 
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 3d39ae7995f..26560ecc38a 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -4810,47 +4810,6 @@
 }
   [(set_attr "type" "vecperm")])
 
-;; V4SF/V4SI interleave
-(define_expand "vsx_xxmrghw_<mode>"
-  [(set (match_operand:VSX_W 0 "vsx_register_operand" "=wa")
-        (vec_select:VSX_W
-	  (vec_concat:<VS_double>
-	    (match_operand:VSX_W 1 "vsx_register_operand" "wa")
-	    (match_operand:VSX_W 2 "vsx_register_operand" "wa"))
-	  (parallel [(const_int 0) (const_int 4)
-		     (const_int 1) (const_int 5)])))]
-  "VECTOR_MEM_VSX_P (<MODE>mode)"
-{
-  rtx (*fun) (rtx, rtx, rtx);
-  fun = BYTES_BIG_ENDIAN ? gen_altivec_vmrghw_direct_<mode>
-			 : gen_altivec_vmrglw_direct_<mode>;
-  if (!BYTES_BIG_ENDIAN)
-    std::swap (operands[1], operands[2]);
-  emit_insn (fun (operands[0], operands[1], operands[2]));
-  DONE;
-}
-  [(set_attr "type" "vecperm")])
-
-(define_expand "vsx_xxmrglw_<mode>"
-  [(set (match_operand:VSX_W 0 "vsx_register_operand" "=wa")
-	(vec_select:VSX_W
-	  (vec_concat:<VS_double>
-	    (match_operand:VSX_W 1 "vsx_register_operand" "wa")
-	    (match_operand:VSX_W 2 "vsx_register_operand" "wa"))
-	  (parallel [(const_int 2) (const_int 6)
-		     (const_int 3) (const_int 7)])))]
-  "VECTOR_MEM_VSX_P (<MODE>mode)"
-{
-  rtx (*fun) (rtx, rtx, rtx);
-  fun = BYTES_BIG_ENDIAN ? gen_altivec_vmrglw_direct_<mode>
-			 : gen_altivec_vmrghw_direct_<mode>;
-  if (!BYTES_BIG_ENDIAN)
-    std::swap (operands[1], operands[2]);
-  emit_insn (fun (operands[0], operands[1], operands[2]));
-  DONE;
-}
-  [(set_attr "type" "vecperm")])
-
 ;; Shift left double by word immediate
 (define_insn "vsx_xxsldwi_<mode>"
   [(set (match_operand:VSX_L 0 "vsx_register_operand" "=wa")
-- 
2.44.0

Re: [PATCH 5/13] rs6000, remove duplicated built-ins of vecmergl and vec_mergeh

[Kewen.Lin]

Hi,

on 2024/4/20 05:17, Carl Love wrote:
> rs6000, remove duplicated built-ins of vecmergl and vec_mergeh
> 
> The following undocumented built-ins are same as existing documented
> overloaded builtins.
> 
>   const vf __builtin_vsx_xxmrghw (vf, vf);
> same as  vf __builtin_vec_mergeh (vf, vf);      (overloaded vec_mergeh)
> 
>   const vsi __builtin_vsx_xxmrghw_4si (vsi, vsi);
> same as vsi __builtin_vec_mergeh (vsi, vsi);   (overloaded vec_mergeh)
> 
>   const vf __builtin_vsx_xxmrglw (vf, vf);
> same as vf __builtin_vec_mergel (vf, vf);      (overloaded vec_mergel)
> 
>   const vsi __builtin_vsx_xxmrglw_4si (vsi, vsi);
> same as vsi __builtin_vec_mergel (vsi, vsi);   (overloaded vec_mergel)
> 
> This patch removes the duplicate built-in definitions so only the
> documented built-ins will be available for use.  The case statements in
> rs6000_gimple_fold_builtin are removed as they are no longer needed.  The
> patch removes the now unused define_expands for vsx_xxmrghw_<mode> and
> vsx_xxmrglw_<mode>.

Ok for trunk, thanks!

BR,
Kewen

> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_xxmrghw,
> 	__builtin_vsx_xxmrghw_4si, __builtin_vsx_xxmrglw,
> 	__builtin_vsx_xxmrglw_4si, __builtin_vsx_xxsel_16qi): Remove
> 	built-in definition.
> 	* config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin):
> 	remove case entries RS6000_BIF_XXMRGLW_4SI,
> 	RS6000_BIF_XXMRGLW_4SF, RS6000_BIF_XXMRGHW_4SI,
> 	RS6000_BIF_XXMRGHW_4SF.
> 	* config/rs6000/vsx.md (vsx_xxmrghw_<mode>, vsx_xxmrglw_<mode>):
> 	Remove unused define_expands.
> ---
>  gcc/config/rs6000/rs6000-builtin.cc   |  4 ---
>  gcc/config/rs6000/rs6000-builtins.def | 12 --------
>  gcc/config/rs6000/vsx.md              | 41 ---------------------------
>  3 files changed, 57 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
> index ac9f16fe51a..f83d65b06ef 100644
> --- a/gcc/config/rs6000/rs6000-builtin.cc
> +++ b/gcc/config/rs6000/rs6000-builtin.cc
> @@ -2097,20 +2097,16 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
>      /* vec_mergel (integrals).  */
>      case RS6000_BIF_VMRGLH:
>      case RS6000_BIF_VMRGLW:
> -    case RS6000_BIF_XXMRGLW_4SI:
>      case RS6000_BIF_VMRGLB:
>      case RS6000_BIF_VEC_MERGEL_V2DI:
> -    case RS6000_BIF_XXMRGLW_4SF:
>      case RS6000_BIF_VEC_MERGEL_V2DF:
>        fold_mergehl_helper (gsi, stmt, 1);
>        return true;
>      /* vec_mergeh (integrals).  */
>      case RS6000_BIF_VMRGHH:
>      case RS6000_BIF_VMRGHW:
> -    case RS6000_BIF_XXMRGHW_4SI:
>      case RS6000_BIF_VMRGHB:
>      case RS6000_BIF_VEC_MERGEH_V2DI:
> -    case RS6000_BIF_XXMRGHW_4SF:
>      case RS6000_BIF_VEC_MERGEH_V2DF:
>        fold_mergehl_helper (gsi, stmt, 0);
>        return true;
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 5b7237a2327..d09e21a9151 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1904,18 +1904,6 @@
>    const signed int __builtin_vsx_xvtsqrtsp_fg (vf);
>      XVTSQRTSP_FG vsx_tsqrtv4sf2_fg {}
>  
> -  const vf __builtin_vsx_xxmrghw (vf, vf);
> -    XXMRGHW_4SF vsx_xxmrghw_v4sf {}
> -
> -  const vsi __builtin_vsx_xxmrghw_4si (vsi, vsi);
> -    XXMRGHW_4SI vsx_xxmrghw_v4si {}
> -
> -  const vf __builtin_vsx_xxmrglw (vf, vf);
> -    XXMRGLW_4SF vsx_xxmrglw_v4sf {}
> -
> -  const vsi __builtin_vsx_xxmrglw_4si (vsi, vsi);
> -    XXMRGLW_4SI vsx_xxmrglw_v4si {}
> -
>    const vsc __builtin_vsx_xxpermdi_16qi (vsc, vsc, const int<2>);
>      XXPERMDI_16QI vsx_xxpermdi_v16qi {}
>  
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 3d39ae7995f..26560ecc38a 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -4810,47 +4810,6 @@
>  }
>    [(set_attr "type" "vecperm")])
>  
> -;; V4SF/V4SI interleave
> -(define_expand "vsx_xxmrghw_<mode>"
> -  [(set (match_operand:VSX_W 0 "vsx_register_operand" "=wa")
> -        (vec_select:VSX_W
> -	  (vec_concat:<VS_double>
> -	    (match_operand:VSX_W 1 "vsx_register_operand" "wa")
> -	    (match_operand:VSX_W 2 "vsx_register_operand" "wa"))
> -	  (parallel [(const_int 0) (const_int 4)
> -		     (const_int 1) (const_int 5)])))]
> -  "VECTOR_MEM_VSX_P (<MODE>mode)"
> -{
> -  rtx (*fun) (rtx, rtx, rtx);
> -  fun = BYTES_BIG_ENDIAN ? gen_altivec_vmrghw_direct_<mode>
> -			 : gen_altivec_vmrglw_direct_<mode>;
> -  if (!BYTES_BIG_ENDIAN)
> -    std::swap (operands[1], operands[2]);
> -  emit_insn (fun (operands[0], operands[1], operands[2]));
> -  DONE;
> -}
> -  [(set_attr "type" "vecperm")])
> -
> -(define_expand "vsx_xxmrglw_<mode>"
> -  [(set (match_operand:VSX_W 0 "vsx_register_operand" "=wa")
> -	(vec_select:VSX_W
> -	  (vec_concat:<VS_double>
> -	    (match_operand:VSX_W 1 "vsx_register_operand" "wa")
> -	    (match_operand:VSX_W 2 "vsx_register_operand" "wa"))
> -	  (parallel [(const_int 2) (const_int 6)
> -		     (const_int 3) (const_int 7)])))]
> -  "VECTOR_MEM_VSX_P (<MODE>mode)"
> -{
> -  rtx (*fun) (rtx, rtx, rtx);
> -  fun = BYTES_BIG_ENDIAN ? gen_altivec_vmrglw_direct_<mode>
> -			 : gen_altivec_vmrghw_direct_<mode>;
> -  if (!BYTES_BIG_ENDIAN)
> -    std::swap (operands[1], operands[2]);
> -  emit_insn (fun (operands[0], operands[1], operands[2]));
> -  DONE;
> -}
> -  [(set_attr "type" "vecperm")])
> -
>  ;; Shift left double by word immediate
>  (define_insn "vsx_xxsldwi_<mode>"
>    [(set (match_operand:VSX_L 0 "vsx_register_operand" "=wa")

[PATCH 6/13] rs6000, add overloaded vec_sel with int128 arguments

[Carl Love]

rs6000, add overloaded vec_sel with int128 arguments

Extend the vec_sel built-in to take three signed/unsigned int128 arguments
and return a signed/unsigned int128 result.

Extending the vec_sel built-in makes the existing buit-ins
__builtin_vsx_xxsel_1ti and __builtin_vsx_xxsel_1ti_uns obsolete.  The
patch removes these built-ins.

The patch adds documentation and test cases for the new overloaded vec_sel
built-ins.

gcc/ChangeLog:
	* config/rs6000/rs6000-builtins.def (__builtin_vsx_xxsel_1ti,
	__builtin_vsx_xxsel_1ti_uns): Remove built-in definitions.
	* config/rs6000/rs6000-overload.def (vec_sel): Add new overloaded
	definitions.
	* doc/extend.texi: Add documentation for new vec_sel arguments.

gcc/testsuite/ChangeLog:
	* gcc.target/powerpc/vec_sel_runnable-int128.c: New test file.
---
 gcc/config/rs6000/rs6000-builtins.def         |  6 --
 gcc/config/rs6000/rs6000-overload.def         |  4 +
 gcc/doc/extend.texi                           | 14 ++++
 .../powerpc/vec-sel-runnable-i128.c           | 84 +++++++++++++++++++
 4 files changed, 102 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index d09e21a9151..46d2ae7b7cb 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1931,12 +1931,6 @@
   const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
     XXSEL_16QI_UNS vector_select_v16qi_uns {}
 
-  const vsq __builtin_vsx_xxsel_1ti (vsq, vsq, vsq);
-    XXSEL_1TI vector_select_v1ti {}
-
-  const vsq __builtin_vsx_xxsel_1ti_uns (vsq, vsq, vsq);
-    XXSEL_1TI_UNS vector_select_v1ti_uns {}
-
   const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
     XXSEL_2DF vector_select_v2df {}
 
diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
index 68501c05289..5912c9452f4 100644
--- a/gcc/config/rs6000/rs6000-overload.def
+++ b/gcc/config/rs6000/rs6000-overload.def
@@ -3274,6 +3274,10 @@
     VSEL_2DF  VSEL_2DF_B
   vd __builtin_vec_sel (vd, vd, vull);
     VSEL_2DF  VSEL_2DF_U
+  vsq __builtin_vec_sel (vsq, vsq, vsq);
+    VSEL_1TI  VSEL_1TI_S
+  vuq __builtin_vec_sel (vuq, vuq, vuq);
+    VSEL_1TI_UNS  VSEL_1TI_U
 ; The following variants are deprecated.
   vsll __builtin_vec_sel (vsll, vsll, vsll);
     VSEL_2DI_B  VSEL_2DI_S
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 64a43b55e2d..86b8e536dbe 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -23358,6 +23358,20 @@ The programmer is responsible for understanding the endianness issues involved
 with the first argument and the result.
 @findex vec_replace_unaligned
 
+Vector select
+
+@smallexample
+vector signed __int128 vec_sel (vector signed __int128,
+               vector signed __int128, vector signed __int128);
+vector unsigned __int128 vec_sel (vector unsigned __int128,
+               vector unsigned __int128, vector unsigned __int128);
+@end smallexample
+
+The overloaded built-in @code{vec_sel} with vector signed/unsigned __int128
+arguments and returns a vector selecting bits from the two source vectors based
+on the values of the third input vector.  This built-in is an extension of the
+@code{vec_sel} built-in documented in the PVIPR.
+
 Vector Shift Left Double Bit Immediate
 @smallexample
 @exdent vector signed char vec_sldb (vector signed char, vector signed char,
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
new file mode 100644
index 00000000000..58eb383e8c3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
@@ -0,0 +1,84 @@
+/* { dg-do run  { target power10_hw }} */
+/* { dg-require-effective-target int128 } */
+/* { dg-require-effective-target power10_hw } */
+/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
+
+
+#include <altivec.h>
+
+
+#define DEBUG 0
+
+#if DEBUG
+#include <stdio.h>
+void print_i128 (unsigned __int128 val)
+{
+  printf(" 0x%016llx%016llx",
+         (unsigned long long)(val >> 64),
+         (unsigned long long)(val & 0xFFFFFFFFFFFFFFFF));
+}
+#endif
+
+extern void abort (void);
+
+int
+main (int argc, char *argv [])
+{
+  vector signed __int128 src_va_s128;
+  vector signed __int128 src_vb_s128;
+  vector signed __int128 src_vc_s128;
+  vector signed __int128 vresult_s128;
+  vector signed __int128 expected_vresult_s128;
+
+  vector unsigned __int128 src_va_u128;
+  vector unsigned __int128 src_vb_u128;
+  vector unsigned __int128 src_vc_u128;
+  vector unsigned __int128 vresult_u128;
+  vector unsigned __int128 expected_vresult_u128;
+
+  src_va_s128 = (vector signed __int128) {0x123456789ABCDEF0};
+  src_vb_s128 = (vector signed __int128) {0xFEDCBA9876543210};
+  src_vc_s128 = (vector signed __int128) {0x3333333333333333};
+  expected_vresult_s128 = (vector signed __int128) {0x32147658ba9cfed0};
+
+  /* Signed arguments.  */
+  vresult_s128 = vec_sel (src_va_s128, src_vb_s128, src_vc_s128);
+
+  if (!vec_all_eq (vresult_s128, expected_vresult_s128))
+#if DEBUG
+    {
+      printf ("ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.\n");
+      printf ("  Result:          ");
+      print_i128 ((unsigned __int128) vresult_s128);
+      printf ("\n  Expected result: ");
+      print_i128 ((unsigned __int128) expected_vresult_s128);
+      printf ("\n");
+    }
+#else
+    abort ();
+#endif
+
+  src_va_u128 = (vector unsigned __int128) {0x13579ACE02468BDF};
+  src_vb_u128 = (vector unsigned __int128) {0xA987654FEDCB3210};
+  src_vc_u128 = (vector unsigned __int128) {0x5555555555555555};
+  expected_vresult_u128 = (vector unsigned __int128) {0x32147658ba9cfed0};
+
+  /* Unigned arguments.  */
+  vresult_u128 = vec_sel (src_va_u128, src_vb_u128, src_vc_u128);
+
+  if (!vec_all_eq (vresult_u128, expected_vresult_u128))
+#if DEBUG
+    {
+      printf ("ERROR, vec_sel (src_va_u128, src_vb_u128, src_vc_u128) result does not match expected output.\n");
+      printf ("  Result:          ");
+      print_i128 ((unsigned __int128) vresult_u128);
+      printf ("\n  Expected result: ");
+      print_i128 ((unsigned __int128) expected_vresult_u128);
+      printf ("\n");
+    }
+#else
+    abort ();
+#endif
+
+    return 0;
+}
-- 
2.44.0

Re: [PATCH 6/13] rs6000, add overloaded vec_sel with int128 arguments

[Kewen.Lin]

Hi,

on 2024/4/20 05:17, Carl Love wrote:
> rs6000, add overloaded vec_sel with int128 arguments
> 
> Extend the vec_sel built-in to take three signed/unsigned int128 arguments
> and return a signed/unsigned int128 result.
> 
> Extending the vec_sel built-in makes the existing buit-ins
> __builtin_vsx_xxsel_1ti and __builtin_vsx_xxsel_1ti_uns obsolete.  The
> patch removes these built-ins.
> 
> The patch adds documentation and test cases for the new overloaded vec_sel
> built-ins.
> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_xxsel_1ti,
> 	__builtin_vsx_xxsel_1ti_uns): Remove built-in definitions.
> 	* config/rs6000/rs6000-overload.def (vec_sel): Add new overloaded
> 	definitions.
> 	* doc/extend.texi: Add documentation for new vec_sel arguments.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.target/powerpc/vec_sel_runnable-int128.c: New test file.
> ---
>  gcc/config/rs6000/rs6000-builtins.def         |  6 --
>  gcc/config/rs6000/rs6000-overload.def         |  4 +
>  gcc/doc/extend.texi                           | 14 ++++
>  .../powerpc/vec-sel-runnable-i128.c           | 84 +++++++++++++++++++
>  4 files changed, 102 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index d09e21a9151..46d2ae7b7cb 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1931,12 +1931,6 @@
>    const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
>      XXSEL_16QI_UNS vector_select_v16qi_uns {}
>  
> -  const vsq __builtin_vsx_xxsel_1ti (vsq, vsq, vsq);
> -    XXSEL_1TI vector_select_v1ti {}
> -
> -  const vsq __builtin_vsx_xxsel_1ti_uns (vsq, vsq, vsq);
> -    XXSEL_1TI_UNS vector_select_v1ti_uns {}
> -
>    const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
>      XXSEL_2DF vector_select_v2df {}
>  
> diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
> index 68501c05289..5912c9452f4 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -3274,6 +3274,10 @@
>      VSEL_2DF  VSEL_2DF_B
>    vd __builtin_vec_sel (vd, vd, vull);
>      VSEL_2DF  VSEL_2DF_U
> +  vsq __builtin_vec_sel (vsq, vsq, vsq);
> +    VSEL_1TI  VSEL_1TI_S
> +  vuq __builtin_vec_sel (vuq, vuq, vuq);
> +    VSEL_1TI_UNS  VSEL_1TI_U
>  ; The following variants are deprecated.
>    vsll __builtin_vec_sel (vsll, vsll, vsll);
>      VSEL_2DI_B  VSEL_2DI_S
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 64a43b55e2d..86b8e536dbe 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -23358,6 +23358,20 @@ The programmer is responsible for understanding the endianness issues involved
>  with the first argument and the result.
>  @findex vec_replace_unaligned
>  
> +Vector select
> +
> +@smallexample
> +vector signed __int128 vec_sel (vector signed __int128,
> +               vector signed __int128, vector signed __int128);
> +vector unsigned __int128 vec_sel (vector unsigned __int128,
> +               vector unsigned __int128, vector unsigned __int128);
> +@end smallexample
> +
> +The overloaded built-in @code{vec_sel} with vector signed/unsigned __int128
> +arguments and returns a vector selecting bits from the two source vectors based
> +on the values of the third input vector.  This built-in is an extension of the
> +@code{vec_sel} built-in documented in the PVIPR.
> +

Why did you place this in a section for ISA 3.1 (Power10)?  It doesn't really
require this support.  The used instance VSEL_1TI and VSEL_1TI_UNS are placed
in altivec stanza, so it looks that we should put it under the section
"PowerPC AltiVec Built-in Functions on ISA 2.05".  And since it's an extension
of @code{vec_sel} documented in the PVIPR, I prefer to just mention it's "an
extension of the @code{vec_sel} built-in documented in the PVIPR" and omitting
the description to avoid possible slightly different wording.

>  Vector Shift Left Double Bit Immediate
>  @smallexample
>  @exdent vector signed char vec_sldb (vector signed char, vector signed char,
> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
> new file mode 100644
> index 00000000000..58eb383e8c3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
> @@ -0,0 +1,84 @@
> +/* { dg-do run  { target power10_hw }} */
> +/* { dg-require-effective-target int128 } */
> +/* { dg-require-effective-target power10_hw } */

As mentioned above, this doesn't require power10, you can specify vmx_hw.
(btw removing { target power10_hw } on dg-do run line).

> +/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */

s/-mdejagnu-cpu=power10/-maltivec/
s/-save-temps//

> +
> +
> +#include <altivec.h>
> +
> +
> +#define DEBUG 0
> +
> +#if DEBUG
> +#include <stdio.h>
> +void print_i128 (unsigned __int128 val)
> +{
> +  printf(" 0x%016llx%016llx",
> +         (unsigned long long)(val >> 64),
> +         (unsigned long long)(val & 0xFFFFFFFFFFFFFFFF));
> +}
> +#endif

Did you really test this debugging work as expected?
With my experience when making r14-10011-g6e62ede7aaccc6,
this debugging doesn't work and the way to initialize
a vector int128 variable can easily suffer from endianness
issue, so please double check this and test it on BE as well.

BR,
Kewen

> +
> +extern void abort (void);
> +
> +int
> +main (int argc, char *argv [])
> +{
> +  vector signed __int128 src_va_s128;
> +  vector signed __int128 src_vb_s128;
> +  vector signed __int128 src_vc_s128;
> +  vector signed __int128 vresult_s128;
> +  vector signed __int128 expected_vresult_s128;
> +
> +  vector unsigned __int128 src_va_u128;
> +  vector unsigned __int128 src_vb_u128;
> +  vector unsigned __int128 src_vc_u128;
> +  vector unsigned __int128 vresult_u128;
> +  vector unsigned __int128 expected_vresult_u128;
> +
> +  src_va_s128 = (vector signed __int128) {0x123456789ABCDEF0};
> +  src_vb_s128 = (vector signed __int128) {0xFEDCBA9876543210};
> +  src_vc_s128 = (vector signed __int128) {0x3333333333333333};
> +  expected_vresult_s128 = (vector signed __int128) {0x32147658ba9cfed0};
> +
> +  /* Signed arguments.  */
> +  vresult_s128 = vec_sel (src_va_s128, src_vb_s128, src_vc_s128);
> +
> +  if (!vec_all_eq (vresult_s128, expected_vresult_s128))
> +#if DEBUG
> +    {
> +      printf ("ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.\n");
> +      printf ("  Result:          ");
> +      print_i128 ((unsigned __int128) vresult_s128);
> +      printf ("\n  Expected result: ");
> +      print_i128 ((unsigned __int128) expected_vresult_s128);
> +      printf ("\n");
> +    }
> +#else
> +    abort ();
> +#endif
> +
> +  src_va_u128 = (vector unsigned __int128) {0x13579ACE02468BDF};
> +  src_vb_u128 = (vector unsigned __int128) {0xA987654FEDCB3210};
> +  src_vc_u128 = (vector unsigned __int128) {0x5555555555555555};
> +  expected_vresult_u128 = (vector unsigned __int128) {0x32147658ba9cfed0};
> +
> +  /* Unigned arguments.  */
> +  vresult_u128 = vec_sel (src_va_u128, src_vb_u128, src_vc_u128);
> +
> +  if (!vec_all_eq (vresult_u128, expected_vresult_u128))
> +#if DEBUG
> +    {
> +      printf ("ERROR, vec_sel (src_va_u128, src_vb_u128, src_vc_u128) result does not match expected output.\n");
> +      printf ("  Result:          ");
> +      print_i128 ((unsigned __int128) vresult_u128);
> +      printf ("\n  Expected result: ");
> +      print_i128 ((unsigned __int128) expected_vresult_u128);
> +      printf ("\n");
> +    }
> +#else
> +    abort ();
> +#endif
> +
> +    return 0;
> +}

Re: [PATCH 6/13] rs6000, add overloaded vec_sel with int128 arguments

[Carl Love]

Kewen:

On 5/13/24 19:54, Kewen.Lin wrote:
> Hi,
> 
> on 2024/4/20 05:17, Carl Love wrote:
>> rs6000, add overloaded vec_sel with int128 arguments
>>
>> Extend the vec_sel built-in to take three signed/unsigned int128 arguments
>> and return a signed/unsigned int128 result.
>>
>> Extending the vec_sel built-in makes the existing buit-ins
>> __builtin_vsx_xxsel_1ti and __builtin_vsx_xxsel_1ti_uns obsolete.  The
>> patch removes these built-ins.
>>
>> The patch adds documentation and test cases for the new overloaded vec_sel
>> built-ins.
>>
>> gcc/ChangeLog:
>> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_xxsel_1ti,
>> 	__builtin_vsx_xxsel_1ti_uns): Remove built-in definitions.
>> 	* config/rs6000/rs6000-overload.def (vec_sel): Add new overloaded
>> 	definitions.
>> 	* doc/extend.texi: Add documentation for new vec_sel arguments.
>>
>> gcc/testsuite/ChangeLog:
>> 	* gcc.target/powerpc/vec_sel_runnable-int128.c: New test file.
>> ---
>>  gcc/config/rs6000/rs6000-builtins.def         |  6 --
>>  gcc/config/rs6000/rs6000-overload.def         |  4 +
>>  gcc/doc/extend.texi                           | 14 ++++
>>  .../powerpc/vec-sel-runnable-i128.c           | 84 +++++++++++++++++++
>>  4 files changed, 102 insertions(+), 6 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
>>
>> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
>> index d09e21a9151..46d2ae7b7cb 100644
>> --- a/gcc/config/rs6000/rs6000-builtins.def
>> +++ b/gcc/config/rs6000/rs6000-builtins.def
>> @@ -1931,12 +1931,6 @@
>>    const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
>>      XXSEL_16QI_UNS vector_select_v16qi_uns {}
>>  
>> -  const vsq __builtin_vsx_xxsel_1ti (vsq, vsq, vsq);
>> -    XXSEL_1TI vector_select_v1ti {}
>> -
>> -  const vsq __builtin_vsx_xxsel_1ti_uns (vsq, vsq, vsq);
>> -    XXSEL_1TI_UNS vector_select_v1ti_uns {}
>> -
>>    const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
>>      XXSEL_2DF vector_select_v2df {}
>>  
>> diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
>> index 68501c05289..5912c9452f4 100644
>> --- a/gcc/config/rs6000/rs6000-overload.def
>> +++ b/gcc/config/rs6000/rs6000-overload.def
>> @@ -3274,6 +3274,10 @@
>>      VSEL_2DF  VSEL_2DF_B
>>    vd __builtin_vec_sel (vd, vd, vull);
>>      VSEL_2DF  VSEL_2DF_U
>> +  vsq __builtin_vec_sel (vsq, vsq, vsq);
>> +    VSEL_1TI  VSEL_1TI_S
>> +  vuq __builtin_vec_sel (vuq, vuq, vuq);
>> +    VSEL_1TI_UNS  VSEL_1TI_U
>>  ; The following variants are deprecated.
>>    vsll __builtin_vec_sel (vsll, vsll, vsll);
>>      VSEL_2DI_B  VSEL_2DI_S
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 64a43b55e2d..86b8e536dbe 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -23358,6 +23358,20 @@ The programmer is responsible for understanding the endianness issues involved
>>  with the first argument and the result.
>>  @findex vec_replace_unaligned
>>  
>> +Vector select
>> +
>> +@smallexample
>> +vector signed __int128 vec_sel (vector signed __int128,
>> +               vector signed __int128, vector signed __int128);
>> +vector unsigned __int128 vec_sel (vector unsigned __int128,
>> +               vector unsigned __int128, vector unsigned __int128);
>> +@end smallexample
>> +
>> +The overloaded built-in @code{vec_sel} with vector signed/unsigned __int128
>> +arguments and returns a vector selecting bits from the two source vectors based
>> +on the values of the third input vector.  This built-in is an extension of the
>> +@code{vec_sel} built-in documented in the PVIPR.
>> +
> 
> Why did you place this in a section for ISA 3.1 (Power10)?  It doesn't really
> require this support.  The used instance VSEL_1TI and VSEL_1TI_UNS are placed
> in altivec stanza, so it looks that we should put it under the section
> "PowerPC AltiVec Built-in Functions on ISA 2.05".  And since it's an extension
> of @code{vec_sel} documented in the PVIPR, I prefer to just mention it's "an
> extension of the @code{vec_sel} built-in documented in the PVIPR" and omitting
> the description to avoid possible slightly different wording.

Honestly, at this point in time I don't remember why I put it there.  It has been too long since I created the patch.  That said, the test case requires Power 10 do to the comparison check using built-in vec_all_eq but that is another issue.  The built-in generates the xxsel instruction that is an ISA 2.06 instruction.  So, I would say it should to into the ISA 2.06 section.  I moved it to the ISA 2.06 section.

For consistency with the previous patches/feedback, the descriptions are being dropped and replaced with the instance being a new extension of the built-in that is documented in the PVIPR.
> 
>>  Vector Shift Left Double Bit Immediate
>>  @smallexample
>>  @exdent vector signed char vec_sldb (vector signed char, vector signed char,
>> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
>> new file mode 100644
>> index 00000000000..58eb383e8c3
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
>> @@ -0,0 +1,84 @@
>> +/* { dg-do run  { target power10_hw }} */
>> +/* { dg-require-effective-target int128 } */
>> +/* { dg-require-effective-target power10_hw } */
> 
> As mentioned above, this doesn't require power10, you can specify vmx_hw.
> (btw removing { target power10_hw } on dg-do run line).
> 

As mentioned, the testcase uses the vec_all_eq which requires Power 10.  So, I rewrote the test case
to check the result value and expected result value byte by byte so the test will run on Power 7 (ISA 2.06).  The new version of the test case compiles and runs with the command:

gcc -g -mcpu=power7  vec-sel-runnable-i128.c -o vec-sel-runnable-i128


>> +/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
> 
> s/-mdejagnu-cpu=power10/-maltivec/
> s/-save-temps//
> 

Removed the Power 10, using vmx_hw instead.

>> +
>> +
>> +#include <altivec.h>
>> +
>> +
>> +#define DEBUG 0
>> +
>> +#if DEBUG
>> +#include <stdio.h>
>> +void print_i128 (unsigned __int128 val)
>> +{
>> +  printf(" 0x%016llx%016llx",
>> +         (unsigned long long)(val >> 64),
>> +         (unsigned long long)(val & 0xFFFFFFFFFFFFFFFF));
>> +}
>> +#endif
> 
> Did you really test this debugging work as expected?
> With my experience when making r14-10011-g6e62ede7aaccc6,
> this debugging doesn't work and the way to initialize
> a vector int128 variable can easily suffer from endianness
> issue, so please double check this and test it on BE as well.

Yes, It seemed to work with this version of gcc.  I retested the patch per your request.  I set DEBUG to 1, changed the expected result and ran it on LE:

./vec-sel-runnable-i128
ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
  Result:           0x000000000000000032147658ba9cfed0
  Expected result:  0x000000000000000032147658ba9cfed1

I compiled the patch series on BE and ran the test there:

 ./vec-sel-runnable-i128
ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
  Result:           0x000000000000000032147658ba9cfed0
  Expected result:  0x000000000000000032147658ba9cfed1

I am not sure exactly what issue you are concerned about with the print statement.  But, we could implement the print statement as follows if you prefer:

void print_i128 (unsigned __int128 val)
{
  int i;
  union convert_u {
    unsigned __int128 val;
    char bytes[16];
  } convert;

  convert.val = val;
#if __LITTLE_ENDIAN__
  for (i = 15; i >= 0; i--)
#else
  for (i = 0; i < 16; i++)
#endif
  printf(" 0x");
  
    printf("%02x", convert.bytes[i]);
}

which gives the same result (on LE: 

./vec-sel-runnable-i128
ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
  Result:           0x000000000000000032147658ba9cfed0
  Expected result:  0x000000000000000032147658ba9cfed1

and on BE:

./vec-sel-runnable-i128
ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
  Result:           0x000000000000000032147658ba9cfed0
  Expected result:  0x000000000000000032147658ba9cfed1

Sounds like there was some issue that you noticed on r14-10011-g6e62ede7aaccc6.  The new version of
print_i128 should be functionally equivalent but perhaps is "safer"?

Let me know if you would prefer I use the new version of the print_i128 function or if the orginal is OK?  

Thanks.

                            Carl 

> 
> BR,
> Kewen
> 
>> +
>> +extern void abort (void);
>> +
>> +int
>> +main (int argc, char *argv [])
>> +{
>> +  vector signed __int128 src_va_s128;
>> +  vector signed __int128 src_vb_s128;
>> +  vector signed __int128 src_vc_s128;
>> +  vector signed __int128 vresult_s128;
>> +  vector signed __int128 expected_vresult_s128;
>> +
>> +  vector unsigned __int128 src_va_u128;
>> +  vector unsigned __int128 src_vb_u128;
>> +  vector unsigned __int128 src_vc_u128;
>> +  vector unsigned __int128 vresult_u128;
>> +  vector unsigned __int128 expected_vresult_u128;
>> +
>> +  src_va_s128 = (vector signed __int128) {0x123456789ABCDEF0};
>> +  src_vb_s128 = (vector signed __int128) {0xFEDCBA9876543210};
>> +  src_vc_s128 = (vector signed __int128) {0x3333333333333333};
>> +  expected_vresult_s128 = (vector signed __int128) {0x32147658ba9cfed0};
>> +
>> +  /* Signed arguments.  */
>> +  vresult_s128 = vec_sel (src_va_s128, src_vb_s128, src_vc_s128);
>> +
>> +  if (!vec_all_eq (vresult_s128, expected_vresult_s128))
>> +#if DEBUG
>> +    {
>> +      printf ("ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.\n");
>> +      printf ("  Result:          ");
>> +      print_i128 ((unsigned __int128) vresult_s128);
>> +      printf ("\n  Expected result: ");
>> +      print_i128 ((unsigned __int128) expected_vresult_s128);
>> +      printf ("\n");
>> +    }
>> +#else
>> +    abort ();
>> +#endif
>> +
>> +  src_va_u128 = (vector unsigned __int128) {0x13579ACE02468BDF};
>> +  src_vb_u128 = (vector unsigned __int128) {0xA987654FEDCB3210};
>> +  src_vc_u128 = (vector unsigned __int128) {0x5555555555555555};
>> +  expected_vresult_u128 = (vector unsigned __int128) {0x32147658ba9cfed0};
>> +
>> +  /* Unigned arguments.  */
>> +  vresult_u128 = vec_sel (src_va_u128, src_vb_u128, src_vc_u128);
>> +
>> +  if (!vec_all_eq (vresult_u128, expected_vresult_u128))
>> +#if DEBUG
>> +    {
>> +      printf ("ERROR, vec_sel (src_va_u128, src_vb_u128, src_vc_u128) result does not match expected output.\n");
>> +      printf ("  Result:          ");
>> +      print_i128 ((unsigned __int128) vresult_u128);
>> +      printf ("\n  Expected result: ");
>> +      print_i128 ((unsigned __int128) expected_vresult_u128);
>> +      printf ("\n");
>> +    }
>> +#else
>> +    abort ();
>> +#endif
>> +
>> +    return 0;
>> +}
> 

Re: [PATCH 6/13] rs6000, add overloaded vec_sel with int128 arguments

[Kewen.Lin]

Hi Carl,

on 2024/5/22 08:13, Carl Love wrote:
> Kewen:
> 
> On 5/13/24 19:54, Kewen.Lin wrote:
>> Hi,
>>
>> on 2024/4/20 05:17, Carl Love wrote:
>>> rs6000, add overloaded vec_sel with int128 arguments
>>>
>>> Extend the vec_sel built-in to take three signed/unsigned int128 arguments
>>> and return a signed/unsigned int128 result.
>>>
>>> Extending the vec_sel built-in makes the existing buit-ins
>>> __builtin_vsx_xxsel_1ti and __builtin_vsx_xxsel_1ti_uns obsolete.  The
>>> patch removes these built-ins.
>>>
>>> The patch adds documentation and test cases for the new overloaded vec_sel
>>> built-ins.
>>>
>>> gcc/ChangeLog:
>>> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_xxsel_1ti,
>>> 	__builtin_vsx_xxsel_1ti_uns): Remove built-in definitions.
>>> 	* config/rs6000/rs6000-overload.def (vec_sel): Add new overloaded
>>> 	definitions.
>>> 	* doc/extend.texi: Add documentation for new vec_sel arguments.
>>>
>>> gcc/testsuite/ChangeLog:
>>> 	* gcc.target/powerpc/vec_sel_runnable-int128.c: New test file.
>>> ---
>>>  gcc/config/rs6000/rs6000-builtins.def         |  6 --
>>>  gcc/config/rs6000/rs6000-overload.def         |  4 +
>>>  gcc/doc/extend.texi                           | 14 ++++
>>>  .../powerpc/vec-sel-runnable-i128.c           | 84 +++++++++++++++++++
>>>  4 files changed, 102 insertions(+), 6 deletions(-)
>>>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
>>>
>>> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
>>> index d09e21a9151..46d2ae7b7cb 100644
>>> --- a/gcc/config/rs6000/rs6000-builtins.def
>>> +++ b/gcc/config/rs6000/rs6000-builtins.def
>>> @@ -1931,12 +1931,6 @@
>>>    const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
>>>      XXSEL_16QI_UNS vector_select_v16qi_uns {}
>>>  
>>> -  const vsq __builtin_vsx_xxsel_1ti (vsq, vsq, vsq);
>>> -    XXSEL_1TI vector_select_v1ti {}
>>> -
>>> -  const vsq __builtin_vsx_xxsel_1ti_uns (vsq, vsq, vsq);
>>> -    XXSEL_1TI_UNS vector_select_v1ti_uns {}
>>> -
>>>    const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
>>>      XXSEL_2DF vector_select_v2df {}
>>>  
>>> diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
>>> index 68501c05289..5912c9452f4 100644
>>> --- a/gcc/config/rs6000/rs6000-overload.def
>>> +++ b/gcc/config/rs6000/rs6000-overload.def
>>> @@ -3274,6 +3274,10 @@
>>>      VSEL_2DF  VSEL_2DF_B
>>>    vd __builtin_vec_sel (vd, vd, vull);
>>>      VSEL_2DF  VSEL_2DF_U
>>> +  vsq __builtin_vec_sel (vsq, vsq, vsq);
>>> +    VSEL_1TI  VSEL_1TI_S
>>> +  vuq __builtin_vec_sel (vuq, vuq, vuq);
>>> +    VSEL_1TI_UNS  VSEL_1TI_U
>>>  ; The following variants are deprecated.
>>>    vsll __builtin_vec_sel (vsll, vsll, vsll);
>>>      VSEL_2DI_B  VSEL_2DI_S
>>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>>> index 64a43b55e2d..86b8e536dbe 100644
>>> --- a/gcc/doc/extend.texi
>>> +++ b/gcc/doc/extend.texi
>>> @@ -23358,6 +23358,20 @@ The programmer is responsible for understanding the endianness issues involved
>>>  with the first argument and the result.
>>>  @findex vec_replace_unaligned
>>>  
>>> +Vector select
>>> +
>>> +@smallexample
>>> +vector signed __int128 vec_sel (vector signed __int128,
>>> +               vector signed __int128, vector signed __int128);
>>> +vector unsigned __int128 vec_sel (vector unsigned __int128,
>>> +               vector unsigned __int128, vector unsigned __int128);
>>> +@end smallexample
>>> +
>>> +The overloaded built-in @code{vec_sel} with vector signed/unsigned __int128
>>> +arguments and returns a vector selecting bits from the two source vectors based
>>> +on the values of the third input vector.  This built-in is an extension of the
>>> +@code{vec_sel} built-in documented in the PVIPR.
>>> +
>>
>> Why did you place this in a section for ISA 3.1 (Power10)?  It doesn't really
>> require this support.  The used instance VSEL_1TI and VSEL_1TI_UNS are placed
>> in altivec stanza, so it looks that we should put it under the section
>> "PowerPC AltiVec Built-in Functions on ISA 2.05".  And since it's an extension
>> of @code{vec_sel} documented in the PVIPR, I prefer to just mention it's "an
>> extension of the @code{vec_sel} built-in documented in the PVIPR" and omitting
>> the description to avoid possible slightly different wording.
> 
> Honestly, at this point in time I don't remember why I put it there.  It has been too long since I created the patch.  That said, the test case requires Power 10 do to the comparison check using built-in vec_all_eq but that is another issue.  
> The built-in generates the xxsel instruction that is an ISA 2.06 instruction.  So, I would say it should to into the ISA 2.06 section.  I moved it to the ISA 2.06 section.

But the underlying implementation is:

  const vsq __builtin_altivec_vsel_1ti (vsq, vsq, vuq);
    VSEL_1TI vector_select_v1ti {}

  const vuq __builtin_altivec_vsel_1ti_uns (vuq, vuq, vuq);
    VSEL_1TI_UNS vector_select_v1ti_uns {}

, it's under altivec stanza and can result with insn vsel (so not xxsel),
vsel is ISA 2.03, so I think ISA 2.05 better matches the implementation.

> For consistency with the previous patches/feedback, the descriptions are being dropped and replaced with the instance being a new extension of the built-in that is documented in the PVIPR.
>>
>>>  Vector Shift Left Double Bit Immediate
>>>  @smallexample
>>>  @exdent vector signed char vec_sldb (vector signed char, vector signed char,
>>> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
>>> new file mode 100644
>>> index 00000000000..58eb383e8c3
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
>>> @@ -0,0 +1,84 @@
>>> +/* { dg-do run  { target power10_hw }} */
>>> +/* { dg-require-effective-target int128 } */
>>> +/* { dg-require-effective-target power10_hw } */
>>
>> As mentioned above, this doesn't require power10, you can specify vmx_hw.
>> (btw removing { target power10_hw } on dg-do run line).
>>
> 
> As mentioned, the testcase uses the vec_all_eq which requires Power 10.  So, I rewrote the test case
> to check the result value and expected result value byte by byte so the test will run on Power 7 (ISA 2.06).  

Nice, thanks!

> The new version of the test case compiles and runs with the command:
> 
> gcc -g -mcpu=power7  vec-sel-runnable-i128.c -o vec-sel-runnable-i128
> 
> 
>>> +/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
>>
>> s/-mdejagnu-cpu=power10/-maltivec/
>> s/-save-temps//
>>
> 
> Removed the Power 10, using vmx_hw instead.
> 
>>> +
>>> +
>>> +#include <altivec.h>
>>> +
>>> +
>>> +#define DEBUG 0
>>> +
>>> +#if DEBUG
>>> +#include <stdio.h>
>>> +void print_i128 (unsigned __int128 val)
>>> +{
>>> +  printf(" 0x%016llx%016llx",
>>> +         (unsigned long long)(val >> 64),
>>> +         (unsigned long long)(val & 0xFFFFFFFFFFFFFFFF));
>>> +}
>>> +#endif
>>
>> Did you really test this debugging work as expected?
>> With my experience when making r14-10011-g6e62ede7aaccc6,
>> this debugging doesn't work and the way to initialize
>> a vector int128 variable can easily suffer from endianness
>> issue, so please double check this and test it on BE as well.
> 
> Yes, It seemed to work with this version of gcc.  I retested the patch per your request.  I set DEBUG to 1, changed the expected result and ran it on LE:
> 
> ./vec-sel-runnable-i128
> ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
>   Result:           0x000000000000000032147658ba9cfed0
>   Expected result:  0x000000000000000032147658ba9cfed1
> 
> I compiled the patch series on BE and ran the test there:
> 
>  ./vec-sel-runnable-i128
> ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
>   Result:           0x000000000000000032147658ba9cfed0
>   Expected result:  0x000000000000000032147658ba9cfed1
> 
> I am not sure exactly what issue you are concerned about with the print statement.  But, we could implement the print statement as follows if you prefer:
> 
> void print_i128 (unsigned __int128 val)
> {
>   int i;
>   union convert_u {
>     unsigned __int128 val;
>     char bytes[16];
>   } convert;
> 
>   convert.val = val;
> #if __LITTLE_ENDIAN__
>   for (i = 15; i >= 0; i--)
> #else
>   for (i = 0; i < 16; i++)
> #endif
>   printf(" 0x");
>   
>     printf("%02x", convert.bytes[i]);
> }
> 
> which gives the same result (on LE: 
> 
> ./vec-sel-runnable-i128
> ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
>   Result:           0x000000000000000032147658ba9cfed0
>   Expected result:  0x000000000000000032147658ba9cfed1
> 
> and on BE:
> 
> ./vec-sel-runnable-i128
> ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_s128) result does not match expected output.
>   Result:           0x000000000000000032147658ba9cfed0
>   Expected result:  0x000000000000000032147658ba9cfed1
> 
> Sounds like there was some issue that you noticed on r14-10011-g6e62ede7aaccc6.  The new version of
> print_i128 should be functionally equivalent but perhaps is "safer"?

Thanks for checking!  Looking into this more closely, I realized you didn't apply the previously
adopted way for printing (the way used in gcc.target/powerpc/builtins-6-p9-runnable.c), sorry for
the false alarm!  So your supposed print_i128 is fine to me.

BR,
Kewen

Re: [PATCH 6/13] rs6000, add overloaded vec_sel with int128 arguments

[Carl Love]

Kewen:

On 5/21/24 20:05, Kewen.Lin wrote:
> Hi Carl,
> 
> on 2024/5/22 08:13, Carl Love wrote:
>> Kewen:

<snip>

>>> Why did you place this in a section for ISA 3.1 (Power10)?  It doesn't really
>>> require this support.  The used instance VSEL_1TI and VSEL_1TI_UNS are placed
>>> in altivec stanza, so it looks that we should put it under the section
>>> "PowerPC AltiVec Built-in Functions on ISA 2.05".  And since it's an extension
>>> of @code{vec_sel} documented in the PVIPR, I prefer to just mention it's "an
>>> extension of the @code{vec_sel} built-in documented in the PVIPR" and omitting
>>> the description to avoid possible slightly different wording.
>>
>> Honestly, at this point in time I don't remember why I put it there.  It has been too long since I created the patch.  That said, the test case requires Power 10 do to the comparison check using built-in vec_all_eq but that is another issue.  
>> The built-in generates the xxsel instruction that is an ISA 2.06 instruction.  So, I would say it should to into the ISA 2.06 section.  I moved it to the ISA 2.06 section.
> 
> But the underlying implementation is:
> 
>   const vsq __builtin_altivec_vsel_1ti (vsq, vsq, vuq);
>     VSEL_1TI vector_select_v1ti {}
> 
>   const vuq __builtin_altivec_vsel_1ti_uns (vuq, vuq, vuq);
>     VSEL_1TI_UNS vector_select_v1ti_uns {}
> 
> , it's under altivec stanza and can result with insn vsel (so not xxsel),
> vsel is ISA 2.03, so I think ISA 2.05 better matches the implementation.

OK, moved to ISA 2.05

> 

<snip>

>>
>> Sounds like there was some issue that you noticed on r14-10011-g6e62ede7aaccc6.  The new version of
>> print_i128 should be functionally equivalent but perhaps is "safer"?
> 
> Thanks for checking!  Looking into this more closely, I realized you didn't apply the previously
> adopted way for printing (the way used in gcc.target/powerpc/builtins-6-p9-runnable.c), sorry for
> the false alarm!  So your supposed print_i128 is fine to me.

OK, no problem.  Will go with the original print_i128 function.

                    Carl 

[PATCH 7/13] rs6000, remove the vec_xxsel built-ins, they are duplicates

[Carl Love]

rs6000, remove the vec_xxsel built-ins, they are duplicates

The following undocumented built-ins are covered by the existing overloaded
vec_sel built-in definitions.

  const vsc __builtin_vsx_xxsel_16qi (vsc, vsc, vsc);
same as vsc __builtin_vec_sel (vsc, vsc, vuc);  (overloaded vec_sel)

  const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
same as vuc __builtin_vec_sel (vuc, vuc, vuc);  (overloaded vec_sel)

  const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
same as  vd __builtin_vec_sel (vd, vd, vull);   (overloaded vec_sel)

  const vsll __builtin_vsx_xxsel_2di (vsll, vsll, vsll);
same as vsll __builtin_vec_sel (vsll, vsll, vsll);  (overloaded vec_sel)

  const vull __builtin_vsx_xxsel_2di_uns (vull, vull, vull);
same as vull __builtin_vec_sel (vull, vull, vsll);  (overloaded vec_sel)

  const vf __builtin_vsx_xxsel_4sf (vf, vf, vf);
same as vf __builtin_vec_sel (vf, vf, vsi)          (overloaded vec_sel)

  const vsi __builtin_vsx_xxsel_4si (vsi, vsi, vsi);
same as vsi __builtin_vec_sel (vsi, vsi, vbi);      (overloaded vec_sel)

  const vui __builtin_vsx_xxsel_4si_uns (vui, vui, vui);
same as vui __builtin_vec_sel (vui, vui, vui);      (overloaded vec_sel)

  const vss __builtin_vsx_xxsel_8hi (vss, vss, vss);
same as vss __builtin_vec_sel (vss, vss, vbs);      (overloaded vec_sel)

  const vus __builtin_vsx_xxsel_8hi_uns (vus, vus, vus);
same as vus __builtin_vec_sel (vus, vus, vus);      (overloaded vec_sel)

This patch removed the duplicate built-in definitions so users will only
use the documented vec_sel built-in.  The __builtin_vsx_xxsel_[4si, 8hi,
16qi, 4sf, 2df] tests are also removed.

gcc/ChangeLog:
        * config/rs6000/rs6000-builtins.def (__builtin_vsx_xxmrglw_4si,
	__builtin_vsx_xxsel_16qi, __builtin_vsx_xxsel_16qi_uns,
	__builtin_vsx_xxsel_2df, __builtin_vsx_xxsel_2di,
	__builtin_vsx_xxsel_2di_uns, __builtin_vsx_xxsel_4sf,
	__builtin_vsx_xxsel_4si, __builtin_vsx_xxsel_4si_uns,
	__builtin_vsx_xxsel_8hi, __builtin_vsx_xxsel_8hi_uns): Remove
	built-in definitions.

gcc/testsuite/ChangeLog:
        * gcc.target/powerpc/vsx-builtin-3.c (__builtin_vsx_xxsel_4si,
        __builtin_vsx_xxsel_8hi, __builtin_vsx_xxsel_16qi,
        __builtin_vsx_xxsel_4sf, __builtin_vsx_xxsel_2df): Remove test
        cases for removed built-ins.
---
 gcc/config/rs6000/rs6000-builtins.def         | 30 -------------------
 .../gcc.target/powerpc/vsx-builtin-3.c        | 26 ----------------
 2 files changed, 56 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index 46d2ae7b7cb..3c409d729ea 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1925,36 +1925,6 @@
   const vss __builtin_vsx_xxpermdi_8hi (vss, vss, const int<2>);
     XXPERMDI_8HI vsx_xxpermdi_v8hi {}
 
-  const vsc __builtin_vsx_xxsel_16qi (vsc, vsc, vsc);
-    XXSEL_16QI vector_select_v16qi {}
-
-  const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
-    XXSEL_16QI_UNS vector_select_v16qi_uns {}
-
-  const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
-    XXSEL_2DF vector_select_v2df {}
-
-  const vsll __builtin_vsx_xxsel_2di (vsll, vsll, vsll);
-    XXSEL_2DI vector_select_v2di {}
-
-  const vull __builtin_vsx_xxsel_2di_uns (vull, vull, vull);
-    XXSEL_2DI_UNS vector_select_v2di_uns {}
-
-  const vf __builtin_vsx_xxsel_4sf (vf, vf, vf);
-    XXSEL_4SF vector_select_v4sf {}
-
-  const vsi __builtin_vsx_xxsel_4si (vsi, vsi, vsi);
-    XXSEL_4SI vector_select_v4si {}
-
-  const vui __builtin_vsx_xxsel_4si_uns (vui, vui, vui);
-    XXSEL_4SI_UNS vector_select_v4si_uns {}
-
-  const vss __builtin_vsx_xxsel_8hi (vss, vss, vss);
-    XXSEL_8HI vector_select_v8hi {}
-
-  const vus __builtin_vsx_xxsel_8hi_uns (vus, vus, vus);
-    XXSEL_8HI_UNS vector_select_v8hi_uns {}
-
   const vsc __builtin_vsx_xxsldwi_16qi (vsc, vsc, const int<2>);
     XXSLDWI_16QI vsx_xxsldwi_v16qi {}
 
diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
index ff875c55304..01f35dad713 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
@@ -2,7 +2,6 @@
 /* { dg-skip-if "" { powerpc*-*-darwin* } } */
 /* { dg-require-effective-target powerpc_vsx_ok } */
 /* { dg-options "-O2 -mdejagnu-cpu=power7" } */
-/* { dg-final { scan-assembler "xxsel" } } */
 /* { dg-final { scan-assembler "vperm" } } */
 /* { dg-final { scan-assembler "xvrdpi" } } */
 /* { dg-final { scan-assembler "xvrdpic" } } */
@@ -57,31 +56,6 @@ extern __vector unsigned long long ull[][4];
 extern __vector __bool long bl[][4];
 #endif
 
-int do_sel(void)
-{
-  int i = 0;
-
-  si[i][0] = __builtin_vsx_xxsel_4si (si[i][1], si[i][2], si[i][3]); i++;
-  ss[i][0] = __builtin_vsx_xxsel_8hi (ss[i][1], ss[i][2], ss[i][3]); i++;
-  sc[i][0] = __builtin_vsx_xxsel_16qi (sc[i][1], sc[i][2], sc[i][3]); i++;
-  f[i][0] = __builtin_vsx_xxsel_4sf (f[i][1], f[i][2], f[i][3]); i++;
-  d[i][0] = __builtin_vsx_xxsel_2df (d[i][1], d[i][2], d[i][3]); i++;
-
-  si[i][0] = __builtin_vsx_xxsel (si[i][1], si[i][2], bi[i][3]); i++;
-  ss[i][0] = __builtin_vsx_xxsel (ss[i][1], ss[i][2], bs[i][3]); i++;
-  sc[i][0] = __builtin_vsx_xxsel (sc[i][1], sc[i][2], bc[i][3]); i++;
-  f[i][0] = __builtin_vsx_xxsel (f[i][1], f[i][2], bi[i][3]); i++;
-  d[i][0] = __builtin_vsx_xxsel (d[i][1], d[i][2], bl[i][3]); i++;
-
-  si[i][0] = __builtin_vsx_xxsel (si[i][1], si[i][2], ui[i][3]); i++;
-  ss[i][0] = __builtin_vsx_xxsel (ss[i][1], ss[i][2], us[i][3]); i++;
-  sc[i][0] = __builtin_vsx_xxsel (sc[i][1], sc[i][2], uc[i][3]); i++;
-  f[i][0] = __builtin_vsx_xxsel (f[i][1], f[i][2], ui[i][3]); i++;
-  d[i][0] = __builtin_vsx_xxsel (d[i][1], d[i][2], ul[i][3]); i++;
-
-  return i;
-}
-
 int do_perm(void)
 {
   int i = 0;
-- 
2.44.0

Re: [PATCH 7/13] rs6000, remove the vec_xxsel built-ins, they are duplicates

[Kewen.Lin]

Hi,

on 2024/4/20 05:18, Carl Love wrote:
> rs6000, remove the vec_xxsel built-ins, they are duplicates
> 
> The following undocumented built-ins are covered by the existing overloaded
> vec_sel built-in definitions.
> 
>   const vsc __builtin_vsx_xxsel_16qi (vsc, vsc, vsc);
> same as vsc __builtin_vec_sel (vsc, vsc, vuc);  (overloaded vec_sel)
> 
>   const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
> same as vuc __builtin_vec_sel (vuc, vuc, vuc);  (overloaded vec_sel)
> 
>   const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
> same as  vd __builtin_vec_sel (vd, vd, vull);   (overloaded vec_sel)
> 
>   const vsll __builtin_vsx_xxsel_2di (vsll, vsll, vsll);
> same as vsll __builtin_vec_sel (vsll, vsll, vsll);  (overloaded vec_sel)
> 
>   const vull __builtin_vsx_xxsel_2di_uns (vull, vull, vull);
> same as vull __builtin_vec_sel (vull, vull, vsll);  (overloaded vec_sel)
> 
>   const vf __builtin_vsx_xxsel_4sf (vf, vf, vf);
> same as vf __builtin_vec_sel (vf, vf, vsi)          (overloaded vec_sel)
> 
>   const vsi __builtin_vsx_xxsel_4si (vsi, vsi, vsi);
> same as vsi __builtin_vec_sel (vsi, vsi, vbi);      (overloaded vec_sel)
> 
>   const vui __builtin_vsx_xxsel_4si_uns (vui, vui, vui);
> same as vui __builtin_vec_sel (vui, vui, vui);      (overloaded vec_sel)
> 
>   const vss __builtin_vsx_xxsel_8hi (vss, vss, vss);
> same as vss __builtin_vec_sel (vss, vss, vbs);      (overloaded vec_sel)
> 
>   const vus __builtin_vsx_xxsel_8hi_uns (vus, vus, vus);
> same as vus __builtin_vec_sel (vus, vus, vus);      (overloaded vec_sel)
> 
> This patch removed the duplicate built-in definitions so users will only
> use the documented vec_sel built-in.  The __builtin_vsx_xxsel_[4si, 8hi,
> 16qi, 4sf, 2df] tests are also removed.
> 
> gcc/ChangeLog:
>         * config/rs6000/rs6000-builtins.def (__builtin_vsx_xxmrglw_4si,

Typo: __builtin_vsx_xxmrglw_4si, which doesn't belong to this patch.

> 	__builtin_vsx_xxsel_16qi, __builtin_vsx_xxsel_16qi_uns,
> 	__builtin_vsx_xxsel_2df, __builtin_vsx_xxsel_2di,
> 	__builtin_vsx_xxsel_2di_uns, __builtin_vsx_xxsel_4sf,
> 	__builtin_vsx_xxsel_4si, __builtin_vsx_xxsel_4si_uns,
> 	__builtin_vsx_xxsel_8hi, __builtin_vsx_xxsel_8hi_uns): Remove
> 	built-in definitions.
> 
> gcc/testsuite/ChangeLog:
>         * gcc.target/powerpc/vsx-builtin-3.c (__builtin_vsx_xxsel_4si,
>         __builtin_vsx_xxsel_8hi, __builtin_vsx_xxsel_16qi,
>         __builtin_vsx_xxsel_4sf, __builtin_vsx_xxsel_2df): Remove test
>         cases for removed built-ins.
> ---
>  gcc/config/rs6000/rs6000-builtins.def         | 30 -------------------
>  .../gcc.target/powerpc/vsx-builtin-3.c        | 26 ----------------
>  2 files changed, 56 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 46d2ae7b7cb..3c409d729ea 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1925,36 +1925,6 @@
>    const vss __builtin_vsx_xxpermdi_8hi (vss, vss, const int<2>);
>      XXPERMDI_8HI vsx_xxpermdi_v8hi {}
>  
> -  const vsc __builtin_vsx_xxsel_16qi (vsc, vsc, vsc);
> -    XXSEL_16QI vector_select_v16qi {}
> -
> -  const vuc __builtin_vsx_xxsel_16qi_uns (vuc, vuc, vuc);
> -    XXSEL_16QI_UNS vector_select_v16qi_uns {}
> -
> -  const vd __builtin_vsx_xxsel_2df (vd, vd, vd);
> -    XXSEL_2DF vector_select_v2df {}
> -
> -  const vsll __builtin_vsx_xxsel_2di (vsll, vsll, vsll);
> -    XXSEL_2DI vector_select_v2di {}
> -
> -  const vull __builtin_vsx_xxsel_2di_uns (vull, vull, vull);
> -    XXSEL_2DI_UNS vector_select_v2di_uns {}
> -
> -  const vf __builtin_vsx_xxsel_4sf (vf, vf, vf);
> -    XXSEL_4SF vector_select_v4sf {}
> -
> -  const vsi __builtin_vsx_xxsel_4si (vsi, vsi, vsi);
> -    XXSEL_4SI vector_select_v4si {}
> -
> -  const vui __builtin_vsx_xxsel_4si_uns (vui, vui, vui);
> -    XXSEL_4SI_UNS vector_select_v4si_uns {}
> -
> -  const vss __builtin_vsx_xxsel_8hi (vss, vss, vss);
> -    XXSEL_8HI vector_select_v8hi {}
> -
> -  const vus __builtin_vsx_xxsel_8hi_uns (vus, vus, vus);
> -    XXSEL_8HI_UNS vector_select_v8hi_uns {}
> -
>    const vsc __builtin_vsx_xxsldwi_16qi (vsc, vsc, const int<2>);
>      XXSLDWI_16QI vsx_xxsldwi_v16qi {}
>  
> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> index ff875c55304..01f35dad713 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> @@ -2,7 +2,6 @@
>  /* { dg-skip-if "" { powerpc*-*-darwin* } } */
>  /* { dg-require-effective-target powerpc_vsx_ok } */
>  /* { dg-options "-O2 -mdejagnu-cpu=power7" } */
> -/* { dg-final { scan-assembler "xxsel" } } */
>  /* { dg-final { scan-assembler "vperm" } } */
>  /* { dg-final { scan-assembler "xvrdpi" } } */
>  /* { dg-final { scan-assembler "xvrdpic" } } */
> @@ -57,31 +56,6 @@ extern __vector unsigned long long ull[][4];
>  extern __vector __bool long bl[][4];
>  #endif
>  
> -int do_sel(void)
> -{
> -  int i = 0;
> -
> -  si[i][0] = __builtin_vsx_xxsel_4si (si[i][1], si[i][2], si[i][3]); i++;
> -  ss[i][0] = __builtin_vsx_xxsel_8hi (ss[i][1], ss[i][2], ss[i][3]); i++;
> -  sc[i][0] = __builtin_vsx_xxsel_16qi (sc[i][1], sc[i][2], sc[i][3]); i++;
> -  f[i][0] = __builtin_vsx_xxsel_4sf (f[i][1], f[i][2], f[i][3]); i++;
> -  d[i][0] = __builtin_vsx_xxsel_2df (d[i][1], d[i][2], d[i][3]); i++;
> -
> -  si[i][0] = __builtin_vsx_xxsel (si[i][1], si[i][2], bi[i][3]); i++;
> -  ss[i][0] = __builtin_vsx_xxsel (ss[i][1], ss[i][2], bs[i][3]); i++;
> -  sc[i][0] = __builtin_vsx_xxsel (sc[i][1], sc[i][2], bc[i][3]); i++;
> -  f[i][0] = __builtin_vsx_xxsel (f[i][1], f[i][2], bi[i][3]); i++;
> -  d[i][0] = __builtin_vsx_xxsel (d[i][1], d[i][2], bl[i][3]); i++;
> -
> -  si[i][0] = __builtin_vsx_xxsel (si[i][1], si[i][2], ui[i][3]); i++;
> -  ss[i][0] = __builtin_vsx_xxsel (ss[i][1], ss[i][2], us[i][3]); i++;
> -  sc[i][0] = __builtin_vsx_xxsel (sc[i][1], sc[i][2], uc[i][3]); i++;
> -  f[i][0] = __builtin_vsx_xxsel (f[i][1], f[i][2], ui[i][3]); i++;
> -  d[i][0] = __builtin_vsx_xxsel (d[i][1], d[i][2], ul[i][3]); i++;
> -
> -  return i;
> -}
> -

I prefer to keep them but just replacing the call with vec_sel.

OK with the above nits tweaked, thanks.

BR,
Kewen

>  int do_perm(void)
>  {
>    int i = 0;

Re: [PATCH 7/13] rs6000, remove the vec_xxsel built-ins, they are duplicates

[Carl Love]

Kewen:

On 5/13/24 19:55, Kewen.Lin wrote:
> Hi,
> 
> on 2024/4/20 05:18, Carl Love wrote:
>> rs6000, remove the vec_xxsel built-ins, they are duplicates
<snip>

>> -int do_sel(void)
>> -{
>> -  int i = 0;
>> -
>> -  si[i][0] = __builtin_vsx_xxsel_4si (si[i][1], si[i][2], si[i][3]); i++;
                                                              ^ changed to ui
>> -  ss[i][0] = __builtin_vsx_xxsel_8hi (ss[i][1], ss[i][2], ss[i][3]); i++;
                                                              ^ changed to ui
>> -  sc[i][0] = __builtin_vsx_xxsel_16qi (sc[i][1], sc[i][2], sc[i][3]); i++;
                                                               ^ changed to uc
>> -  f[i][0] = __builtin_vsx_xxsel_4sf (f[i][1], f[i][2], f[i][3]); i++;
>> -  d[i][0] = __builtin_vsx_xxsel_2df (d[i][1], d[i][2], d[i][3]); i++;
>> -
>> -  si[i][0] = __builtin_vsx_xxsel (si[i][1], si[i][2], bi[i][3]); i++;
>> -  ss[i][0] = __builtin_vsx_xxsel (ss[i][1], ss[i][2], bs[i][3]); i++;
>> -  sc[i][0] = __builtin_vsx_xxsel (sc[i][1], sc[i][2], bc[i][3]); i++;
>> -  f[i][0] = __builtin_vsx_xxsel (f[i][1], f[i][2], bi[i][3]); i++;
>> -  d[i][0] = __builtin_vsx_xxsel (d[i][1], d[i][2], bl[i][3]); i++;
>> -
>> -  si[i][0] = __builtin_vsx_xxsel (si[i][1], si[i][2], ui[i][3]); i++;
>> -  ss[i][0] = __builtin_vsx_xxsel (ss[i][1], ss[i][2], us[i][3]); i++;
>> -  sc[i][0] = __builtin_vsx_xxsel (sc[i][1], sc[i][2], uc[i][3]); i++;
>> -  f[i][0] = __builtin_vsx_xxsel (f[i][1], f[i][2], ui[i][3]); i++;
>> -  d[i][0] = __builtin_vsx_xxsel (d[i][1], d[i][2], ul[i][3]); i++;
>> -
>> -  return i;
>> -}
>> -
> 
> I prefer to keep them but just replacing the call with vec_sel.
> 
> OK with the above nits tweaked, thanks.

OK, changed __builtin_vsx_xxsel_4si_* to vec_sel, changed__builtin_vsx_xxsel to vec_sel.
Had to add #include <altivec.h>.

Finally, changed the third argument for the first three calls, as noted above, to be compatible with the vec_sel built-in specification.

                           Carl

> 
> BR,
> Kewen
> 
>>  int do_perm(void)
>>  {
>>    int i = 0;
> 

[PATCH 8/13] rs6000, remove __builtin_vsx_vperm_* built-ins

[Carl Love]

rs6000, remove __builtin_vsx_vperm_* built-ins

The undocumented built-ins:
  __builtin_vsx_vperm_16qi_uns,
  __builtin_vsx_vperm_1ti,
  __builtin_vsx_vperm_1ti_uns,
  __builtin_vsx_vperm_2df,
  __builtin_vsx_vperm_2di,
  __builtin_vsx_vperm_2di_uns,
  __builtin_vsx_vperm_4sf,
  __builtin_vsx_vperm_4si,
  __builtin_vsx_vperm_4si_uns

are duplicats of the __builtin_altivec_* builtins that are used by
the overloaded vec_perm built-in that is documented in the PVIPR.

gcc/ChangeLog:
	* config/rs6000/rs6000-builtins.def (__builtin_vsx_vperm_16qi_uns,
	__builtin_vsx_vperm_1ti, __builtin_vsx_vperm_1ti_uns,
	__builtin_vsx_vperm_2df, __builtin_vsx_vperm_2di,
	__builtin_vsx_vperm_2di_uns, __builtin_vsx_vperm_4sf,
	__builtin_vsx_vperm_4si, __builtin_vsx_vperm_4si_uns): Remove
	built-in definitions and comments.

gcc/testsuite/ChangeLog:
	* gcc.target/powerpc/vsx-builtin-3.c (__builtin_vsx_vperm_16qi_uns,
	 __builtin_vsx_vperm_1ti, __builtin_vsx_vperm_1ti_uns,
	__builtin_vsx_vperm_2df, __builtin_vsx_vperm_2di,
	__builtin_vsx_vperm_2di_uns, __builtin_vsx_vperm_4sf,
	__builtin_vsx_vperm_4si, __builtin_vsx_vperm_4si_uns): Remove
	test cases.
---
 gcc/config/rs6000/rs6000-builtins.def         | 33 -------------------
 .../gcc.target/powerpc/vsx-builtin-3.c        | 20 -----------
 2 files changed, 53 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index 3c409d729ea..f33564d3d9c 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1529,39 +1529,6 @@
   const vf __builtin_vsx_uns_floato_v2di (vsll);
     UNS_FLOATO_V2DI unsfloatov2di {}
 
-; 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.
-  const vsc __builtin_vsx_vperm_16qi (vsc, vsc, vuc);
-    VPERM_16QI_X altivec_vperm_v16qi {}
-
-  const vuc __builtin_vsx_vperm_16qi_uns (vuc, vuc, vuc);
-    VPERM_16QI_UNS_X altivec_vperm_v16qi_uns {}
-
-  const vsq __builtin_vsx_vperm_1ti (vsq, vsq, vsc);
-    VPERM_1TI_X altivec_vperm_v1ti {}
-
-  const vsq __builtin_vsx_vperm_1ti_uns (vsq, vsq, vsc);
-    VPERM_1TI_UNS_X altivec_vperm_v1ti_uns {}
-
-  const vd __builtin_vsx_vperm_2df (vd, vd, vuc);
-    VPERM_2DF_X altivec_vperm_v2df {}
-
-  const vsll __builtin_vsx_vperm_2di (vsll, vsll, vuc);
-    VPERM_2DI_X altivec_vperm_v2di {}
-
-  const vull __builtin_vsx_vperm_2di_uns (vull, vull, vuc);
-    VPERM_2DI_UNS_X altivec_vperm_v2di_uns {}
-
-  const vf __builtin_vsx_vperm_4sf (vf, vf, vuc);
-    VPERM_4SF_X altivec_vperm_v4sf {}
-
-  const vsi __builtin_vsx_vperm_4si (vsi, vsi, vuc);
-    VPERM_4SI_X altivec_vperm_v4si {}
-
-  const vui __builtin_vsx_vperm_4si_uns (vui, vui, vuc);
-    VPERM_4SI_UNS_X altivec_vperm_v4si_uns {}
-
   const vss __builtin_vsx_vperm_8hi (vss, vss, vuc);
     VPERM_8HI_X altivec_vperm_v8hi {}
 
diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
index 01f35dad713..35ea31b2616 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
@@ -2,7 +2,6 @@
 /* { dg-skip-if "" { powerpc*-*-darwin* } } */
 /* { dg-require-effective-target powerpc_vsx_ok } */
 /* { dg-options "-O2 -mdejagnu-cpu=power7" } */
-/* { dg-final { scan-assembler "vperm" } } */
 /* { dg-final { scan-assembler "xvrdpi" } } */
 /* { dg-final { scan-assembler "xvrdpic" } } */
 /* { dg-final { scan-assembler "xvrdpim" } } */
@@ -56,25 +55,6 @@ extern __vector unsigned long long ull[][4];
 extern __vector __bool long bl[][4];
 #endif
 
-int do_perm(void)
-{
-  int i = 0;
-
-  si[i][0] = __builtin_vsx_vperm_4si (si[i][1], si[i][2], uc[i][3]); i++;
-  ss[i][0] = __builtin_vsx_vperm_8hi (ss[i][1], ss[i][2], uc[i][3]); i++;
-  sc[i][0] = __builtin_vsx_vperm_16qi (sc[i][1], sc[i][2], uc[i][3]); i++;
-  f[i][0] = __builtin_vsx_vperm_4sf (f[i][1], f[i][2], uc[i][3]); i++;
-  d[i][0] = __builtin_vsx_vperm_2df (d[i][1], d[i][2], uc[i][3]); i++;
-
-  si[i][0] = __builtin_vsx_vperm (si[i][1], si[i][2], uc[i][3]); i++;
-  ss[i][0] = __builtin_vsx_vperm (ss[i][1], ss[i][2], uc[i][3]); i++;
-  sc[i][0] = __builtin_vsx_vperm (sc[i][1], sc[i][2], uc[i][3]); i++;
-  f[i][0] = __builtin_vsx_vperm (f[i][1], f[i][2], uc[i][3]); i++;
-  d[i][0] = __builtin_vsx_vperm (d[i][1], d[i][2], uc[i][3]); i++;
-
-  return i;
-}
-
 int do_xxperm (void)
 {
   int i = 0;
-- 
2.44.0

Re: [PATCH 8/13] rs6000, remove __builtin_vsx_vperm_* built-ins

[Kewen.Lin]

Hi,

on 2024/4/20 05:18, Carl Love wrote:
> rs6000, remove __builtin_vsx_vperm_* built-ins
> 
> The undocumented built-ins:
>   __builtin_vsx_vperm_16qi_uns,
>   __builtin_vsx_vperm_1ti,
>   __builtin_vsx_vperm_1ti_uns,
>   __builtin_vsx_vperm_2df,
>   __builtin_vsx_vperm_2di,
>   __builtin_vsx_vperm_2di_uns,
>   __builtin_vsx_vperm_4sf,
>   __builtin_vsx_vperm_4si,
>   __builtin_vsx_vperm_4si_uns
> 
> are duplicats of the __builtin_altivec_* builtins that are used by
> the overloaded vec_perm built-in that is documented in the PVIPR.
> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_vperm_16qi_uns,
> 	__builtin_vsx_vperm_1ti, __builtin_vsx_vperm_1ti_uns,
> 	__builtin_vsx_vperm_2df, __builtin_vsx_vperm_2di,
> 	__builtin_vsx_vperm_2di_uns, __builtin_vsx_vperm_4sf,
> 	__builtin_vsx_vperm_4si, __builtin_vsx_vperm_4si_uns): Remove
> 	built-in definitions and comments.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.target/powerpc/vsx-builtin-3.c (__builtin_vsx_vperm_16qi_uns,
> 	 __builtin_vsx_vperm_1ti, __builtin_vsx_vperm_1ti_uns,
> 	__builtin_vsx_vperm_2df, __builtin_vsx_vperm_2di,
> 	__builtin_vsx_vperm_2di_uns, __builtin_vsx_vperm_4sf,
> 	__builtin_vsx_vperm_4si, __builtin_vsx_vperm_4si_uns): Remove
> 	test cases.
> ---
>  gcc/config/rs6000/rs6000-builtins.def         | 33 -------------------
>  .../gcc.target/powerpc/vsx-builtin-3.c        | 20 -----------
>  2 files changed, 53 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 3c409d729ea..f33564d3d9c 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1529,39 +1529,6 @@
>    const vf __builtin_vsx_uns_floato_v2di (vsll);
>      UNS_FLOATO_V2DI unsfloatov2di {}
>  
> -; 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.
> -  const vsc __builtin_vsx_vperm_16qi (vsc, vsc, vuc);
> -    VPERM_16QI_X altivec_vperm_v16qi {}
> -
> -  const vuc __builtin_vsx_vperm_16qi_uns (vuc, vuc, vuc);
> -    VPERM_16QI_UNS_X altivec_vperm_v16qi_uns {}
> -
> -  const vsq __builtin_vsx_vperm_1ti (vsq, vsq, vsc);
> -    VPERM_1TI_X altivec_vperm_v1ti {}
> -
> -  const vsq __builtin_vsx_vperm_1ti_uns (vsq, vsq, vsc);
> -    VPERM_1TI_UNS_X altivec_vperm_v1ti_uns {}
> -
> -  const vd __builtin_vsx_vperm_2df (vd, vd, vuc);
> -    VPERM_2DF_X altivec_vperm_v2df {}
> -
> -  const vsll __builtin_vsx_vperm_2di (vsll, vsll, vuc);
> -    VPERM_2DI_X altivec_vperm_v2di {}
> -
> -  const vull __builtin_vsx_vperm_2di_uns (vull, vull, vuc);
> -    VPERM_2DI_UNS_X altivec_vperm_v2di_uns {}
> -
> -  const vf __builtin_vsx_vperm_4sf (vf, vf, vuc);
> -    VPERM_4SF_X altivec_vperm_v4sf {}
> -
> -  const vsi __builtin_vsx_vperm_4si (vsi, vsi, vuc);
> -    VPERM_4SI_X altivec_vperm_v4si {}
> -
> -  const vui __builtin_vsx_vperm_4si_uns (vui, vui, vuc);
> -    VPERM_4SI_UNS_X altivec_vperm_v4si_uns {}
> -
>    const vss __builtin_vsx_vperm_8hi (vss, vss, vuc);
>      VPERM_8HI_X altivec_vperm_v8hi {}
>  
> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> index 01f35dad713..35ea31b2616 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> @@ -2,7 +2,6 @@
>  /* { dg-skip-if "" { powerpc*-*-darwin* } } */
>  /* { dg-require-effective-target powerpc_vsx_ok } */
>  /* { dg-options "-O2 -mdejagnu-cpu=power7" } */
> -/* { dg-final { scan-assembler "vperm" } } */
>  /* { dg-final { scan-assembler "xvrdpi" } } */
>  /* { dg-final { scan-assembler "xvrdpic" } } */
>  /* { dg-final { scan-assembler "xvrdpim" } } */
> @@ -56,25 +55,6 @@ extern __vector unsigned long long ull[][4];
>  extern __vector __bool long bl[][4];
>  #endif
>  
> -int do_perm(void)
> -{
> -  int i = 0;
> -
> -  si[i][0] = __builtin_vsx_vperm_4si (si[i][1], si[i][2], uc[i][3]); i++;
> -  ss[i][0] = __builtin_vsx_vperm_8hi (ss[i][1], ss[i][2], uc[i][3]); i++;
> -  sc[i][0] = __builtin_vsx_vperm_16qi (sc[i][1], sc[i][2], uc[i][3]); i++;
> -  f[i][0] = __builtin_vsx_vperm_4sf (f[i][1], f[i][2], uc[i][3]); i++;
> -  d[i][0] = __builtin_vsx_vperm_2df (d[i][1], d[i][2], uc[i][3]); i++;
> -
> -  si[i][0] = __builtin_vsx_vperm (si[i][1], si[i][2], uc[i][3]); i++;
> -  ss[i][0] = __builtin_vsx_vperm (ss[i][1], ss[i][2], uc[i][3]); i++;
> -  sc[i][0] = __builtin_vsx_vperm (sc[i][1], sc[i][2], uc[i][3]); i++;
> -  f[i][0] = __builtin_vsx_vperm (f[i][1], f[i][2], uc[i][3]); i++;
> -  d[i][0] = __builtin_vsx_vperm (d[i][1], d[i][2], uc[i][3]); i++;
> -
> -  return i;
> -}
> -

I prefer to just relace these __builtin_vsx_vperm with vec_perm,
OK with this tweaked (also keep the above removed vperm scan), thanks!

BR,
Kewen

>  int do_xxperm (void)
>  {
>    int i = 0;

Re: [PATCH 8/13] rs6000, remove __builtin_vsx_vperm_* built-ins

[Carl Love]

Kewen:

On 5/13/24 19:59, Kewen.Lin wrote:
> Hi,
> 
> on 2024/4/20 05:18, Carl Love wrote:

<snip>

>> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
>> index 01f35dad713..35ea31b2616 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
>> @@ -2,7 +2,6 @@
>>  /* { dg-skip-if "" { powerpc*-*-darwin* } } */
>>  /* { dg-require-effective-target powerpc_vsx_ok } */
>>  /* { dg-options "-O2 -mdejagnu-cpu=power7" } */
>> -/* { dg-final { scan-assembler "vperm" } } */
>>  /* { dg-final { scan-assembler "xvrdpi" } } */
>>  /* { dg-final { scan-assembler "xvrdpic" } } */
>>  /* { dg-final { scan-assembler "xvrdpim" } } */
>> @@ -56,25 +55,6 @@ extern __vector unsigned long long ull[][4];
>>  extern __vector __bool long bl[][4];
>>  #endif
>>  
>> -int do_perm(void)
>> -{
>> -  int i = 0;
>> -
>> -  si[i][0] = __builtin_vsx_vperm_4si (si[i][1], si[i][2], uc[i][3]); i++;
>> -  ss[i][0] = __builtin_vsx_vperm_8hi (ss[i][1], ss[i][2], uc[i][3]); i++;
>> -  sc[i][0] = __builtin_vsx_vperm_16qi (sc[i][1], sc[i][2], uc[i][3]); i++;
>> -  f[i][0] = __builtin_vsx_vperm_4sf (f[i][1], f[i][2], uc[i][3]); i++;
>> -  d[i][0] = __builtin_vsx_vperm_2df (d[i][1], d[i][2], uc[i][3]); i++;
>> -
>> -  si[i][0] = __builtin_vsx_vperm (si[i][1], si[i][2], uc[i][3]); i++;
>> -  ss[i][0] = __builtin_vsx_vperm (ss[i][1], ss[i][2], uc[i][3]); i++;
>> -  sc[i][0] = __builtin_vsx_vperm (sc[i][1], sc[i][2], uc[i][3]); i++;
>> -  f[i][0] = __builtin_vsx_vperm (f[i][1], f[i][2], uc[i][3]); i++;
>> -  d[i][0] = __builtin_vsx_vperm (d[i][1], d[i][2], uc[i][3]); i++;
>> -
>> -  return i;
>> -}
>> -
> 
> I prefer to just relace these __builtin_vsx_vperm with vec_perm,
> OK with this tweaked (also keep the above removed vperm scan), thanks!

OK, sounds good.  Updated the patch to change built-in calls to vec_perm.  Updated ChangeLog message to match change.
       
                         Carl 

[PATCH 9/13] rs6000, remove __builtin_vsx_xvnegdp and __builtin_vsx_xvnegsp built-ins

[Carl Love]

rs6000, remove __builtin_vsx_xvnegdp and __builtin_vsx_xvnegsp built-ins

The undocumented __builtin_vsx_xvnegdp and __builtin_vsx_xvnegsp are
redundant.  The overloaded vec_neg built-in provides the same
functionality.  The two buit-ins are not documented nor are there any
test cases for them.

Remove the definitions so users will use the overloaded vec_neg built-in
which is documented in the PVIPR.

gcc/ChangeLog:
        * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvnegdp,
	__builtin_vsx_xvnegsp): Remove built-in definitions.
---
 gcc/config/rs6000/rs6000-builtins.def | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index f33564d3d9c..d65c858ac0c 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1763,12 +1763,6 @@
   const vf __builtin_vsx_xvnabssp (vf);
     XVNABSSP vsx_nabsv4sf2 {}
 
-  const vd __builtin_vsx_xvnegdp (vd);
-    XVNEGDP negv2df2 {}
-
-  const vf __builtin_vsx_xvnegsp (vf);
-    XVNEGSP negv4sf2 {}
-
   const vd __builtin_vsx_xvnmadddp (vd, vd, vd);
     XVNMADDDP nfmav2df4 {}
 
-- 
2.44.0

Re: [PATCH 9/13] rs6000, remove __builtin_vsx_xvnegdp and __builtin_vsx_xvnegsp built-ins

[Kewen.Lin]

Hi,

on 2024/4/20 05:18, Carl Love wrote:
> rs6000, remove __builtin_vsx_xvnegdp and __builtin_vsx_xvnegsp built-ins
> 
> The undocumented __builtin_vsx_xvnegdp and __builtin_vsx_xvnegsp are
> redundant.  The overloaded vec_neg built-in provides the same
> functionality.  The two buit-ins are not documented nor are there any
> test cases for them.
> 
> Remove the definitions so users will use the overloaded vec_neg built-in
> which is documented in the PVIPR.

OK, thanks!

BR,
Kewen

> 
> gcc/ChangeLog:
>         * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvnegdp,
> 	__builtin_vsx_xvnegsp): Remove built-in definitions.
> ---
>  gcc/config/rs6000/rs6000-builtins.def | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index f33564d3d9c..d65c858ac0c 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1763,12 +1763,6 @@
>    const vf __builtin_vsx_xvnabssp (vf);
>      XVNABSSP vsx_nabsv4sf2 {}
>  
> -  const vd __builtin_vsx_xvnegdp (vd);
> -    XVNEGDP negv2df2 {}
> -
> -  const vf __builtin_vsx_xvnegsp (vf);
> -    XVNEGSP negv4sf2 {}
> -
>    const vd __builtin_vsx_xvnmadddp (vd, vd, vd);
>      XVNMADDDP nfmav2df4 {}
>  

[PATCH 10/13] rs6000, extend vec_xxpermdi built-in for __int128 args

[Carl Love]

rs6000, extend vec_xxpermdi built-in for __int128 args

Add a new overloaded instance for vec_xxpermdi

   __int128 vec_xxpermdi (__int128, __int128, const int);

Update the documentation to include a reference to the new built-in
instance.

gcc/ChangeLog:
        * config/rs6000/rs6000-builtins.def (vec_xxpermdi): Add new
	overloaded built-in instance.
---
 gcc/config/rs6000/rs6000-overload.def | 2 ++
 gcc/doc/extend.texi                   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
index 5912c9452f4..49962e2f2a2 100644
--- a/gcc/config/rs6000/rs6000-overload.def
+++ b/gcc/config/rs6000/rs6000-overload.def
@@ -4932,6 +4932,8 @@
     XXPERMDI_4SF  XXPERMDI_VF
   vd __builtin_vsx_xxpermdi (vd, vd, const int);
     XXPERMDI_2DF  XXPERMDI_VD
+  vsq __builtin_vsx_xxpermdi (vsq, vsq, const int);
+    XXPERMDI_1TI  XXPERMDI_1TI
 
 [VEC_XXSLDWI, vec_xxsldwi, __builtin_vsx_xxsldwi]
   vsc __builtin_vsx_xxsldwi (vsc, vsc, const int);
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 86b8e536dbe..47cf2f3bc8b 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -22505,6 +22505,7 @@ void vec_vsx_st (vector bool char, int, vector bool char *);
 void vec_vsx_st (vector bool char, int, unsigned char *);
 void vec_vsx_st (vector bool char, int, signed char *);
 
+vector __int128 vec_xxpermdi (vector __int128, vector __int128, const int);
 vector double vec_xxpermdi (vector double, vector double, const int);
 vector float vec_xxpermdi (vector float, vector float, const int);
 vector long long vec_xxpermdi (vector long long, vector long long, const int);
-- 
2.44.0

Re: [PATCH 10/13] rs6000, extend vec_xxpermdi built-in for __int128 args

[Kewen.Lin]

Hi,

on 2024/4/20 05:18, Carl Love wrote:
> rs6000, extend vec_xxpermdi built-in for __int128 args
> 
> Add a new overloaded instance for vec_xxpermdi
> 
>    __int128 vec_xxpermdi (__int128, __int128, const int);
> 
> Update the documentation to include a reference to the new built-in
> instance.
> 
> gcc/ChangeLog:
>         * config/rs6000/rs6000-builtins.def (vec_xxpermdi): Add new
> 	overloaded built-in instance.
> ---
>  gcc/config/rs6000/rs6000-overload.def | 2 ++
>  gcc/doc/extend.texi                   | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
> index 5912c9452f4..49962e2f2a2 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -4932,6 +4932,8 @@
>      XXPERMDI_4SF  XXPERMDI_VF
>    vd __builtin_vsx_xxpermdi (vd, vd, const int);
>      XXPERMDI_2DF  XXPERMDI_VD
> +  vsq __builtin_vsx_xxpermdi (vsq, vsq, const int);
> +    XXPERMDI_1TI  XXPERMDI_1TI

This actually introduces the signed __int128, considering the other
existing ones, I think we want both signed and unsigned.

>  
>  [VEC_XXSLDWI, vec_xxsldwi, __builtin_vsx_xxsldwi]
>    vsc __builtin_vsx_xxsldwi (vsc, vsc, const int);
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 86b8e536dbe..47cf2f3bc8b 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -22505,6 +22505,7 @@ void vec_vsx_st (vector bool char, int, vector bool char *);
>  void vec_vsx_st (vector bool char, int, unsigned char *);
>  void vec_vsx_st (vector bool char, int, signed char *);
>  
> +vector __int128 vec_xxpermdi (vector __int128, vector __int128, const int);
>  vector double vec_xxpermdi (vector double, vector double, const int);
>  vector float vec_xxpermdi (vector float, vector float, const int);

Nit: Considering the existing ones sorted by element size descending, I guess
it's better to move the above here (and with the explicit signed and unsigned).

And we need a test case for it as well?

BR,
Kewen

>  vector long long vec_xxpermdi (vector long long, vector long long, const int);

Re: [PATCH 10/13] rs6000, extend vec_xxpermdi built-in for __int128 args

[Carl Love]

On 5/13/24 22:14, Kewen.Lin wrote:
> Hi,
> 
> on 2024/4/20 05:18, Carl Love wrote:
>> rs6000, extend vec_xxpermdi built-in for __int128 args
>>
>> Add a new overloaded instance for vec_xxpermdi
>>
>>    __int128 vec_xxpermdi (__int128, __int128, const int);
>>
>> Update the documentation to include a reference to the new built-in
>> instance.
>>
>> gcc/ChangeLog:
>>         * config/rs6000/rs6000-builtins.def (vec_xxpermdi): Add new
>> 	overloaded built-in instance.
>> ---
>>  gcc/config/rs6000/rs6000-overload.def | 2 ++
>>  gcc/doc/extend.texi                   | 1 +
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
>> index 5912c9452f4..49962e2f2a2 100644
>> --- a/gcc/config/rs6000/rs6000-overload.def
>> +++ b/gcc/config/rs6000/rs6000-overload.def
>> @@ -4932,6 +4932,8 @@
>>      XXPERMDI_4SF  XXPERMDI_VF
>>    vd __builtin_vsx_xxpermdi (vd, vd, const int);
>>      XXPERMDI_2DF  XXPERMDI_VD
>> +  vsq __builtin_vsx_xxpermdi (vsq, vsq, const int);
>> +    XXPERMDI_1TI  XXPERMDI_1TI
> 
> This actually introduces the signed __int128, considering the other
> existing ones, I think we want both signed and unsigned.

Added unsigned as well.

> 
>>  
>>  [VEC_XXSLDWI, vec_xxsldwi, __builtin_vsx_xxsldwi]
>>    vsc __builtin_vsx_xxsldwi (vsc, vsc, const int);
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 86b8e536dbe..47cf2f3bc8b 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -22505,6 +22505,7 @@ void vec_vsx_st (vector bool char, int, vector bool char *);
>>  void vec_vsx_st (vector bool char, int, unsigned char *);
>>  void vec_vsx_st (vector bool char, int, signed char *);
>>  
>> +vector __int128 vec_xxpermdi (vector __int128, vector __int128, const int);
>>  vector double vec_xxpermdi (vector double, vector double, const int);
>>  vector float vec_xxpermdi (vector float, vector float, const int);
> 
> Nit: Considering the existing ones sorted by element size descending, I guess
> it's better to move the above here (and with the explicit signed and unsigned).

OK, moved the new prototype down below the float prototype and added the unsigned prototype.
> 
> And we need a test case for it as well?
Yes, we need a test case for both.  Added a new runnable test file.

                   Carl 

[PATCH 11/13] rs6000, remove __builtin_vsx_xvcmpeqsp_p built-in

[Carl Love]

rs6000, remove __builtin_vsx_xvcmpeqsp_p built-in

The built-in __builtin_vsx_xvcmpeqsp_p is a duplicate of the overloaded
__builtin_altivec_vcmpeqfp_p built-in.  The built-in is undocumented and
there are no test cases for it.  The patch removes built-in
__builtin_vsx_xvcmpeqsp_p.

gcc/ChangeLog:
	* config/rs6000/rs6000-builtin.cc (case RS6000_BIF_RSQRT):
	Remove case statement.
        * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcmpeqsp_p):
	Remove built-in definition.
---
 gcc/config/rs6000/rs6000-builtin.cc   | 6 ------
 gcc/config/rs6000/rs6000-builtins.def | 6 ------
 2 files changed, 12 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
index f83d65b06ef..74ed8fc1805 100644
--- a/gcc/config/rs6000/rs6000-builtin.cc
+++ b/gcc/config/rs6000/rs6000-builtin.cc
@@ -269,12 +269,6 @@ rs6000_builtin_md_vectorized_function (tree fndecl, tree type_out,
     = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl);
   switch (fn)
     {
-    case RS6000_BIF_RSQRTF:
-      if (VECTOR_UNIT_ALTIVEC_OR_VSX_P (V4SFmode)
-	  && out_mode == SFmode && out_n == 4
-	  && in_mode == SFmode && in_n == 4)
-	return rs6000_builtin_decls[RS6000_BIF_VRSQRTFP];
-      break;
     case RS6000_BIF_RSQRT:
       if (VECTOR_UNIT_VSX_P (V2DFmode)
 	  && out_mode == DFmode && out_n == 2
diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index d65c858ac0c..2f6149edd5f 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -917,9 +917,6 @@
   fpmath vf __builtin_altivec_vrsqrtefp (vf);
     VRSQRTEFP rsqrtev4sf2 {}
 
-  fpmath vf __builtin_altivec_vrsqrtfp (vf);
-    VRSQRTFP rsqrtv4sf2 {}
-
   const vsc __builtin_altivec_vsel_16qi (vsc, vsc, vuc);
     VSEL_16QI vector_select_v16qi {}
 
@@ -1619,9 +1616,6 @@
   const vf __builtin_vsx_xvcmpeqsp (vf, vf);
     XVCMPEQSP vector_eqv4sf {}
 
-  const signed int __builtin_vsx_xvcmpeqsp_p (signed int, vf, vf);
-    XVCMPEQSP_P vector_eq_v4sf_p {pred}
-
   const vd __builtin_vsx_xvcmpgedp (vd, vd);
     XVCMPGEDP vector_gev2df {}
 
-- 
2.44.0

Re: [PATCH 11/13] rs6000, remove __builtin_vsx_xvcmpeqsp_p built-in

[Kewen.Lin]

Hi,

on 2024/4/20 05:18, Carl Love wrote:
> rs6000, remove __builtin_vsx_xvcmpeqsp_p built-in
> 
> The built-in __builtin_vsx_xvcmpeqsp_p is a duplicate of the overloaded
> __builtin_altivec_vcmpeqfp_p built-in.  The built-in is undocumented and
> there are no test cases for it.  The patch removes built-in
> __builtin_vsx_xvcmpeqsp_p.
As the previous review comments in the v1 (this is actually v2):
https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646728.html
, both __builtin_vsx_xvcmpeqsp_p and __builtin_vsx_xvcmpeqsp can be
dropped, so please consider __builtin_vsx_xvcmpeqsp as well.

> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtin.cc (case RS6000_BIF_RSQRT):
> 	Remove case statement.

It seems you mixed this with some other patch, this line doesn't
belong to this patch, ...

>         * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcmpeqsp_p):
> 	Remove built-in definition.
> ---
>  gcc/config/rs6000/rs6000-builtin.cc   | 6 ------
>  gcc/config/rs6000/rs6000-builtins.def | 6 ------
>  2 files changed, 12 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
> index f83d65b06ef..74ed8fc1805 100644
> --- a/gcc/config/rs6000/rs6000-builtin.cc
> +++ b/gcc/config/rs6000/rs6000-builtin.cc
> @@ -269,12 +269,6 @@ rs6000_builtin_md_vectorized_function (tree fndecl, tree type_out,
>      = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl);
>    switch (fn)
>      {
> -    case RS6000_BIF_RSQRTF:
> -      if (VECTOR_UNIT_ALTIVEC_OR_VSX_P (V4SFmode)
> -	  && out_mode == SFmode && out_n == 4
> -	  && in_mode == SFmode && in_n == 4)
> -	return rs6000_builtin_decls[RS6000_BIF_VRSQRTFP];
> -      break;

... and this ...

>      case RS6000_BIF_RSQRT:
>        if (VECTOR_UNIT_VSX_P (V2DFmode)
>  	  && out_mode == DFmode && out_n == 2
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index d65c858ac0c..2f6149edd5f 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -917,9 +917,6 @@
>    fpmath vf __builtin_altivec_vrsqrtefp (vf);
>      VRSQRTEFP rsqrtev4sf2 {}
>  
> -  fpmath vf __builtin_altivec_vrsqrtfp (vf);
> -    VRSQRTFP rsqrtv4sf2 {}
> -

..., also this.

BR,
Kewen

>    const vsc __builtin_altivec_vsel_16qi (vsc, vsc, vuc);
>      VSEL_16QI vector_select_v16qi {}
>  
> @@ -1619,9 +1616,6 @@
>    const vf __builtin_vsx_xvcmpeqsp (vf, vf);
>      XVCMPEQSP vector_eqv4sf {}
>  
> -  const signed int __builtin_vsx_xvcmpeqsp_p (signed int, vf, vf);
> -    XVCMPEQSP_P vector_eq_v4sf_p {pred}
> -
>    const vd __builtin_vsx_xvcmpgedp (vd, vd);
>      XVCMPGEDP vector_gev2df {}
>  

Re: [PATCH 11/13] rs6000, remove __builtin_vsx_xvcmpeqsp_p built-in

[Carl Love]

On 5/13/24 22:26, Kewen.Lin wrote:
> Hi,
> 
> on 2024/4/20 05:18, Carl Love wrote:
>> rs6000, remove __builtin_vsx_xvcmpeqsp_p built-in
>>
>> The built-in __builtin_vsx_xvcmpeqsp_p is a duplicate of the overloaded
>> __builtin_altivec_vcmpeqfp_p built-in.  The built-in is undocumented and
>> there are no test cases for it.  The patch removes built-in
>> __builtin_vsx_xvcmpeqsp_p.
> As the previous review comments in the v1 (this is actually v2):
> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646728.html
> , both __builtin_vsx_xvcmpeqsp_p and __builtin_vsx_xvcmpeqsp can be
> dropped, so please consider __builtin_vsx_xvcmpeqsp as well.

Yes, as you noted, __builtin_vsx_xvcmpeqsp is removed in the next patch.
> 
>>
>> gcc/ChangeLog:
>> 	* config/rs6000/rs6000-builtin.cc (case RS6000_BIF_RSQRT):
>> 	Remove case statement.
> 
> It seems you mixed this with some other patch, this line doesn't
> belong to this patch, ...

Took that out of this patch.  Didn't get the changes separated cleanly.

> 
>>         * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcmpeqsp_p):
>> 	Remove built-in definition.
>> ---
>>  gcc/config/rs6000/rs6000-builtin.cc   | 6 ------
>>  gcc/config/rs6000/rs6000-builtins.def | 6 ------
>>  2 files changed, 12 deletions(-)
>>
>> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
>> index f83d65b06ef..74ed8fc1805 100644
>> --- a/gcc/config/rs6000/rs6000-builtin.cc
>> +++ b/gcc/config/rs6000/rs6000-builtin.cc
>> @@ -269,12 +269,6 @@ rs6000_builtin_md_vectorized_function (tree fndecl, tree type_out,
>>      = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl);
>>    switch (fn)
>>      {
>> -    case RS6000_BIF_RSQRTF:
>> -      if (VECTOR_UNIT_ALTIVEC_OR_VSX_P (V4SFmode)
>> -	  && out_mode == SFmode && out_n == 4
>> -	  && in_mode == SFmode && in_n == 4)
>> -	return rs6000_builtin_decls[RS6000_BIF_VRSQRTFP];
>> -      break;
> 
> ... and this ...

Ditto

> 
>>      case RS6000_BIF_RSQRT:
>>        if (VECTOR_UNIT_VSX_P (V2DFmode)
>>  	  && out_mode == DFmode && out_n == 2
>> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
>> index d65c858ac0c..2f6149edd5f 100644
>> --- a/gcc/config/rs6000/rs6000-builtins.def
>> +++ b/gcc/config/rs6000/rs6000-builtins.def
>> @@ -917,9 +917,6 @@
>>    fpmath vf __builtin_altivec_vrsqrtefp (vf);
>>      VRSQRTEFP rsqrtev4sf2 {}
>>  
>> -  fpmath vf __builtin_altivec_vrsqrtfp (vf);
>> -    VRSQRTFP rsqrtv4sf2 {}
>> -
> 
> ..., also this.

Ditto

> 
> BR,
> Kewen
> 
>>    const vsc __builtin_altivec_vsel_16qi (vsc, vsc, vuc);
>>      VSEL_16QI vector_select_v16qi {}
>>  
>> @@ -1619,9 +1616,6 @@
>>    const vf __builtin_vsx_xvcmpeqsp (vf, vf);
>>      XVCMPEQSP vector_eqv4sf {}
>>  
>> -  const signed int __builtin_vsx_xvcmpeqsp_p (signed int, vf, vf);
>> -    XVCMPEQSP_P vector_eq_v4sf_p {pred}
>> -
>>    const vd __builtin_vsx_xvcmpgedp (vd, vd);
>>      XVCMPGEDP vector_gev2df {}
>>  

[PATCH 12/13] rs6000, remove __builtin_vsx_xvcmpeqsp built-in

[Carl Love]

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.

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;
-- 
2.44.0

Re: [PATCH 12/13] rs6000, remove __builtin_vsx_xvcmpeqsp built-in

[Kewen.Lin]

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!

BR,
Kewen

Re: [PATCH 12/13] rs6000, remove __builtin_vsx_xvcmpeqsp built-in

[Carl Love]

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
> 

Re: [PATCH 12/13] rs6000, remove __builtin_vsx_xvcmpeqsp built-in

[Kewen.Lin]

Hi,

on 2024/5/24 02:21, Carl Love wrote:
> 
> 
> 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.
> 

Ah, so they are not equivalent from prototype perspective.

> 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.

IMHO PVIPR prototype (returning vector bool) makes more sense,
it does match better with what the result holds.

> 
> 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++;

For the test case change, I'd expect that it can work with:

-  f[i][0] = __builtin_vsx_xvcmpeqsp (f[i][1], f[i][2]); i++;
+  f[i][0] = (vector float) 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?
> 

Since __builtin_vsx_xvcmpeqsp is an undocumented built-in, I don't
expect users to use it, even there is someone, IMHO vector bool is
a better fit.  In case someone actually wants the vector non-bool
type, he/she can just add an explicit conversion.  So I'm inclined
to remove the vsx_xvcmpeqsp, users should try to use PVIPR built-ins
as possible as they can.  But I'm also fine for holding on this, as
there are some other related built-ins cmp* (cmpge,cmpgt...), we
can re-visit and handle them together later.

BR,
Kewen

Re: [PATCH 12/13] rs6000, remove __builtin_vsx_xvcmpeqsp built-in

[Carl Love]

Kewen:

On 5/24/24 03:43, Kewen.Lin wrote:
> Hi,
> 
> on 2024/5/24 02:21, Carl Love wrote:
>>
>>
>> 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.
>>
> 
> Ah, so they are not equivalent from prototype perspective.
> 
>> 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.
> 
> IMHO PVIPR prototype (returning vector bool) makes more sense,
> it does match better with what the result holds.

Yes, I tend to agree.  I think the user would use be likely using the test so they could create a mask to selectively replace vector elements.  A bool type make more sense in that case.

> 
>>
>> 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++;
> 
> For the test case change, I'd expect that it can work with:
> 
> -  f[i][0] = __builtin_vsx_xvcmpeqsp (f[i][1], f[i][2]); i++;
> +  f[i][0] = (vector float) vec_cmpeq (f[i][1], f[i][2]); i++;

Yes, that does work.

> 
>>
>> I am thinking we should drop this patch from the series, i.e. don't remove __builtin_vsx_xvcmpeqsp.  Thoughts?
>>
> 
> Since __builtin_vsx_xvcmpeqsp is an undocumented built-in, I don't
> expect users to use it, even there is someone, IMHO vector bool is
> a better fit.  In case someone actually wants the vector non-bool
> type, he/she can just add an explicit conversion.  So I'm inclined
> to remove the vsx_xvcmpeqsp, users should try to use PVIPR built-ins
> as possible as they can.  But I'm also fine for holding on this, as
> there are some other related built-ins cmp* (cmpge,cmpgt...), we
> can re-visit and handle them together later.

My preference would be to skip this for now and then come back later with a new patch to address all of the various comparisons for both float and double.  

                                     Carl 

[PATCH 13/13] rs6000, remove vector set and vector init built-ins.

[Carl Love]

rs6000, remove vector set and vector init built-ins.

The vector init built-ins:

  __builtin_vec_init_v16qi, __builtin_vec_init_v8hi,
  __builtin_vec_init_v4si, __builtin_vec_init_v4sf,
  __builtin_vec_init_v2di, __builtin_vec_init_v2df,
  __builtin_vec_set_v1ti

perform the same operation as initializing the vector in C code.  For
example:

  result_v4si = __builtin_vec_init_v4si (1, 2, 3, 4);
  result_v4si = {1, 2, 3, 4};

These two constructs were tested and verified they generate identical
assembly instructions with no optimization and -O3 optimization.

The vector set built-ins:

  __builtin_vec_set_v16qi, __builtin_vec_set_v8hi.
  __builtin_vec_set_v4si, __builtin_vec_set_v4sf

perform the same operation as setting a specific element in the vector in
C code.  For example:

  src_v4si = __builtin_vec_set_v4si (src_v4si, int_val, index);
  src_v4si[index] = int_val;

The built-in actually generates more instructions than the inline C code
with no optimization but is identical with -O3 optimizations.

All of the above built-ins that are removed do not have test cases and
are not documented.

Built-ins   __builtin_vec_set_v1ti __builtin_vec_set_v2di,
__builtin_vec_set_v2df are not removed as they are used in function
resolve_vec_insert() in file rs6000-c.cc.

The built-ins are removed as they don't provide any benefit over just
using C code.

gcc/ChangeLog:
	* config/rs6000/rs6000-builtins.def (__builtin_vec_init_v16qi,
	 __builtin_vec_init_v8hi, __builtin_vec_init_v4si,
	__builtin_vec_init_v4sf, __builtin_vec_init_v2di,
	__builtin_vec_init_v2df, __builtin_vec_set_v1ti,
	__builtin_vec_set_v16qi, __builtin_vec_set_v8hi.
	__builtin_vec_set_v4si, __builtin_vec_set_v4sf,
	__builtin_vec_set_v2di, __builtin_vec_set_v2df,
	__builtin_vec_set_v1ti): Remove built-in definitions.
---
 gcc/config/rs6000/rs6000-builtins.def | 42 ++-------------------------
 1 file changed, 2 insertions(+), 40 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index 19d05b8043a..d04ad4ce7e5 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1115,37 +1115,6 @@
   const signed short __builtin_vec_ext_v8hi (vss, signed int);
     VEC_EXT_V8HI nothing {extract}
 
-  const vsc __builtin_vec_init_v16qi (signed char, signed char, signed char, \
-            signed char, signed char, signed char, signed char, signed char, \
-            signed char, signed char, signed char, signed char, signed char, \
-            signed char, signed char, signed char);
-    VEC_INIT_V16QI nothing {init}
-
-  const vf __builtin_vec_init_v4sf (float, float, float, float);
-    VEC_INIT_V4SF nothing {init}
-
-  const vsi __builtin_vec_init_v4si (signed int, signed int, signed int, \
-                                     signed int);
-    VEC_INIT_V4SI nothing {init}
-
-  const vss __builtin_vec_init_v8hi (signed short, signed short, signed short,\
-             signed short, signed short, signed short, signed short, \
-             signed short);
-    VEC_INIT_V8HI nothing {init}
-
-  const vsc __builtin_vec_set_v16qi (vsc, signed char, const int<4>);
-    VEC_SET_V16QI nothing {set}
-
-  const vf __builtin_vec_set_v4sf (vf, float, const int<2>);
-    VEC_SET_V4SF nothing {set}
-
-  const vsi __builtin_vec_set_v4si (vsi, signed int, const int<2>);
-    VEC_SET_V4SI nothing {set}
-
-  const vss __builtin_vec_set_v8hi (vss, signed short, const int<3>);
-    VEC_SET_V8HI nothing {set}
-
-
 ; Cell builtins.
 [cell]
   pure vsc __builtin_altivec_lvlx (signed long, const void *);
@@ -1292,15 +1261,8 @@
   const signed long long __builtin_vec_ext_v2di (vsll, signed int);
     VEC_EXT_V2DI nothing {extract}
 
-  const vsq __builtin_vec_init_v1ti (signed __int128);
-    VEC_INIT_V1TI nothing {init}
-
-  const vd __builtin_vec_init_v2df (double, double);
-    VEC_INIT_V2DF nothing {init}
-
-  const vsll __builtin_vec_init_v2di (signed long long, signed long long);
-    VEC_INIT_V2DI nothing {init}
-
+;; VEC_SET_V1TI, VEC_SET_V2DF and VEC_SET_V2DI are used in
+;; resolve_vec_insert(), rs6000-c.cc
   const vsq __builtin_vec_set_v1ti (vsq, signed __int128, const int<0,0>);
     VEC_SET_V1TI nothing {set}
 
-- 
2.44.0

Re: [PATCH 13/13] rs6000, remove vector set and vector init built-ins.

[Kewen.Lin]

Hi,

on 2024/4/20 05:18, Carl Love wrote:
> rs6000, remove vector set and vector init built-ins.
> 
> The vector init built-ins:
> 
>   __builtin_vec_init_v16qi, __builtin_vec_init_v8hi,
>   __builtin_vec_init_v4si, __builtin_vec_init_v4sf,
>   __builtin_vec_init_v2di, __builtin_vec_init_v2df,
>   __builtin_vec_set_v1ti
> 
> perform the same operation as initializing the vector in C code.  For
> example:
> 
>   result_v4si = __builtin_vec_init_v4si (1, 2, 3, 4);
>   result_v4si = {1, 2, 3, 4};
> 
> These two constructs were tested and verified they generate identical
> assembly instructions with no optimization and -O3 optimization.
> 
> The vector set built-ins:
> 
>   __builtin_vec_set_v16qi, __builtin_vec_set_v8hi.
>   __builtin_vec_set_v4si, __builtin_vec_set_v4sf
> 
> perform the same operation as setting a specific element in the vector in
> C code.  For example:
> 
>   src_v4si = __builtin_vec_set_v4si (src_v4si, int_val, index);
>   src_v4si[index] = int_val;
> 
> The built-in actually generates more instructions than the inline C code
> with no optimization but is identical with -O3 optimizations.
> 
> All of the above built-ins that are removed do not have test cases and
> are not documented.
> 
> Built-ins   __builtin_vec_set_v1ti __builtin_vec_set_v2di,
> __builtin_vec_set_v2df are not removed as they are used in function
> resolve_vec_insert() in file rs6000-c.cc.

I think we can replace these calls with the equivalent gimple codes
(early expanding it) and then we can get rid of these instances.

BR,
Kewen

> 
> The built-ins are removed as they don't provide any benefit over just
> using C code.
> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtins.def (__builtin_vec_init_v16qi,
> 	 __builtin_vec_init_v8hi, __builtin_vec_init_v4si,
> 	__builtin_vec_init_v4sf, __builtin_vec_init_v2di,
> 	__builtin_vec_init_v2df, __builtin_vec_set_v1ti,
> 	__builtin_vec_set_v16qi, __builtin_vec_set_v8hi.
> 	__builtin_vec_set_v4si, __builtin_vec_set_v4sf,
> 	__builtin_vec_set_v2di, __builtin_vec_set_v2df,
> 	__builtin_vec_set_v1ti): Remove built-in definitions.
> ---
>  gcc/config/rs6000/rs6000-builtins.def | 42 ++-------------------------
>  1 file changed, 2 insertions(+), 40 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 19d05b8043a..d04ad4ce7e5 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1115,37 +1115,6 @@
>    const signed short __builtin_vec_ext_v8hi (vss, signed int);
>      VEC_EXT_V8HI nothing {extract}
>  
> -  const vsc __builtin_vec_init_v16qi (signed char, signed char, signed char, \
> -            signed char, signed char, signed char, signed char, signed char, \
> -            signed char, signed char, signed char, signed char, signed char, \
> -            signed char, signed char, signed char);
> -    VEC_INIT_V16QI nothing {init}
> -
> -  const vf __builtin_vec_init_v4sf (float, float, float, float);
> -    VEC_INIT_V4SF nothing {init}
> -
> -  const vsi __builtin_vec_init_v4si (signed int, signed int, signed int, \
> -                                     signed int);
> -    VEC_INIT_V4SI nothing {init}
> -
> -  const vss __builtin_vec_init_v8hi (signed short, signed short, signed short,\
> -             signed short, signed short, signed short, signed short, \
> -             signed short);
> -    VEC_INIT_V8HI nothing {init}
> -
> -  const vsc __builtin_vec_set_v16qi (vsc, signed char, const int<4>);
> -    VEC_SET_V16QI nothing {set}
> -
> -  const vf __builtin_vec_set_v4sf (vf, float, const int<2>);
> -    VEC_SET_V4SF nothing {set}
> -
> -  const vsi __builtin_vec_set_v4si (vsi, signed int, const int<2>);
> -    VEC_SET_V4SI nothing {set}
> -
> -  const vss __builtin_vec_set_v8hi (vss, signed short, const int<3>);
> -    VEC_SET_V8HI nothing {set}
> -
> -
>  ; Cell builtins.
>  [cell]
>    pure vsc __builtin_altivec_lvlx (signed long, const void *);
> @@ -1292,15 +1261,8 @@
>    const signed long long __builtin_vec_ext_v2di (vsll, signed int);
>      VEC_EXT_V2DI nothing {extract}
>  
> -  const vsq __builtin_vec_init_v1ti (signed __int128);
> -    VEC_INIT_V1TI nothing {init}
> -
> -  const vd __builtin_vec_init_v2df (double, double);
> -    VEC_INIT_V2DF nothing {init}
> -
> -  const vsll __builtin_vec_init_v2di (signed long long, signed long long);
> -    VEC_INIT_V2DI nothing {init}
> -
> +;; VEC_SET_V1TI, VEC_SET_V2DF and VEC_SET_V2DI are used in
> +;; resolve_vec_insert(), rs6000-c.cc
>    const vsq __builtin_vec_set_v1ti (vsq, signed __int128, const int<0,0>);
>      VEC_SET_V1TI nothing {set}
>  

Re: [PATCH 13/13] rs6000, remove vector set and vector init built-ins.

[Carl Love]

Kewen:

On 5/13/24 22:44, Kewen.Lin wrote:
>> perform the same operation as setting a specific element in the vector in
>> C code.  For example:
>>
>>   src_v4si = __builtin_vec_set_v4si (src_v4si, int_val, index);
>>   src_v4si[index] = int_val;
>>
>> The built-in actually generates more instructions than the inline C code
>> with no optimization but is identical with -O3 optimizations.
>>
>> All of the above built-ins that are removed do not have test cases and
>> are not documented.
>>
>> Built-ins   __builtin_vec_set_v1ti __builtin_vec_set_v2di,
>> __builtin_vec_set_v2df are not removed as they are used in function
>> resolve_vec_insert() in file rs6000-c.cc.
> I think we can replace these calls with the equivalent gimple codes
> (early expanding it) and then we can get rid of these instances.

Hmm, going to need a little coaching here.  I am not sure how to do this.  Looks like I get to lean some  something new.

                   Carl 

Re: [PATCH 13/13] rs6000, remove vector set and vector init built-ins.

[Kewen.Lin]

Hi Carl,

on 2024/5/23 08:29, Carl Love wrote:
> Kewen:
> 
> On 5/13/24 22:44, Kewen.Lin wrote:
>>> perform the same operation as setting a specific element in the vector in
>>> C code.  For example:
>>>
>>>   src_v4si = __builtin_vec_set_v4si (src_v4si, int_val, index);
>>>   src_v4si[index] = int_val;
>>>
>>> The built-in actually generates more instructions than the inline C code
>>> with no optimization but is identical with -O3 optimizations.
>>>
>>> All of the above built-ins that are removed do not have test cases and
>>> are not documented.
>>>
>>> Built-ins   __builtin_vec_set_v1ti __builtin_vec_set_v2di,
>>> __builtin_vec_set_v2df are not removed as they are used in function
>>> resolve_vec_insert() in file rs6000-c.cc.
>> I think we can replace these calls with the equivalent gimple codes
>> (early expanding it) and then we can get rid of these instances.
> 
> Hmm, going to need a little coaching here.  I am not sure how to do this.  Looks like I get to lean some  something new.
> 

We have functions rs6000_gimple_fold.*_builtin to fold the builtins,
it's folding (expanding) the bif with equivalent gimple codes, what
we want here is similar, you can refer to some implementation there.
For the expected gimple code, you can refer to what's generated with
normal C code.  Feel free to let me know when you meet some issues
when you are trying, even you prefer me to follow up this.

BR,
Kewen

[PING} Re: [PATCH 0/13] rs6000, built-in cleanup patch series

[Carl Love]

Ping, just wondering if anyone has had a chance to look at the patch series.

Thanks.

                                  Carl  

On 4/19/24 14:04, Carl Love wrote:
> GCC maintainers:
> 
> The following patch series removes duplicate built-ins.  There are patches to extend an existing overloaded built-in to cover additional input types.  The final patch removes built-ins to set and initialize vectors.  The code generated by these built-ins with the default optimization is efficient than the code generated by using straight C code.  The assembly code for the built-in and straight C code is the same with -O3
> optimizations.  In this case, the built-ins are removed as they add no additional value.
> 
> The patches have all been tested on Power 10 LE.  The last patch was also tested on Power 8 BE.
> 
> No regression tests were seen.
> 
> Please let me know if the patches are acceptable for mainline.  Thanks.
> 
>                        Carl 
>