rs6000: Fix arguments for __builtin_altivec_tr_stxvrwx, __builtin_altivec_tr_stxvrhx
Checks
Commit Message
Kewen, Segher, Peter:
The following patch is a redo of the previous "rs6000: Fix
__builtin_vec_xst_trunc definition" patch.
This patch fixes the argument in the two builtin definitions
__builtin_altivec_tr_stxvrwx and __builtin_altivec_tr_stxvrhx. It also
adds with a testcase to validate the related builtins which have the
third argument of char *, short *, int * and long long *.
I have tested the patch on Power 10 with no regressions.
Please let me know if this patch is acceptable for mainline.
Carl
------------------------------------------------------------
rs6000: Fix arguments for __builtin_altivec_tr_stxvrwx, __builtin_altivec_tr_stxvrhx
The third argument for __builtin_altivec_tr_stxvrhx should be short *
not int *. Similarly, the third argument for __builtin_altivec_tr_stxvrwx
should be int * not short *. This patch fixes the arguments in the two
builtins.
A runnable test case is added to test the __builtin_altivec_tr_stxvrbx,
__builtin_altivec_tr_stxvrhx, __builtin_altivec_tr_stxvrwx and
__builtin_altivec_tr_stxvrdx builtins.
gcc/
* config/rs6000/rs6000-builtins.def (__builtin_altivec_tr_stxvrhx,
__builtin_altivec_tr_stxvrwx): Fix type of third argument.
gcc/testsuite/
* gcc.target/powerpc/builtin_altivec_tr_stxvr_runnable.c: New test
for __builtin_altivec_tr_stxvrbx, __builtin_altivec_tr_stxvrhx,
__builtin_altivec_tr_stxvrwx, __builtin_altivec_tr_stxvrdx.
---
gcc/config/rs6000/rs6000-builtins.def | 4 +-
.../builtin_altivec_tr_stxvr_runnable.c | 107 ++++++++++++++++++
2 files changed, 109 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin_altivec_tr_stxvr_runnable.c
Comments
Hi Carl,
on 2023/6/2 04:01, Carl Love wrote:
> Kewen, Segher, Peter:
>
> The following patch is a redo of the previous "rs6000: Fix
> __builtin_vec_xst_trunc definition" patch.
>
> This patch fixes the argument in the two builtin definitions
> __builtin_altivec_tr_stxvrwx and __builtin_altivec_tr_stxvrhx. It also
> adds with a testcase to validate the related builtins which have the
> third argument of char *, short *, int * and long long *.
>
> I have tested the patch on Power 10 with no regressions.
>
> Please let me know if this patch is acceptable for mainline.
Thanks for catching and fixing this.
OK for trunk with or without some nits below in test case fixed
(as it's just a test case :)).
>
> Carl
>
> ------------------------------------------------------------
> rs6000: Fix arguments for __builtin_altivec_tr_stxvrwx, __builtin_altivec_tr_stxvrhx
>
> The third argument for __builtin_altivec_tr_stxvrhx should be short *
> not int *. Similarly, the third argument for __builtin_altivec_tr_stxvrwx
> should be int * not short *. This patch fixes the arguments in the two
> builtins.
>
> A runnable test case is added to test the __builtin_altivec_tr_stxvrbx,
> __builtin_altivec_tr_stxvrhx, __builtin_altivec_tr_stxvrwx and
> __builtin_altivec_tr_stxvrdx builtins.
>
> gcc/
> * config/rs6000/rs6000-builtins.def (__builtin_altivec_tr_stxvrhx,
> __builtin_altivec_tr_stxvrwx): Fix type of third argument.
>
> gcc/testsuite/
> * gcc.target/powerpc/builtin_altivec_tr_stxvr_runnable.c: New test
> for __builtin_altivec_tr_stxvrbx, __builtin_altivec_tr_stxvrhx,
> __builtin_altivec_tr_stxvrwx, __builtin_altivec_tr_stxvrdx.
> ---
> gcc/config/rs6000/rs6000-builtins.def | 4 +-
> .../builtin_altivec_tr_stxvr_runnable.c | 107 ++++++++++++++++++
> 2 files changed, 109 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin_altivec_tr_stxvr_runnable.c
>
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 638d0bc72ca..d7839f2e06b 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -3161,10 +3161,10 @@
> void __builtin_altivec_tr_stxvrbx (vsq, signed long, signed char *);
> TR_STXVRBX vsx_stxvrbx {stvec}
>
> - void __builtin_altivec_tr_stxvrhx (vsq, signed long, signed int *);
> + void __builtin_altivec_tr_stxvrhx (vsq, signed long, signed short *);
> TR_STXVRHX vsx_stxvrhx {stvec}
>
> - void __builtin_altivec_tr_stxvrwx (vsq, signed long, signed short *);
> + void __builtin_altivec_tr_stxvrwx (vsq, signed long, signed int *);
> TR_STXVRWX vsx_stxvrwx {stvec}
>
> void __builtin_altivec_tr_stxvrdx (vsq, signed long, signed long long *);
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtin_altivec_tr_stxvr_runnable.c b/gcc/testsuite/gcc.target/powerpc/builtin_altivec_tr_stxvr_runnable.c
> new file mode 100644
> index 00000000000..46014d83535
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtin_altivec_tr_stxvr_runnable.c
> @@ -0,0 +1,107 @@
> +/* Test of __builtin_vec_xst_trunc */
> +
> +/* { dg-do run { target power10_hw } } */
> +/* { dg-require-effective-target int128 } */
> +/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
> +
> +#include <altivec.h>
> +#include <stdio.h>
> +#include <inttypes.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +#define DEBUG 0
> +
> +vector signed __int128 store_data =
> + { (__int128) 0x8ACE000000000000 << 64 | (__int128) 0xfedcba9876543217ULL};
> +
> +union conv_t {
> + vector signed __int128 vsi128;
> + unsigned long long ull[2];
> +} conv;
> +
> +void abort (void);
> +
> +
> +int
> +main () {
> + int i;
> + signed long sl;
> + signed char sc, expected_sc;
> + signed short ss, expected_ss;
> + signed int si, expected_si;
> + signed long long int sll, expected_sll;
> + signed char *psc;
> + signed short *pss;
> + signed int *psi;
> + signed long long int *psll;
> +
> +#if DEBUG
> + val.vsi128 = store_data;
> + printf("Data to store [%d] = 0x%llx %llx\n", i, val.ull[1], val.ull[0]);
odd indent.
> +#endif
> +
> + psc = ≻
> + pss = &ss;
> + psi = &si;
> + psll = &sll;
> +
> + sl = 1;
> + sc =0xA1;
one more space after "=".
> + expected_sc = 0xA1;
> + __builtin_altivec_tr_stxvrbx (store_data, sl, psc);
> +
> + if (expected_sc != sc & 0xFF)
> +#if DEBUG
> + printf(" ERROR: Signed char = 0x%x doesn't match expected value 0x%x\n",
> + sc & 0xFF, expected_sc);
> +#else
> + abort();
> +#endif
> +
> + sl = 1;
redundant, and easy to cause misunderstanding that sl can get changed.
> + ss = 0x52;
> + expected_ss = 0x1752;
> + __builtin_altivec_tr_stxvrhx (store_data, sl, pss);
> +
> + if (expected_ss != ss & 0xFFFF)
> +#if DEBUG
> + printf(" ERROR: Signed short = 0x%x doesn't match expected value 0x%x\n",
> + ss, expected_ss) & 0xFFFF;
> +#else
> + abort();
> +#endif
> +
> + sl = 1;
same as above.
> + si = 0x21;
> + expected_si = 0x54321721;
> + __builtin_altivec_tr_stxvrwx (store_data, sl, psi);
odd indent.
> +
> + if (expected_si != si)
> +#if DEBUG
> + printf(" ERROR: Signed int = 0x%x doesn't match expected value 0x%x\n",
> + si, expected_si);
> +#else
> + abort();
> +#endif
> +
> + sl = 1;
same as above.
> + sll = 0x12FFULL;
> + expected_sll = 0xdcba9876543217FF;
> + __builtin_altivec_tr_stxvrdx (store_data, sl, psll);
Ditto.
BR,
Kewen
> +
> + if (expected_sll != sll)
> +#if DEBUG
> + printf(" ERROR: Signed long long int = 0x%llx doesn't match expected value 0x%llx\n",
> + sll, expected_sll);
> +#else
> + abort();
> +#endif
> +
> + return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mstxvrbx\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mstxvrhx\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mstxvrwx\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mstxvrdx\M} 1 } } */
@@ -3161,10 +3161,10 @@
void __builtin_altivec_tr_stxvrbx (vsq, signed long, signed char *);
TR_STXVRBX vsx_stxvrbx {stvec}
- void __builtin_altivec_tr_stxvrhx (vsq, signed long, signed int *);
+ void __builtin_altivec_tr_stxvrhx (vsq, signed long, signed short *);
TR_STXVRHX vsx_stxvrhx {stvec}
- void __builtin_altivec_tr_stxvrwx (vsq, signed long, signed short *);
+ void __builtin_altivec_tr_stxvrwx (vsq, signed long, signed int *);
TR_STXVRWX vsx_stxvrwx {stvec}
void __builtin_altivec_tr_stxvrdx (vsq, signed long, signed long long *);
new file mode 100644
@@ -0,0 +1,107 @@
+/* Test of __builtin_vec_xst_trunc */
+
+/* { dg-do run { target power10_hw } } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
+
+#include <altivec.h>
+#include <stdio.h>
+#include <inttypes.h>
+#include <string.h>
+#include <stdlib.h>
+
+#define DEBUG 0
+
+vector signed __int128 store_data =
+ { (__int128) 0x8ACE000000000000 << 64 | (__int128) 0xfedcba9876543217ULL};
+
+union conv_t {
+ vector signed __int128 vsi128;
+ unsigned long long ull[2];
+} conv;
+
+void abort (void);
+
+
+int
+main () {
+ int i;
+ signed long sl;
+ signed char sc, expected_sc;
+ signed short ss, expected_ss;
+ signed int si, expected_si;
+ signed long long int sll, expected_sll;
+ signed char *psc;
+ signed short *pss;
+ signed int *psi;
+ signed long long int *psll;
+
+#if DEBUG
+ val.vsi128 = store_data;
+ printf("Data to store [%d] = 0x%llx %llx\n", i, val.ull[1], val.ull[0]);
+#endif
+
+ psc = ≻
+ pss = &ss;
+ psi = &si;
+ psll = &sll;
+
+ sl = 1;
+ sc =0xA1;
+ expected_sc = 0xA1;
+ __builtin_altivec_tr_stxvrbx (store_data, sl, psc);
+
+ if (expected_sc != sc & 0xFF)
+#if DEBUG
+ printf(" ERROR: Signed char = 0x%x doesn't match expected value 0x%x\n",
+ sc & 0xFF, expected_sc);
+#else
+ abort();
+#endif
+
+ sl = 1;
+ ss = 0x52;
+ expected_ss = 0x1752;
+ __builtin_altivec_tr_stxvrhx (store_data, sl, pss);
+
+ if (expected_ss != ss & 0xFFFF)
+#if DEBUG
+ printf(" ERROR: Signed short = 0x%x doesn't match expected value 0x%x\n",
+ ss, expected_ss) & 0xFFFF;
+#else
+ abort();
+#endif
+
+ sl = 1;
+ si = 0x21;
+ expected_si = 0x54321721;
+ __builtin_altivec_tr_stxvrwx (store_data, sl, psi);
+
+ if (expected_si != si)
+#if DEBUG
+ printf(" ERROR: Signed int = 0x%x doesn't match expected value 0x%x\n",
+ si, expected_si);
+#else
+ abort();
+#endif
+
+ sl = 1;
+ sll = 0x12FFULL;
+ expected_sll = 0xdcba9876543217FF;
+ __builtin_altivec_tr_stxvrdx (store_data, sl, psll);
+
+ if (expected_sll != sll)
+#if DEBUG
+ printf(" ERROR: Signed long long int = 0x%llx doesn't match expected value 0x%llx\n",
+ sll, expected_sll);
+#else
+ abort();
+#endif
+
+ return 0;
+}
+
+/* { dg-final { scan-assembler-times {\mstxvrbx\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mstxvrhx\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mstxvrwx\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mstxvrdx\M} 1 } } */