Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
Hi!
On Tue, Jun 30, 2020 at 01:58:50AM -0400, Michael Meissner wrote:
> On Mon, Jun 29, 2020 at 01:42:56PM -0500, Segher Boessenkool wrote:
> > On Mon, Jun 29, 2020 at 02:23:22PM -0400, Michael Meissner wrote:
> > > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> > > +
> > > +/* Test that PLI (PADDI) is generated to load a large constant. */
> > > +unsigned long long
> > > +large (void)
> > > +{
> > > + return 0x12345678ULL;
> > > +}
> > > +
> > > +/* { dg-final { scan-assembler {\mpli\M} } } */
> >
> > I have no idea why 64-bit mode (or 64-bit addressing) is needed here.
> > *Is* it needed?
>
> Yes it is needed. Otherwise two separate load immediates would be needed to
> load each part of the DI constant that is held in 2 registers.
But that will work just fine, because one of those insns will be a pli,
exactly as tested for!
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/powerpc/prefix-no-update.c
> > > @@ -0,0 +1,51 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target powerpc_prefixed_addr } */
> > > +/* { dg-require-effective-target lp64 } */
> > > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> >
> > For this testcase, I have no idea at all why you want lp64?
>
> Becuase to show the bug you need a stack frame larger than 64K.
I don't see it? (Also, why would that require lp64?)
Segher
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Mon, Jun 29, 2020 at 01:42:56PM -0500, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Jun 29, 2020 at 02:23:22PM -0400, Michael Meissner wrote:
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/prefix-di-constant.c
> > @@ -0,0 +1,13 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_prefixed_addr } */
> > +/* { dg-require-effective-target lp64 } */
>
> Please always say (_in the test_) why something is required, if it isn't
> obvious.
>
> > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> > +
> > +/* Test that PLI (PADDI) is generated to load a large constant. */
> > +unsigned long long
> > +large (void)
> > +{
> > + return 0x12345678ULL;
> > +}
> > +
> > +/* { dg-final { scan-assembler {\mpli\M} } } */
>
> I have no idea why 64-bit mode (or 64-bit addressing) is needed here.
> *Is* it needed?
Yes it is needed. Otherwise two separate load immediates would be needed to
load each part of the DI constant that is held in 2 registers.
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c
> > @@ -0,0 +1,161 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_prefixed_addr } */
> > +/* { dg-require-effective-target lp64 } */
> > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
>
> > + unsigned int si; /* offset 49 bytes. */
>
> > +int
> > +load_si (struct packed_struct *p)
> > +{
> > + return p->si;/* PLWA 3,49(3). */
> > +}
>
> Here it is because this would be just lwz on 32-bit.
>
> But that is the only difference, so you could just make that single test
> conditional, not the whole file.
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/prefix-no-update.c
> > @@ -0,0 +1,51 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_prefixed_addr } */
> > +/* { dg-require-effective-target lp64 } */
> > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
>
> For this testcase, I have no idea at all why you want lp64?
Becuase to show the bug you need a stack frame larger than 64K.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
Hi!
On Mon, Jun 29, 2020 at 02:23:22PM -0400, Michael Meissner wrote:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/prefix-di-constant.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_prefixed_addr } */
> +/* { dg-require-effective-target lp64 } */
Please always say (_in the test_) why something is required, if it isn't
obvious.
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> +
> +/* Test that PLI (PADDI) is generated to load a large constant. */
> +unsigned long long
> +large (void)
> +{
> + return 0x12345678ULL;
> +}
> +
> +/* { dg-final { scan-assembler {\mpli\M} } } */
I have no idea why 64-bit mode (or 64-bit addressing) is needed here.
*Is* it needed?
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c
> @@ -0,0 +1,161 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_prefixed_addr } */
> +/* { dg-require-effective-target lp64 } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> + unsigned int si; /* offset 49 bytes. */
> +int
> +load_si (struct packed_struct *p)
> +{
> + return p->si; /* PLWA 3,49(3). */
> +}
Here it is because this would be just lwz on 32-bit.
But that is the only difference, so you could just make that single test
conditional, not the whole file.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/prefix-no-update.c
> @@ -0,0 +1,51 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_prefixed_addr } */
> +/* { dg-require-effective-target lp64 } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
For this testcase, I have no idea at all why you want lp64?
Thanks,
Segher
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
>From 212475e5757fe3335cba30c9c3eec1707ac0c271 Mon Sep 17 00:00:00 2001 From: Michael Meissner Date: Sat, 27 Jun 2020 00:40:48 -0500 Subject: [PATCH, committed] Add PowerPC tests for power10. 2020-06-27 Michael Meissner * gcc.target/powerpc/prefix-add.c: New test. * gcc.target/powerpc/prefix-si-constant.c: New test. * gcc.target/powerpc/prefix-di-constant.c: New test. * gcc.target/powerpc/prefix-ds-dq.c: New test. * gcc.target/powerpc/prefix-no-update.c: New test. * gcc.target/powerpc/prefix-large-dd.c: New test. * gcc.target/powerpc/prefix-large-df.c: New test. * gcc.target/powerpc/prefix-large-di.c: New test. * gcc.target/powerpc/prefix-large-hi.c: New test. * gcc.target/powerpc/prefix-large-kf.c: New test. * gcc.target/powerpc/prefix-large-qi.c: New test. * gcc.target/powerpc/prefix-large-sd.c: New test. * gcc.target/powerpc/prefix-large-sf.c: New test. * gcc.target/powerpc/prefix-large-si.c: New test. * gcc.target/powerpc/prefix-large-udi.c: New test. * gcc.target/powerpc/prefix-large-uhi.c: New test. * gcc.target/powerpc/prefix-large-uqi.c: New test. * gcc.target/powerpc/prefix-large-usi.c: New test. * gcc.target/powerpc/prefix-large-v2df.c: New test. * gcc.target/powerpc/prefix-large.h: Include file for new tests. * gcc.target/powerpc/prefix-pcrel-dd.c: New test. * gcc.target/powerpc/prefix-pcrel-df.c: New test. * gcc.target/powerpc/prefix-pcrel-di.c: New test. * gcc.target/powerpc/prefix-pcrel-hi.c: New test. * gcc.target/powerpc/prefix-pcrel-kf.c: New test. * gcc.target/powerpc/prefix-pcrel-qi.c: New test. * gcc.target/powerpc/prefix-pcrel-sd.c: New test. * gcc.target/powerpc/prefix-pcrel-sf.c: New test. * gcc.target/powerpc/prefix-pcrel-si.c: New test. * gcc.target/powerpc/prefix-pcrel-udi.c: New test. * gcc.target/powerpc/prefix-pcrel-uhi.c: New test. * gcc.target/powerpc/prefix-pcrel-uqi.c: New test. * gcc.target/powerpc/prefix-pcrel-usi.c: New test. * gcc.target/powerpc/prefix-pcrel-v2df.c: New test. * gcc.target/powerpc/prefix-pcrel.h: Include file for new tests. * gcc.target/powerpc/prefix-stack-protect.c: New test. --- gcc/testsuite/gcc.target/powerpc/prefix-add.c | 14 ++ .../gcc.target/powerpc/prefix-di-constant.c | 13 ++ .../gcc.target/powerpc/prefix-ds-dq.c | 161 ++ .../gcc.target/powerpc/prefix-large-dd.c | 13 ++ .../gcc.target/powerpc/prefix-large-df.c | 13 ++ .../gcc.target/powerpc/prefix-large-di.c | 14 ++ .../gcc.target/powerpc/prefix-large-hi.c | 13 ++ .../gcc.target/powerpc/prefix-large-kf.c | 13 ++ .../gcc.target/powerpc/prefix-large-qi.c | 13 ++ .../gcc.target/powerpc/prefix-large-sd.c | 19 +++ .../gcc.target/powerpc/prefix-large-sf.c | 13 ++ .../gcc.target/powerpc/prefix-large-si.c | 13 ++ .../gcc.target/powerpc/prefix-large-udi.c | 14 ++ .../gcc.target/powerpc/prefix-large-uhi.c | 13 ++ .../gcc.target/powerpc/prefix-large-uqi.c | 13 ++ .../gcc.target/powerpc/prefix-large-usi.c | 13 ++ .../gcc.target/powerpc/prefix-large-v2df.c| 13 ++ .../gcc.target/powerpc/prefix-large.h | 40 + .../gcc.target/powerpc/prefix-no-update.c | 51 ++ .../gcc.target/powerpc/prefix-pcrel-dd.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-df.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-di.c | 14 ++ .../gcc.target/powerpc/prefix-pcrel-hi.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-kf.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-qi.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-sd.c | 15 ++ .../gcc.target/powerpc/prefix-pcrel-sf.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-si.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-udi.c | 14 ++ .../gcc.target/powerpc/prefix-pcrel-uhi.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-uqi.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-usi.c | 13 ++ .../gcc.target/powerpc/prefix-pcrel-v2df.c| 13 ++ .../gcc.target/powerpc/prefix-pcrel.h | 41 + .../gcc.target/powerpc/prefix-si-constant.c | 12 ++ .../gcc.target/powerpc/prefix-stack-protect.c | 21 +++ 36 files changed, 729 insertions(+) create mode 100644 gcc/testsuite/gcc.target/powerpc/prefix-add.c create mode 100644 gcc/testsuite/gcc.target/powerpc/prefix-di-constant.c create mode 100644 gcc/testsuite/gcc.target/powerpc/prefix-ds-dq.c create mode 100644 gcc/testsuite/gcc.target/powerpc/prefix-large-dd.c create mode 100644 gcc/testsuite/gcc.target/powerpc/prefix-large-df.c create mode 100644 gcc/testsuite/gcc.target/powerpc/prefix-large-di.c create mode 100644 gcc/testsuite/gcc.target/powerpc/prefix-large-hi.c create mode 100644
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Sat, Jun 27, 2020 at 01:57:52AM -0400, Michael Meissner wrote: > On Thu, Jun 25, 2020 at 11:52:50AM -0500, Segher Boessenkool wrote: > > On Mon, Jun 01, 2020 at 03:53:37PM -0400, Michael Meissner wrote: > > > Add tests for -mcpu=future that test the generation of PADDI (and PLI > > > which > > > becomes PADDI). > > > > > > 2020-06-01 Michael Meissner > > > > > > * gcc.target/powerpc/prefix-add.c: New test. > > > * gcc.target/powerpc/prefix-si-constant.c: New test. > > > * gcc.target/powerpc/prefix-di-constant.c: New test. > > > > This is okay for trunk (with required changes: -mdejagnu-cpu=power10, > > and the selector names have changed to be something with power10 instead > > of something with future; please retest before commit). > > Since all of these tests are for code that is in GCC 10, can I apply these > patches to the GCC 10 branch after the completion of the -mcpu=power10 changes > for GCC 10 and a suitable waiting period and retest? Sure, thanks! Segher
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Sat, Jun 27, 2020 at 01:49:23AM -0400, Michael Meissner wrote: > On Thu, Jun 25, 2020 at 11:52:50AM -0500, Segher Boessenkool wrote: > > On Mon, Jun 01, 2020 at 03:53:37PM -0400, Michael Meissner wrote: > > > Add tests for -mcpu=future that test the generation of PADDI (and PLI > > > which > > > becomes PADDI). > > > > > > 2020-06-01 Michael Meissner > > > > > > * gcc.target/powerpc/prefix-add.c: New test. > > > * gcc.target/powerpc/prefix-si-constant.c: New test. > > > * gcc.target/powerpc/prefix-di-constant.c: New test. > > > > This is okay for trunk (with required changes: -mdejagnu-cpu=power10, > > and the selector names have changed to be something with power10 instead > > of something with future; please retest before commit). > > Done. I was just about to resubmit the patches with the -mcpu=power10 > changes. > I did retest on power8/power9 little endian, and power8 big endian (both > 32/64-bit). In running the tests, I discovered 3 of the tests that needed > ILP64 to test the particular code that I was looking for that I previously > hadn't flagged. Please post what you committed, then? Segher
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Thu, Jun 25, 2020 at 11:52:50AM -0500, Segher Boessenkool wrote: > On Mon, Jun 01, 2020 at 03:53:37PM -0400, Michael Meissner wrote: > > Add tests for -mcpu=future that test the generation of PADDI (and PLI which > > becomes PADDI). > > > > 2020-06-01 Michael Meissner > > > > * gcc.target/powerpc/prefix-add.c: New test. > > * gcc.target/powerpc/prefix-si-constant.c: New test. > > * gcc.target/powerpc/prefix-di-constant.c: New test. > > This is okay for trunk (with required changes: -mdejagnu-cpu=power10, > and the selector names have changed to be something with power10 instead > of something with future; please retest before commit). Since all of these tests are for code that is in GCC 10, can I apply these patches to the GCC 10 branch after the completion of the -mcpu=power10 changes for GCC 10 and a suitable waiting period and retest? -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Thu, Jun 25, 2020 at 11:52:50AM -0500, Segher Boessenkool wrote: > On Mon, Jun 01, 2020 at 03:53:37PM -0400, Michael Meissner wrote: > > Add tests for -mcpu=future that test the generation of PADDI (and PLI which > > becomes PADDI). > > > > 2020-06-01 Michael Meissner > > > > * gcc.target/powerpc/prefix-add.c: New test. > > * gcc.target/powerpc/prefix-si-constant.c: New test. > > * gcc.target/powerpc/prefix-di-constant.c: New test. > > This is okay for trunk (with required changes: -mdejagnu-cpu=power10, > and the selector names have changed to be something with power10 instead > of something with future; please retest before commit). Done. I was just about to resubmit the patches with the -mcpu=power10 changes. I did retest on power8/power9 little endian, and power8 big endian (both 32/64-bit). In running the tests, I discovered 3 of the tests that needed ILP64 to test the particular code that I was looking for that I previously hadn't flagged. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Mon, Jun 01, 2020 at 03:53:37PM -0400, Michael Meissner wrote: > Add tests for -mcpu=future that test the generation of PADDI (and PLI which > becomes PADDI). > > 2020-06-01 Michael Meissner > > * gcc.target/powerpc/prefix-add.c: New test. > * gcc.target/powerpc/prefix-si-constant.c: New test. > * gcc.target/powerpc/prefix-di-constant.c: New test. This is okay for trunk (with required changes: -mdejagnu-cpu=power10, and the selector names have changed to be something with power10 instead of something with future; please retest before commit). Thanks! Segher
