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