public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10.
@ 2021-07-14 17:52 Michael Meissner
  2021-07-14 20:25 ` Segher Boessenkool
  2021-07-14 21:22 ` Segher Boessenkool
  0 siblings, 2 replies; 6+ messages in thread
From: Michael Meissner @ 2021-07-14 17:52 UTC (permalink / raw)
  To: gcc-patches, Michael Meissner, Segher Boessenkool,
	David Edelsohn, Bill Schmidt, Peter Bergner, Will Schmidt

I forgot to add the patch when I reposted this.

PR 100170: Fix eq/ne tests on power10.

This patch updates eq/ne tests in the testsuite to adjust the test if
power10 code generation is used.

2021-07-04  Michael Meissner  <meissner@linux.ibm.com>

gcc/testsuite/
	PR testsuite/100170
	* gcc.target/powerpc/ppc-eq0-1.c: Add support for the setbc
	instruction.
	* gcc.target/powerpc/ppc-ne0-1.c: Update instruction counts on
	power10.
---
 gcc/testsuite/gcc.target/powerpc/ppc-eq0-1.c | 2 +-
 gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/ppc-eq0-1.c b/gcc/testsuite/gcc.target/powerpc/ppc-eq0-1.c
index 496a6e340c0..bbdc7e00101 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc-eq0-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-eq0-1.c
@@ -7,4 +7,4 @@ int foo(int x)
   return x == 0;
 }
 
-/* { dg-final { scan-assembler "cntlzw|isel" } } */
+/* { dg-final { scan-assembler {\mcntlzw|isel|setbc\M} } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c b/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
index 63c4b6087df..34c6de3874d 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
@@ -2,9 +2,12 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -mno-isel" } */
 
-/* { dg-final { scan-assembler-times "addic" 4 } } */
-/* { dg-final { scan-assembler-times "subfe" 1 } } */
-/* { dg-final { scan-assembler-times "addze" 3 } } */
+/* { dg-final { scan-assembler-times {\maddic\M}  4 { target { ! has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler-times {\msubfe\M}  1 { target { ! has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler-times {\maddic\M}  3 { target {   has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler-not   {\msubfe\M}    { target {   has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler-times {\msetbcr\M} 1 { target {   has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler-times {\maddze\M}  3 } } */
 
 long ne0(long a)
 {
-- 
2.31.1


-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10.
  2021-07-14 17:52 Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10 Michael Meissner
@ 2021-07-14 20:25 ` Segher Boessenkool
  2021-07-14 20:32   ` Segher Boessenkool
  2021-07-14 21:22 ` Segher Boessenkool
  1 sibling, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2021-07-14 20:25 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt,
	Peter Bergner, Will Schmidt

Please do not send the same patches in a new thread.  It is much more
work to keep track of.  Just ping patches by replying to them (either
copy the list or not, either works).  Thanks!


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10.
  2021-07-14 20:25 ` Segher Boessenkool
@ 2021-07-14 20:32   ` Segher Boessenkool
  0 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2021-07-14 20:32 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt,
	Peter Bergner, Will Schmidt

On Wed, Jul 14, 2021 at 03:25:32PM -0500, Segher Boessenkool wrote:
> Please do not send the same patches in a new thread.  It is much more
> work to keep track of.  Just ping patches by replying to them (either
> copy the list or not, either works).  Thanks!

Oh, and do not edit the Subject:.  You managed to have the first 30
characters of it completely useless.  You should never use more than 50
characters total, you use 57 already, although this should be an
unusually *short* subject!  (And subjects are not sentences, do not end
in a full stop.)


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10.
  2021-07-14 17:52 Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10 Michael Meissner
  2021-07-14 20:25 ` Segher Boessenkool
@ 2021-07-14 21:22 ` Segher Boessenkool
  2021-07-26 20:46   ` Michael Meissner
  1 sibling, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2021-07-14 21:22 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt,
	Peter Bergner, Will Schmidt

On Wed, Jul 14, 2021 at 01:52:05PM -0400, Michael Meissner wrote:
> This patch updates eq/ne tests in the testsuite to adjust the test if
> power10 code generation is used.

eq0/ne0.

> --- a/gcc/testsuite/gcc.target/powerpc/ppc-eq0-1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/ppc-eq0-1.c

> -/* { dg-final { scan-assembler "cntlzw|isel" } } */
> +/* { dg-final { scan-assembler {\mcntlzw|isel|setbc\M} } } */

This does not do wha you perhaps think it does.  It looks for one of the
three atoms
"\mcntlzw", "isel", or "setbc\M".  You should write
  \m(cntlzw|isel|setbc)\M
or, if you need it to not capture (like in a scan-assembler-times)
  \m(?:cntlzw|isel|setbc)\M

> --- a/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c

> -/* { dg-final { scan-assembler-times "addic" 4 } } */
> -/* { dg-final { scan-assembler-times "subfe" 1 } } */
> -/* { dg-final { scan-assembler-times "addze" 3 } } */
> +/* { dg-final { scan-assembler-times {\maddic\M}  4 { target { ! has_arch_pwr10 } } } } */
> +/* { dg-final { scan-assembler-times {\msubfe\M}  1 { target { ! has_arch_pwr10 } } } } */
> +/* { dg-final { scan-assembler-times {\maddic\M}  3 { target {   has_arch_pwr10 } } } } */
> +/* { dg-final { scan-assembler-not   {\msubfe\M}    { target {   has_arch_pwr10 } } } } */
> +/* { dg-final { scan-assembler-times {\msetbcr\M} 1 { target {   has_arch_pwr10 } } } } */
> +/* { dg-final { scan-assembler-times {\maddze\M}  3 } } */

It may be easier to split the patch into two, where one part can get the
setbcr (the first, simplest function), and the rest stays the same.

Okay for trunk like that.  Thanks!


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10.
  2021-07-14 21:22 ` Segher Boessenkool
@ 2021-07-26 20:46   ` Michael Meissner
  2021-07-26 21:03     ` Segher Boessenkool
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Meissner @ 2021-07-26 20:46 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt,
	Peter Bergner, Will Schmidt

On Wed, Jul 14, 2021 at 04:22:06PM -0500, Segher Boessenkool wrote:
> > --- a/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
> 
> > -/* { dg-final { scan-assembler-times "addic" 4 } } */
> > -/* { dg-final { scan-assembler-times "subfe" 1 } } */
> > -/* { dg-final { scan-assembler-times "addze" 3 } } */
> > +/* { dg-final { scan-assembler-times {\maddic\M}  4 { target { ! has_arch_pwr10 } } } } */
> > +/* { dg-final { scan-assembler-times {\msubfe\M}  1 { target { ! has_arch_pwr10 } } } } */
> > +/* { dg-final { scan-assembler-times {\maddic\M}  3 { target {   has_arch_pwr10 } } } } */
> > +/* { dg-final { scan-assembler-not   {\msubfe\M}    { target {   has_arch_pwr10 } } } } */
> > +/* { dg-final { scan-assembler-times {\msetbcr\M} 1 { target {   has_arch_pwr10 } } } } */
> > +/* { dg-final { scan-assembler-times {\maddze\M}  3 } } */
> 
> It may be easier to split the patch into two, where one part can get the
> setbcr (the first, simplest function), and the rest stays the same.

I really don't understand this comment.  I don't see how you could split the
patch in two, as the function that generates the setbcr (ne0) for power10 would
generate addic/subfe instead of the setbcr on earlier power systems.  Those
instruction counts have to be changed for the other functions.  So it doesn't
make sense to split the patch to me.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10.
  2021-07-26 20:46   ` Michael Meissner
@ 2021-07-26 21:03     ` Segher Boessenkool
  0 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2021-07-26 21:03 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt,
	Peter Bergner, Will Schmidt

On Mon, Jul 26, 2021 at 04:46:46PM -0400, Michael Meissner wrote:
> On Wed, Jul 14, 2021 at 04:22:06PM -0500, Segher Boessenkool wrote:
> > > --- a/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
> > > +++ b/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
> > 
> > > -/* { dg-final { scan-assembler-times "addic" 4 } } */
> > > -/* { dg-final { scan-assembler-times "subfe" 1 } } */
> > > -/* { dg-final { scan-assembler-times "addze" 3 } } */
> > > +/* { dg-final { scan-assembler-times {\maddic\M}  4 { target { ! has_arch_pwr10 } } } } */
> > > +/* { dg-final { scan-assembler-times {\msubfe\M}  1 { target { ! has_arch_pwr10 } } } } */
> > > +/* { dg-final { scan-assembler-times {\maddic\M}  3 { target {   has_arch_pwr10 } } } } */
> > > +/* { dg-final { scan-assembler-not   {\msubfe\M}    { target {   has_arch_pwr10 } } } } */
> > > +/* { dg-final { scan-assembler-times {\msetbcr\M} 1 { target {   has_arch_pwr10 } } } } */
> > > +/* { dg-final { scan-assembler-times {\maddze\M}  3 } } */
> > 
> > It may be easier to split the patch into two, where one part can get the
> > setbcr (the first, simplest function), and the rest stays the same.
> 
> I really don't understand this comment.  I don't see how you could split the
> patch in two, as the function that generates the setbcr (ne0) for power10 would
> generate addic/subfe instead of the setbcr on earlier power systems.  Those
> instruction counts have to be changed for the other functions.  So it doesn't
> make sense to split the patch to me.

I'm sorry.  I meant split the *testcase* into two :-)  One with the
first test, the other with the rest.


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-26 21:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-14 17:52 Repost #2: [PATCH] PR 100170: Fix eq/ne tests on power10 Michael Meissner
2021-07-14 20:25 ` Segher Boessenkool
2021-07-14 20:32   ` Segher Boessenkool
2021-07-14 21:22 ` Segher Boessenkool
2021-07-26 20:46   ` Michael Meissner
2021-07-26 21:03     ` Segher Boessenkool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).