Kewen:
On 6/3/24 22:58, Kewen.Lin wrote:
> Hi,
>
> on 2024/5/30 00:03, Carl Love wrote:
>> This was patch 6 in the previous series. Updated the documentation file per
>> the comments. No functional changes to the patch.
>>
>> Carl
>> ------------------------------------------------------------
>>
>> 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 instances.
>>
>> gcc/testsuite/ChangeLog:
>> * gcc.target/powerpc/vec-sel-runnable-i128.c: New test file.
>> ---
>> gcc/config/rs6000/rs6000-builtins.def | 6 -
>> gcc/config/rs6000/rs6000-overload.def | 4 +
>> gcc/doc/extend.texi | 12 ++
>> .../powerpc/vec-sel-runnable-i128.c | 129 ++++++++++++++++++
>> 4 files changed, 145 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 13e36df008d..ea0da77f13e 100644
>> --- a/gcc/config/rs6000/rs6000-builtins.def
>> +++ b/gcc/config/rs6000/rs6000-builtins.def
>> @@ -1904,12 +1904,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 4d857bb1af3..a210c5ad10d 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
>
> I just noticed that for integral types, such as: signed/unsigned int, we have
> six instances:
>
> vsi __builtin_vec_sel (vsi, vsi, vbi);
> VSEL_4SI VSEL_4SI_B
> vsi __builtin_vec_sel (vsi, vsi, vui);
> VSEL_4SI VSEL_4SI_U
> vui __builtin_vec_sel (vui, vui, vbi);
> VSEL_4SI_UNS VSEL_4SI_UB
> vui __builtin_vec_sel (vui, vui, vui);
> VSEL_4SI_UNS VSEL_4SI_UU
> vbi __builtin_vec_sel (vbi, vbi, vbi);
> VSEL_4SI_UNS VSEL_4SI_BB
> vbi __builtin_vec_sel (vbi, vbi, vui);
>
> It considers the control vector can only have unsigned and bool types, also
> consider the
> return type can be bool. It aligns with what PVIPR defines, so here we
> should have:
>
> vsq __builtin_vec_sel (vsq, vsq, vbq);
> vsq __builtin_vec_sel (vsq, vsq, vuq);
> vuq __builtin_vec_sel (vuq, vuq, vbq);
> vuq __builtin_vec_sel (vuq, vuq, vuq);
> vbq __builtin_vec_sel (vbq, vbq, vbq);
> vbq __builtin_vec_sel (vbq, vbq, vuq);
>
> Sorry that I didn't find this in the previous review.
Yea, my bad I missed that as well. Fixed to add all six instances.
>
>
>> ; 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 b88e61641a2..0756230b19e 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -21372,6 +21372,18 @@ Additional built-in functions are available for the
>> 64-bit PowerPC
>> family of processors, for efficient use of 128-bit floating point
>> (@code{__float128}) values.
>>
>> +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
>
> As above, the documentation here has to consider vector bool __int128 and
> note that
> the control vector are of type either vector unsigned __int128 or vector bool
> __int128.
>
>> +
>> +The instance is an extension of the exiting overloaded built-in
>> @code{vec_sel}
>> +that is documented in the PVIPR.
>> +
>> @node Basic PowerPC Built-in Functions Available on ISA 2.06
>> @subsubsection Basic PowerPC Built-in Functions Available on ISA 2.06
>>
>> 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..d82225cc847
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-sel-runnable-i128.c
>> @@ -0,0 +1,129 @@
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target vmx_hw } */
>> +/* { dg-options "-save-temps" } */
>> +/* { dg-final { scan-assembler-times "xxsel" 2 } } */
>
> Nit: Can we rename this case to builtins-10.c and separate it into
> one dg-do compile and the other dg-do "run" (like some builtins-*.c
> which have *-x.c and *-x-runnable.c)?
>
> It needs some more adjustments as the overloaded instances change.
Renamed run test file builtins-10-runnable.c . Added tests for all six
instances.
Created builtins-10.c as a compile only test that checks for 6 xxsel instances.
>
> BR,
> Kewen
>
>> +
>> +#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);
>> +
>> +union convert_union {
>> + vector signed __int128 s128;
>> + vector unsigned __int128 u128;
>> + char val[16];
>> +} convert;
>> +
>> +int check_u128_result(vector unsigned __int128 vresult_u128,
>> + vector unsigned __int128 expected_vresult_u128)
>> +{
>> + /* Use a for loop to check each byte manually so the test case will run
>> + with ISA 2.06.
>> +
>> + Return 1 if they match, 0 otherwise. */
>> +
>> + int i;
>> +
>> + union convert_union result;
>> + union convert_union expected;
>> +
>> + result.u128 = vresult_u128;
>> + expected.u128 = expected_vresult_u128;
>> +
>> + /* Check if each byte of the result and expected match. */
>> + for (i = 0; i < 16; i++)
>> + {
>> + if (result.val[i] != expected.val[i])
>> + return 0;
>> + }
>> + return 1;
>> +}
>> +
>> +int check_s128_result(vector signed __int128 vresult_s128,
>> + vector signed __int128 expected_vresult_s128)
>> +{
>> + /* Convert the arguments to unsigned, then check equality. */
>> + union convert_union result;
>> + union convert_union expected;
>> +
>> + result.s128 = vresult_s128;
>> + expected.s128 = expected_vresult_s128;
>> +
>> + return check_u128_result (result.u128, expected.u128);
>> +}
>> +
>> +
>> +int
>> +main (int argc, char *argv [])
>> +{
>> + int i;
>> +
>> + 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 (!check_s128_result (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) {0x0307CFCF47439A9A};
>> +
>> + /* Unigned arguments. */
>> + vresult_u128 = vec_sel (src_va_u128, src_vb_u128, src_vc_u128);
>> +
>> + if (!check_u128_result (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;
>> +}
>
