Hi Carl,
on 2024/8/8 01:15, Carl Love wrote:
>
> GCC maintainers:
>
> The following patch adds missing test cases for the overloaded vec_perm
> built-in. It also fixes and issue with printing the 128-bit values in the
> DEBUG section that was noticed when adding the additional test cases.
>
> The patch has been tested on Power 10 LE and BE with no regressions.
>
> Please let me know if it is acceptable for mainline. Thanks.
>
> Carl
>
> -------------------------------------------------------------
>
> rs6000, add testcases to the overloaded vec_perm built-in
>
> The overloaded vec_perm built-in supports permuting signed and unsigned
> vectors of char, bool char, short int, short bool, int, bool,
> long long int, long long bool, int128, float and double. However, not all
> of the supported arguments are included in the test cases. This patch adds
> the missing test cases.
>
> Additionally, in the 128-bit debug print statements the expected result and
> the result need to be cast to unsigned long long to print correctly. The
> patch makes this additional change to the print statements.
>
> gcc/ChangeLog:
> * doc/extend.texi: Fix spelling mistake in description of the
> vec_sel built-in.
> Add documentation of the 128-bit vec_perm instance.
>
> gcc/testsuite/ChangeLog:
> * gcc.target/powerpc/vsx-builtin-3.c: Add vec_perm test cases for
> arguments of type vector signed long long int, long long bool,
> bool, bool short, bool char and pixel,
> vector unsigned long long int, unsigned int, unsigned short int,
> unsigned char.
> Cast arguments for debug prints to unsigned long long.
> * gcc.target/powerpc/builtins-4-int128-runnable.c: Add vec_perm
> test cases for signed and unsigned int128 arguments.
Nit: Some changelog lines have unnecessary newlines and spaces.
> ---
> gcc/doc/extend.texi | 12 +-
> .../powerpc/builtins-4-int128-runnable.c | 108 +++++++++++++++---
> .../gcc.target/powerpc/vsx-builtin-3.c | 18 +++
> 3 files changed, 121 insertions(+), 17 deletions(-)
>
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 48b27ff9f39..bf6f4094040 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -21553,9 +21553,19 @@ 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}
> +The instance is an extension of the existing overloaded built-in
> @code{vec_sel}
> that is documented in the PVIPR.
Good catch!
>
> +@smallexample
> +vector signed __int128 vec_perm (vector signed __int128,
> + vector signed __int128);
> +vector unsigned __int128 vec_perm (vector unsigned __int128,
> + vector unsigned __int128);
> +@end smallexample
> +
> +The 128-bit integer arguments for the @code{vec_perm} built-in are in
> addition
> +to the instances that are documented in the PVIPR.
Nit: Maybe just copy the above wording for @code{vec_sel} but replaced with
@code{vec_perm} to keep them consistent.
> +
> @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-4-int128-runnable.c
> b/gcc/testsuite/gcc.target/powerpc/builtins-4-int128-runnable.c
> index 62c11132cf3..c61b0ecb854 100644
> --- a/gcc/testsuite/gcc.target/powerpc/builtins-4-int128-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-4-int128-runnable.c
> @@ -18,6 +18,16 @@ int main() {
> __uint128_t data_u128[100];
> __int128_t data_128[100];
>
> +#ifdef __BIG_ENDIAN__
> + vector unsigned char vuc = {0xC, 0xD, 0xE, 0xF, 0x8, 0x9, 0xA, 0xB,
> + 0x1C, 0x1D, 0x1E, 0x1F, 0x18, 0x19, 0x1A,
> 0x1B};
> +#else
> + vector unsigned char vuc = {0x4, 0x5, 0x6, 0x7, 0x0, 0x1, 0x2, 0x3,
> + 0x14, 0x15, 0x16, 0x17, 0x10, 0x11, 0x12, 0x13};
> +#endif
> +
> + vector __int128_t vec_128_arg1, vec_128_arg2;
> + vector __uint128_t vec_u128_arg1, vec_u128_arg2;
> vector __int128_t vec_128_expected1, vec_128_result1;
> vector __uint128_t vec_u128_expected1, vec_u128_result1;
> signed long long zero = (signed long long) 0;
> @@ -37,11 +47,13 @@ int main() {
> {
> #ifdef DEBUG
> printf("Error: vec_xl(), vec_128_result1[0] = %lld %llu; ",
> - vec_128_result1[0] >> 64,
> - vec_128_result1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_128_result1[0] >> 64),
> + (unsigned long long)(vec_128_result1[0]
> + & (__int128_t)0xFFFFFFFFFFFFFFFF));
> printf("vec_128_expected1[0] = %lld %llu\n",
> - vec_128_expected1[0] >> 64,
> - vec_128_expected1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_128_expected1[0] >> 64),
> + (unsigned long long)(vec_128_expected1[0]
> + & (__int128_t)0xFFFFFFFFFFFFFFFF));
> #else
> abort ();
> #endif
> @@ -53,11 +65,13 @@ int main() {
> {
> #ifdef DEBUG
> printf("Error: vec_xl(), vec_u128_result1[0] = %lld; ",
> - vec_u128_result1[0] >> 64,
> - vec_u128_result1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_u128_result1[0] >> 64),
> + (unsigned long long)(vec_u128_result1[0]
> + & (__int128_t)0xFFFFFFFFFFFFFFFF));
> printf("vec_u128_expected1[0] = %lld\n",
> - vec_u128_expected1[0] >> 64,
> - vec_u128_expected1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_u128_expected1[0] >> 64),
> + (unsigned long long)(vec_u128_expected1[0]
> + & (__int128_t)0xFFFFFFFFFFFFFFFF));
> #else
> abort ();
> #endif
> @@ -76,11 +90,12 @@ int main() {
> {
> #ifdef DEBUG
> printf("Error: vec_xl_be(), vec_128_result1[0] = %llu %llu;",
> - vec_128_result1[0] >> 64,
> - vec_128_result1[0] & 0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_128_result1[0] >> 64),
> + (unsigned long long)(vec_128_result1[0] & 0xFFFFFFFFFFFFFFFF));
> printf(" vec_128_expected1[0] = %llu %llu\n",
> - vec_128_expected1[0] >> 64,
> - vec_128_expected1[0] & 0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_128_expected1[0] >> 64),
> + (unsigned long long)(vec_128_expected1[0]
> + & 0xFFFFFFFFFFFFFFFF));
> #else
> abort ();
> #endif
> @@ -98,11 +113,72 @@ int main() {
> {
> #ifdef DEBUG
> printf("Error: vec_xl_be(), vec_u128_result1[0] = %llu %llu;",
> - vec_u128_result1[0] >> 64,
> - vec_u128_result1[0] & 0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_u128_result1[0] >> 64),
> + (unsigned long long)(vec_u128_result1[0] & 0xFFFFFFFFFFFFFFFF));
> + printf(" vec_u128_expected1[0] = %llu %llu\n",
> + (unsigned long long)(vec_u128_expected1[0] >> 64),
> + (unsigned long long)(vec_u128_expected1[0]
> + & 0xFFFFFFFFFFFFFFFF));
> +#else
> + abort ();
> +#endif
> + }
> +
> + /* vec_perm() tests */
> + vec_128_arg1 = (vector __int128_t){ (__uint128_t)0x1122334455667788ULL };
> + vec_128_arg2 = (vector __int128_t){ (__uint128_t)0xAAABBBCCCDDDEEEF };
> +
> +#ifdef __BIG_ENDIAN__
> + vec_128_expected1[0] = 0x5566778811223344ULL;
> + vec_128_expected1[0] = (vec_128_expected1[0] << 64) |
> + 0xcdddeeefaaabbbccULL;
> +#else
> + vec_128_expected1[0] = 0xcdddeeefaaabbbccULL;
> + vec_128_expected1[0] = (vec_128_expected1[0] << 64) |
> + 0x5566778811223344ULL;
> +#endif
> +
> + vec_128_result1 = vec_perm (vec_128_arg1, vec_128_arg2, vuc);
> +
> + if (vec_128_expected1[0] != vec_128_result1[0])
> + {
> +#ifdef DEBUG
> + printf("Error: vec_perm(), vec_128_result1[0] = %llu %llu;",
> + (unsigned long long)(vec_128_result1[0] >> 64),
> + (unsigned long long)(vec_128_result1[0] & 0xFFFFFFFFFFFFFFFF));
> + printf(" vec_128_expected1[0] = %llu %llu\n",
> + (unsigned long long)(vec_128_expected1[0] >> 64),
> + (unsigned long long)(vec_128_expected1[0]
> + & 0xFFFFFFFFFFFFFFFF));
> +#else
> + abort ();
> +#endif
> + }
> + vec_u128_arg1 = (vector __uint128_t){ (__uint128_t)0x1122334455667788ULL };
> + vec_u128_arg2 = (vector __uint128_t){ (__uint128_t)0xAAABBBCCCDDDEEEF };
> +
> +#ifdef __BIG_ENDIAN__
> + vec_u128_expected1[0] = 0x5566778811223344ULL;
> + vec_u128_expected1[0] = (vec_u128_expected1[0] << 64) |
> + 0xcdddeeefaaabbbccULL;
> +#else
> + vec_u128_expected1[0] = 0xcdddeeefaaabbbccULL;
> + vec_u128_expected1[0] = (vec_u128_expected1[0] << 64) |
> + 0x5566778811223344ULL;
> +#endif
> +
> + vec_u128_result1 = vec_perm (vec_u128_arg1, vec_u128_arg2, vuc);
> +
> + if (vec_u128_expected1[0] != vec_u128_result1[0])
> + {
> +#ifdef DEBUG
> + printf("Error: vec_perm(), vec_u128_result1[0] = %llu %llu;",
> + (unsigned long long)(vec_u128_result1[0] >> 64),
> + (unsigned long long)(vec_u128_result1[0] & 0xFFFFFFFFFFFFFFFF));
> printf(" vec_u128_expected1[0] = %llu %llu\n",
> - vec_u128_expected1[0] >> 64,
> - vec_u128_expected1[0] & 0xFFFFFFFFFFFFFFFF);
> + (unsigned long long)(vec_u128_expected1[0] >> 64),
> + (unsigned long long)(vec_u128_expected1[0]
> + & 0xFFFFFFFFFFFFFFFF));
> #else
> abort ();
> #endif
> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> index 67c93be1469..b3b76be34b9 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
> @@ -39,10 +39,17 @@
>
> #include <altivec.h>
>
> +extern __vector long long int sll[][4];
There is a "extern __vector long long sll[][4]" below.
> +extern __vector long long bool bll[][4];
> extern __vector int si[][4];
> +extern __vector bool int bi[][4];
Similar, having "... __vector __bool int bi[][4]" below.
> extern __vector short ss[][4];
> +extern __vector bool short bs[][4];
Similar, having "... __vector __bool short bs[][4]" below.
> extern __vector signed char sc[][4];
> +extern __vector bool char bc[][4];
Ditto.
> +extern __vector pixel p[][4];
Similar, having "... __vector __pixel p[][4]" below.
> extern __vector float f[][4];
> +extern __vector unsigned long long int ull[][4];
As above, I think we only need "bll" and "ull" here.
BR,
Kewen
> extern __vector unsigned int ui[][4];
> extern __vector unsigned short us[][4];
> extern __vector unsigned char uc[][4];
> @@ -88,12 +95,23 @@ int do_perm(void)
> {
> int i = 0;
>
> + sll[i][0] = vec_perm (sll[i][1], sll[i][2], uc[i][3]); i++;
> + bll[i][0] = vec_perm (bll[i][1], bll[i][2], uc[i][3]); i++;
> si[i][0] = vec_perm (si[i][1], si[i][2], uc[i][3]); i++;
> + bi[i][0] = vec_perm (bi[i][1], bi[i][2], uc[i][3]); i++;
> ss[i][0] = vec_perm (ss[i][1], ss[i][2], uc[i][3]); i++;
> + bs[i][0] = vec_perm (bs[i][1], bs[i][2], uc[i][3]); i++;
> sc[i][0] = vec_perm (sc[i][1], sc[i][2], uc[i][3]); i++;
> + bc[i][0] = vec_perm (bc[i][1], bc[i][2], uc[i][3]); i++;
> + p[i][0] = vec_perm (p[i][1], p[i][2], uc[i][3]); i++;
> f[i][0] = vec_perm (f[i][1], f[i][2], uc[i][3]); i++;
> d[i][0] = vec_perm (d[i][1], d[i][2], uc[i][3]); i++;
>
> + ull[i][0] = vec_perm (ull[i][1], ull[i][2], uc[i][3]); i++;
> + ui[i][0] = vec_perm (ui[i][1], ui[i][2], uc[i][3]); i++;
> + us[i][0] = vec_perm (us[i][1], us[i][2], uc[i][3]); i++;
> + uc[i][0] = vec_perm (uc[i][1], uc[i][2], uc[i][3]); i++;
> +
> si[i][0] = vec_perm (si[i][1], si[i][2], uc[i][3]); i++;
> ss[i][0] = vec_perm (ss[i][1], ss[i][2], uc[i][3]); i++;
> sc[i][0] = vec_perm (sc[i][1], sc[i][2], uc[i][3]); i++;
