Hi Carl,
on 2024/6/14 03:40, Carl Love wrote:
>
> GCC maintainers:
>
> The patch has been updated per the comments from version 3. Please let me
> know if the patch is acceptable for mainline.
>
> Carl
>
> -----------------------------------------------------------------
>
> rs6000, add overloaded vec_sel with int128 arguments
>
> Extend the vec_sel built-in to take three signed/unsigned/bool int128
> arguments and return a signed/unsigned/bool 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.
Nit: unexpected tab between "overloaded" and "definitions", should be a space,
better to mention which types of overloaded function are added, like "
for vector signed, unsigned and bool int128 types."
> * doc/extend.texi: Add documentation for new vec_sel instances.
Likewise.
>
> gcc/testsuite/ChangeLog:
> * gcc.target/powerpc/builtins-10-runnable.c: New runnable test
> file.
> * gcc.target/powerpc/builtins-10.c: New compile only test file.
> ---
> gcc/config/rs6000/rs6000-builtins.def | 6 -
> gcc/config/rs6000/rs6000-overload.def | 12 +
> gcc/doc/extend.texi | 20 ++
> .../gcc.target/powerpc/builtins-10-runnable.c | 220 ++++++++++++++++++
> .../gcc.target/powerpc/builtins-10.c | 63 +++++
> 5 files changed, 315 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/builtins-10-runnable.c
> create mode 100644 gcc/testsuite/gcc.target/powerpc/builtins-10.c
>
> diff --git a/gcc/config/rs6000/rs6000-builtins.def
> b/gcc/config/rs6000/rs6000-builtins.def
> index b90b3f34167..c969cd0f3f6 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1907,12 +1907,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..6cec1ad4f1a 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -3274,6 +3274,18 @@
> VSEL_2DF VSEL_2DF_B
> vd __builtin_vec_sel (vd, vd, vull);
> VSEL_2DF VSEL_2DF_U
> + vsq __builtin_vec_sel (vsq, vsq, vbq);
> + VSEL_1TI VSEL_1TI_B
> + vsq __builtin_vec_sel (vsq, vsq, vuq);
> + VSEL_1TI VSEL_1TI_U
> + vuq __builtin_vec_sel (vuq, vuq, vbq);
> + VSEL_1TI_UNS VSEL_1TI_UB
> + vuq __builtin_vec_sel (vuq, vuq, vuq);
> + VSEL_1TI_UNS VSEL_1TI_UU
> + vbq __builtin_vec_sel (vbq, vbq, vbq);
> + VSEL_1TI_UNS VSEL_1TI_BB
> + vbq __builtin_vec_sel (vbq, vbq, vuq);
> + VSEL_1TI_UNS VSEL_1TI_BU
Nit: Put these new lines after line "VSEL_2DI_UNS VSEL_2DI_BU"
and before "vf __builtin_vec_sel (vf, vf, vbi);", to make all
integral element type be placed together.
> ; 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 b1620274285..d7d8d149a43 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -21420,6 +21420,26 @@ 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 bool __int128);
> +vector signed __int128 vec_sel (vector signed __int128,
> + vector signed __int128, vector unsigned __int128);
> +vector unsigned __int128 vec_sel (vector unsigned __int128,
> + vector unsigned __int128, vector bool __int128);
> +vector unsigned __int128 vec_sel (vector unsigned __int128,
> + vector unsigned __int128, vector unsigned __int128);
> +vector bool __int128 vec_sel (vector bool __int128,
> + vector bool __int128, vector bool __int128);
> +vector bool __int128 vec_sel (vector bool __int128,
> + vector bool __int128, vector unsigned __int128);
> +@end smallexample
> +
> +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/builtins-10-runnable.c
> b/gcc/testsuite/gcc.target/powerpc/builtins-10-runnable.c
> new file mode 100644
> index 00000000000..b7b4a95ea0e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-10-runnable.c
> @@ -0,0 +1,220 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target vmx_hw } */
> +/* { dg-options "-maltivec -O2 " } */
> +
> +#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;
> + vector bool __int128 b128;
> + 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 check_b128_result(vector bool __int128 vresult_b128,
> + vector bool __int128 expected_vresult_b128)
> +{
> + /* Convert the arguments to unsigned, then check equality. */
> + union convert_union result;
> + union convert_union expected;
> +
> + result.b128 = vresult_b128;
> + expected.b128 = expected_vresult_b128;
> +
> + 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;
> +
> + vector bool __int128 src_va_b128;
> + vector bool __int128 src_vb_b128;
> + vector bool __int128 src_vc_b128;
> + vector bool __int128 vresult_b128;
> + vector bool __int128 expected_vresult_b128;
> +
> + src_va_s128 = (vector signed __int128) {0x123456789ABCDEF0};
> + src_vb_s128 = (vector signed __int128) {0xFEDCBA9876543210};
> + src_vc_b128 = (vector bool __int128) {0x3333333333333333};
> + src_vc_u128 = (vector unsigned __int128) {0xBBBBBBBBBBBBBBBB};
> +
> + /* Signed arguments. */
> + expected_vresult_s128 = (vector signed __int128) {0x32147658ba9cfed0};
> + vresult_s128 = vec_sel (src_va_s128, src_vb_s128, src_vc_b128);
> +
> + if (!check_s128_result (vresult_s128, expected_vresult_s128))
> +#if DEBUG
> + {
> + printf ("ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_b128) 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
> +
> + expected_vresult_s128 = (vector signed __int128) {0xba9cfed832147650};
> + vresult_s128 = vec_sel (src_va_s128, src_vb_s128, src_vc_u128);
> +
> + if (!check_s128_result (vresult_s128, expected_vresult_s128))
> +#if DEBUG
> + {
> + printf ("ERROR, vec_sel (src_va_s128, src_vb_s128, src_vc_u128) 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_va_b128 = (vector bool __int128) {0xFFFFFFFF00000000};
> + src_vb_u128 = (vector unsigned __int128) {0xA987654FEDCB3210};
> + src_vb_b128 = (vector bool __int128) {0xFFFF0000FFFF0000};
> + src_vc_u128 = (vector unsigned __int128) {0x5555555555555555};
> +
> + /* Unigned arguments. */
Nit: s/Unigned/Unsigned/
> + expected_vresult_u128 = (vector unsigned __int128) {0x2147a9cf2147badc};
> + vresult_u128 = vec_sel (src_va_u128, src_vb_u128, src_vc_b128);
> +
> + if (!check_u128_result (vresult_u128, expected_vresult_u128))
> +#if DEBUG
> + {
> + printf ("ERROR, vec_sel (src_va_u128, src_vb_u128, src_vc_b128) 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
> +
> + expected_vresult_u128 = (vector unsigned __int128) {0x307cfcf47439a9a};
> + 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
> +
> + /* Boolean arguments. */
> + expected_vresult_b128 = (vector bool __int128) {0xffffcccc33330000};
> + vresult_b128 = vec_sel (src_va_b128, src_vb_b128, src_vc_b128);
> +
> + if (!check_b128_result (vresult_b128, expected_vresult_b128))
> +#if DEBUG
> + {
> + printf ("ERROR, vec_sel (src_va_b128, src_vb_b128, src_vc_b128) result
> does not match expected output.\n");
> + printf (" Result: ");
> + print_i128 ((unsigned __int128) vresult_b128);
> + printf ("\n Expected result: ");
> + print_i128 ((unsigned __int128) expected_vresult_b128);
> + printf ("\n");
> + }
> +#else
> + abort ();
> +#endif
> +
> + expected_vresult_b128 = (vector bool __int128) {0xffffaaaa55550000};
> + vresult_b128 = vec_sel (src_va_b128, src_vb_b128, src_vc_u128);
> +
> + if (!check_b128_result (vresult_b128, expected_vresult_b128))
> +#if DEBUG
> + {
> + printf ("ERROR, vec_sel (src_va_b128, src_vb_b128, src_vc_u128) result
> does not match expected output.\n");
> + printf (" Result: ");
> + print_i128 ((unsigned __int128) vresult_b128);
> + printf ("\n Expected result: ");
> + print_i128 ((unsigned __int128) expected_vresult_b128);
> + printf ("\n");
> + }
> +#else
> + abort ();
> +#endif
> +
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-10.c
> b/gcc/testsuite/gcc.target/powerpc/builtins-10.c
> new file mode 100644
> index 00000000000..eddc4c93b32
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-10.c
> @@ -0,0 +1,63 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target vmx_hw } */
s/vmx_hw/powerpc_altivec/, this is for compiling, shouldn't use _hw.
> +/* { dg-options "-save-temps" } */
s/-save-temps/-O2 -maltivec/
Also move dg-options line before powerpc_altivec line (as powerpc_altivec
evaluation considers current_compiler_flags now).
OK for trunk with all the above fixed. Thanks!
BR,
Kewen
> +/* { dg-final { scan-assembler-times "xxsel" 6 } } */
> +
> +#include <altivec.h>
> +
> +/* Signed args */
> +vector signed __int128
> +test_vec_sel_ssb (vector signed __int128 src_va_s128,
> + vector signed __int128 src_vb_s128,
> + vector bool __int128 src_vc_b128)
> +{
> + return vec_sel (src_va_s128, src_vb_s128, src_vc_b128);
> +}
> +
> +vector signed __int128
> +test_vec_sel_ssu (vector signed __int128 src_va_s128,
> + vector signed __int128 src_vb_s128,
> + vector unsigned __int128 src_vc_u128)
> +{
> + return vec_sel (src_va_s128, src_vb_s128, src_vc_u128);
> +}
> +
> +/* Unsigned args */
> +vector unsigned __int128
> +test_vec_sel_uub (vector unsigned __int128 src_va_u128,
> + vector unsigned __int128 src_vb_u128,
> + vector bool __int128 src_vc_b128)
> +{
> + return vec_sel (src_va_u128, src_vb_u128, src_vc_b128);
> +}
> +
> +vector unsigned __int128
> +test_vec_sel_uuu (vector unsigned __int128 src_va_u128,
> + vector unsigned __int128 src_vb_u128,
> + vector unsigned __int128 src_vc_u128)
> +{
> + return vec_sel (src_va_u128, src_vb_u128, src_vc_u128);
> +}
> +
> +/* Boolean args */
> +vector bool __int128
> +test_vec_sel_bbb (vector bool __int128 src_va_b128,
> + vector bool __int128 src_vb_b128,
> + vector bool __int128 src_vc_b128)
> +{
> + return vec_sel (src_va_b128, src_vb_b128, src_vc_b128);
> +}
> +
> +vector bool __int128
> +test_vec_sel_bbu (vector bool __int128 src_va_b128,
> + vector bool __int128 src_vb_b128,
> + vector unsigned __int128 src_vc_u128)
> +{
> + return vec_sel (src_va_b128, src_vb_b128, src_vc_u128);
> +}
> +
> +/* Expected results:
> + vec_sel xxsel */
> +
> +/* { dg-final { scan-assembler-times "xxsel" 6 } } */
> +
