| Message ID | YSa/qDv2385Qt6YO@toto.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix float128-call.c test for power8 IEEE 128 and power10. | expand |
Hi Mike, Thanks for this clean-up! On 8/25/21 5:09 PM, Michael Meissner wrote: > From 327273dfeec5c000f3c33ca7b88ee0097fd33586 Mon Sep 17 00:00:00 2001 > From: Michael Meissner <meissner@linux.ibm.com> > Date: Wed, 25 Aug 2021 00:31:35 -0400 > Subject: [PATCH] Fix float128-call.c test for power8 IEEE 128 and power10. > > I built a compiler on a little endian power8 system where the default long > double was IEEE 128-bit instead of IBM 128-bit. I discovered that on > power8, we would generate a lxvd2x and xxpermdi to deal with the endianess > instead of the Altivec lxv. > > In addition, I noticed the constant that was being loaded (1.0q) could be > loaded by the lxvkq instruction. > > I rewrote the test to handle all forms of vector load and store that can > be generated. And I changed the constant to be one that lxvkq does not > support. > > I did bootstrap tests on the following systems, and the the test ran in all > environments (each of the systems were configured for the cpu mentioned): > > 1) Little endian power9 with IBM 128-bit long double > 2) Little endian power9 with IEEE 128-bit long double > 3) Little endian power8 with IBM 128-bit long double > 4) Little endian power8 with IEEE 128-bit long double > 5) Little endian power10 with IBM 128-bit long double > 6) Little endian power10 with IEEE 128-bit long double > 7) Big endian power8 with IBM 128-bit long double > > Can I check this patch into the master branch and later backport it to GCC-11? > > 2021-08-25 Michael Meissner <meissner@linux.ibm.com> > > gcc/testsuite/ > * gcc.target/powerpc/float128-call.c: Fix test for IEEE 128-bit > long double and power10. > --- > .../gcc.target/powerpc/float128-call.c | 27 +++++++++++++------ > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/gcc/testsuite/gcc.target/powerpc/float128-call.c b/gcc/testsuite/gcc.target/powerpc/float128-call.c > index b64ffc68bfa..d1cf47e4298 100644 > --- a/gcc/testsuite/gcc.target/powerpc/float128-call.c > +++ b/gcc/testsuite/gcc.target/powerpc/float128-call.c > @@ -6,22 +6,33 @@ > #error "-mfloat128 is not supported." > #endif > > +/* Pick a constant to load that cannot be generated by the power10 lxvkq > + instruction. */ > #ifdef __LONG_DOUBLE_IEEE128__ > #define TYPE long double > -#define ONE 1.0L > +#define TEN 10.0L > > #else > #define TYPE __float128 > -#define ONE 1.0Q > +#define TEN 10.0Q > #endif > > /* Test to make sure vector registers are used for passing IEEE 128-bit > floating point values and returning them. Also make sure the 'q' suffix is > - handled. */ > -TYPE one (void) { return ONE; } > + handled for __float128. */ > +TYPE one (void) { return TEN; } This amuses me, and I want to keep it this way. :-) > void store (TYPE a, TYPE *p) { *p = a; } > > -/* { dg-final { scan-assembler {\mlxvd2x 34\M} {target be} } } */ > -/* { dg-final { scan-assembler {\mstxvd2x 34\M} {target be} } } */ > -/* { dg-final { scan-assembler {\mlvx 2\M} {target le} } } */ > -/* { dg-final { scan-assembler {\mstvx 2\M} {target le} } } */ > +/* This regexp captures the different vector load/stores that can be generated: > + > + lxvd2x -- big endian power7/power8, little endian power8 > + lvx -- Altivec > + lxv -- power9 > + plxv -- power10 > + lxvx -- X-form variant. > + stxvd2x -- big endian power7/power8, little endian power8 > + stvx -- Altivec For symmetry, also mention stxvx as an X-form variant? Looks fine to me, recommend approval. Thanks, Bill > + lxvx -- power9/power10. */ > + > +/* { dg-final { scan-assembler {\mlxvd2x 34\M|\mlvx 2\M|\mp?lxvx? 34\M} } } */ > +/* { dg-final { scan-assembler {\mstxvd2x 34\M|\mstvx 2\M|\mstxvx 34\M} } } */
This patch is okay. Thanks, David On Fri, Aug 27, 2021 at 12:41 PM Bill Schmidt <wschmidt@linux.ibm.com> wrote: > > Hi Mike, > > Thanks for this clean-up! > > On 8/25/21 5:09 PM, Michael Meissner wrote: > > From 327273dfeec5c000f3c33ca7b88ee0097fd33586 Mon Sep 17 00:00:00 2001 > > From: Michael Meissner <meissner@linux.ibm.com> > > Date: Wed, 25 Aug 2021 00:31:35 -0400 > > Subject: [PATCH] Fix float128-call.c test for power8 IEEE 128 and power10. > > > > I built a compiler on a little endian power8 system where the default long > > double was IEEE 128-bit instead of IBM 128-bit. I discovered that on > > power8, we would generate a lxvd2x and xxpermdi to deal with the endianess > > instead of the Altivec lxv. > > > > In addition, I noticed the constant that was being loaded (1.0q) could be > > loaded by the lxvkq instruction. > > > > I rewrote the test to handle all forms of vector load and store that can > > be generated. And I changed the constant to be one that lxvkq does not > > support. > > > > I did bootstrap tests on the following systems, and the the test ran in all > > environments (each of the systems were configured for the cpu mentioned): > > > > 1) Little endian power9 with IBM 128-bit long double > > 2) Little endian power9 with IEEE 128-bit long double > > 3) Little endian power8 with IBM 128-bit long double > > 4) Little endian power8 with IEEE 128-bit long double > > 5) Little endian power10 with IBM 128-bit long double > > 6) Little endian power10 with IEEE 128-bit long double > > 7) Big endian power8 with IBM 128-bit long double > > > > Can I check this patch into the master branch and later backport it to GCC-11? > > > > 2021-08-25 Michael Meissner <meissner@linux.ibm.com> > > > > gcc/testsuite/ > > * gcc.target/powerpc/float128-call.c: Fix test for IEEE 128-bit > > long double and power10. > > --- > > .../gcc.target/powerpc/float128-call.c | 27 +++++++++++++------ > > 1 file changed, 19 insertions(+), 8 deletions(-) > > > > diff --git a/gcc/testsuite/gcc.target/powerpc/float128-call.c b/gcc/testsuite/gcc.target/powerpc/float128-call.c > > index b64ffc68bfa..d1cf47e4298 100644 > > --- a/gcc/testsuite/gcc.target/powerpc/float128-call.c > > +++ b/gcc/testsuite/gcc.target/powerpc/float128-call.c > > @@ -6,22 +6,33 @@ > > #error "-mfloat128 is not supported." > > #endif > > > > +/* Pick a constant to load that cannot be generated by the power10 lxvkq > > + instruction. */ > > #ifdef __LONG_DOUBLE_IEEE128__ > > #define TYPE long double > > -#define ONE 1.0L > > +#define TEN 10.0L > > > > #else > > #define TYPE __float128 > > -#define ONE 1.0Q > > +#define TEN 10.0Q > > #endif > > > > /* Test to make sure vector registers are used for passing IEEE 128-bit > > floating point values and returning them. Also make sure the 'q' suffix is > > - handled. */ > > -TYPE one (void) { return ONE; } > > + handled for __float128. */ > > +TYPE one (void) { return TEN; } > > This amuses me, and I want to keep it this way. :-) > > void store (TYPE a, TYPE *p) { *p = a; } > > > > -/* { dg-final { scan-assembler {\mlxvd2x 34\M} {target be} } } */ > > -/* { dg-final { scan-assembler {\mstxvd2x 34\M} {target be} } } */ > > -/* { dg-final { scan-assembler {\mlvx 2\M} {target le} } } */ > > -/* { dg-final { scan-assembler {\mstvx 2\M} {target le} } } */ > > +/* This regexp captures the different vector load/stores that can be generated: > > + > > + lxvd2x -- big endian power7/power8, little endian power8 > > + lvx -- Altivec > > + lxv -- power9 > > + plxv -- power10 > > + lxvx -- X-form variant. > > + stxvd2x -- big endian power7/power8, little endian power8 > > + stvx -- Altivec > > For symmetry, also mention stxvx as an X-form variant? > > Looks fine to me, recommend approval. > > Thanks, > Bill > > > + lxvx -- power9/power10. */ > > + > > +/* { dg-final { scan-assembler {\mlxvd2x 34\M|\mlvx 2\M|\mp?lxvx? 34\M} } } */ > > +/* { dg-final { scan-assembler {\mstxvd2x 34\M|\mstvx 2\M|\mstxvx 34\M} } } */
On Fri, Aug 27, 2021 at 11:41:06AM -0500, Bill Schmidt wrote: > This amuses me, and I want to keep it this way. :-) > > void store (TYPE a, TYPE *p) { *p = a; } > > -/* { dg-final { scan-assembler {\mlxvd2x 34\M} {target be} } } */ > > -/* { dg-final { scan-assembler {\mstxvd2x 34\M} {target be} } } */ > > -/* { dg-final { scan-assembler {\mlvx 2\M} {target le} } } */ > > -/* { dg-final { scan-assembler {\mstvx 2\M} {target le} } } */ > > +/* This regexp captures the different vector load/stores that can be generated: > > + > > + lxvd2x -- big endian power7/power8, little endian power8 > > + lvx -- Altivec > > + lxv -- power9 > > + plxv -- power10 > > + lxvx -- X-form variant. > > + stxvd2x -- big endian power7/power8, little endian power8 > > + stvx -- Altivec > > For symmetry, also mention stxvx as an X-form variant? Yep, thanks. Because of the way the test is written (load a constant and store through a pointer), it wouldn't generate stxv and pstxv.
On Wed, Aug 25, 2021 at 06:09:44PM -0400, Michael Meissner wrote: > I built a compiler on a little endian power8 system where the default long > double was IEEE 128-bit instead of IBM 128-bit. I discovered that on > power8, we would generate a lxvd2x and xxpermdi to deal with the endianess > instead of the Altivec lxv. You mean lvx. Okay. > +/* { dg-final { scan-assembler {\mlxvd2x 34\M|\mlvx 2\M|\mp?lxvx? 34\M} } } */ > +/* { dg-final { scan-assembler {\mstxvd2x 34\M|\mstvx 2\M|\mstxvx 34\M} } } */ "stxvx?" as well? For robustness. Can add "p?" as well, or would it be bad if that ever is used, is this test testing it is not done? Segher
On Fri, Aug 27, 2021 at 12:29:42PM -0500, Segher Boessenkool wrote: > On Wed, Aug 25, 2021 at 06:09:44PM -0400, Michael Meissner wrote: > > I built a compiler on a little endian power8 system where the default long > > double was IEEE 128-bit instead of IBM 128-bit. I discovered that on > > power8, we would generate a lxvd2x and xxpermdi to deal with the endianess > > instead of the Altivec lxv. > > You mean lvx. Okay. > > > +/* { dg-final { scan-assembler {\mlxvd2x 34\M|\mlvx 2\M|\mp?lxvx? 34\M} } } */ > > +/* { dg-final { scan-assembler {\mstxvd2x 34\M|\mstvx 2\M|\mstxvx 34\M} } } */ > > "stxvx?" as well? For robustness. Can add "p?" as well, or would it > be bad if that ever is used, is this test testing it is not done? I updated the comments and regex. In this particular test, the d-form stores would never be generated, but it can be useful to include them.
diff --git a/gcc/testsuite/gcc.target/powerpc/float128-call.c b/gcc/testsuite/gcc.target/powerpc/float128-call.c index b64ffc68bfa..d1cf47e4298 100644 --- a/gcc/testsuite/gcc.target/powerpc/float128-call.c +++ b/gcc/testsuite/gcc.target/powerpc/float128-call.c @@ -6,22 +6,33 @@ #error "-mfloat128 is not supported." #endif +/* Pick a constant to load that cannot be generated by the power10 lxvkq + instruction. */ #ifdef __LONG_DOUBLE_IEEE128__ #define TYPE long double -#define ONE 1.0L +#define TEN 10.0L #else #define TYPE __float128 -#define ONE 1.0Q +#define TEN 10.0Q #endif /* Test to make sure vector registers are used for passing IEEE 128-bit floating point values and returning them. Also make sure the 'q' suffix is - handled. */ -TYPE one (void) { return ONE; } + handled for __float128. */ +TYPE one (void) { return TEN; } void store (TYPE a, TYPE *p) { *p = a; } -/* { dg-final { scan-assembler {\mlxvd2x 34\M} {target be} } } */ -/* { dg-final { scan-assembler {\mstxvd2x 34\M} {target be} } } */ -/* { dg-final { scan-assembler {\mlvx 2\M} {target le} } } */ -/* { dg-final { scan-assembler {\mstvx 2\M} {target le} } } */ +/* This regexp captures the different vector load/stores that can be generated: + + lxvd2x -- big endian power7/power8, little endian power8 + lvx -- Altivec + lxv -- power9 + plxv -- power10 + lxvx -- X-form variant. + stxvd2x -- big endian power7/power8, little endian power8 + stvx -- Altivec + lxvx -- power9/power10. */ + +/* { dg-final { scan-assembler {\mlxvd2x 34\M|\mlvx 2\M|\mp?lxvx? 34\M} } } */ +/* { dg-final { scan-assembler {\mstxvd2x 34\M|\mstvx 2\M|\mstxvx 34\M} } } */
From 327273dfeec5c000f3c33ca7b88ee0097fd33586 Mon Sep 17 00:00:00 2001 From: Michael Meissner <meissner@linux.ibm.com> Date: Wed, 25 Aug 2021 00:31:35 -0400 Subject: [PATCH] Fix float128-call.c test for power8 IEEE 128 and power10. I built a compiler on a little endian power8 system where the default long double was IEEE 128-bit instead of IBM 128-bit. I discovered that on power8, we would generate a lxvd2x and xxpermdi to deal with the endianess instead of the Altivec lxv. In addition, I noticed the constant that was being loaded (1.0q) could be loaded by the lxvkq instruction. I rewrote the test to handle all forms of vector load and store that can be generated. And I changed the constant to be one that lxvkq does not support. I did bootstrap tests on the following systems, and the the test ran in all environments (each of the systems were configured for the cpu mentioned): 1) Little endian power9 with IBM 128-bit long double 2) Little endian power9 with IEEE 128-bit long double 3) Little endian power8 with IBM 128-bit long double 4) Little endian power8 with IEEE 128-bit long double 5) Little endian power10 with IBM 128-bit long double 6) Little endian power10 with IEEE 128-bit long double 7) Big endian power8 with IBM 128-bit long double Can I check this patch into the master branch and later backport it to GCC-11? 2021-08-25 Michael Meissner <meissner@linux.ibm.com> gcc/testsuite/ * gcc.target/powerpc/float128-call.c: Fix test for IEEE 128-bit long double and power10. --- .../gcc.target/powerpc/float128-call.c | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-)