[rs6000] Fix PR84534: several powerpc test cases fail starting with r257915

Message ID 593f8da7-fb69-0400-f691-dbabc32cb639@vnet.ibm.com
State New
Headers show
Series
  • [rs6000] Fix PR84534: several powerpc test cases fail starting with r257915
Related show

Commit Message

Peter Bergner Feb. 28, 2018, 3:39 p.m.
This patch fixes PR84534 by XFAILing one test because we are deprecating
-maltivec=be when run on LE.  The other tests are fixed by not counting
xxlor insns.  We cannot rely on stable counts of xxlor insns, because we
generate them not only when using __builtin_vec_or(), but also whenever
we need to copy one vsx reg to another...which can vary depending on
previous optimizations, moon phases, etc.  Therefore, we just make sure
we generate at least one.  I'll note I did try adding -dp and seeing
which patterns generated the xxlors, but there wasn't a clear match
between the xxlor's generated because of the __builtin_vec_or and the
reg copies.

Ok for trunk?

Peter

gcc/testsuite/
	PR target/84534
	* gcc.target/powerpc/vec-setup-be-long.c: xfail.
	* gcc.target/powerpc/vsx-vector-6-le.c: Do not count xxlor's.
	* gcc.target/powerpc/vsx-vector-6-le.p9.c: Likewise.

Comments

Segher Boessenkool Feb. 28, 2018, 10:36 p.m. | #1
Hi!

On Wed, Feb 28, 2018 at 09:39:14AM -0600, Peter Bergner wrote:
> This patch fixes PR84534 by XFAILing one test because we are deprecating

> -maltivec=be when run on LE.  The other tests are fixed by not counting

> xxlor insns.  We cannot rely on stable counts of xxlor insns, because we

> generate them not only when using __builtin_vec_or(), but also whenever

> we need to copy one vsx reg to another...which can vary depending on

> previous optimizations, moon phases, etc.  Therefore, we just make sure

> we generate at least one.  I'll note I did try adding -dp and seeing

> which patterns generated the xxlors, but there wasn't a clear match

> between the xxlor's generated because of the __builtin_vec_or and the

> reg copies.


> gcc/testsuite/

> 	PR target/84534

> 	* gcc.target/powerpc/vec-setup-be-long.c: xfail.


"Add xfail for powerpc64le" or similar?

> 	* gcc.target/powerpc/vsx-vector-6-le.c: Do not count xxlor's.

> 	* gcc.target/powerpc/vsx-vector-6-le.p9.c: Likewise.


Please add a comment to the testcase why there is no count here.

Okay for trunk with that.  Thanks!


Segher
Peter Bergner Feb. 28, 2018, 10:51 p.m. | #2
On 2/28/18 4:36 PM, Segher Boessenkool wrote:
>> 	* gcc.target/powerpc/vec-setup-be-long.c: xfail.

> 

> "Add xfail for powerpc64le" or similar?


Doh!  I was going to say "why?" since we're xfailing it everywhere, but I
see I messed up that hunk, which should be "xfail {*-*-*}".  The test case
currently only runs on powerpc64le*-*-linux* and we want to xfail it on
powerpc64le*-*-linux*, so that leaves not really running it anywhere.
Offline, I mentioned using:

  -/* { dg-do run { target { powerpc64le*-*-linux* } } } */
  +/* { dg-do run { target { powerpc64le*-*-linux* } xfail { powerpc64le*-*-linux* } } } */

...and you said we could just use "xfail {*-*-*}".  But thinking about
it some more, doesn't "xfail {*-*-*}" add XFAILs on BE, AIX, etc. that
never used to run the test because the target didn't allow it?
So should we go with my original idea above?  Or maybe we don't care
that we XFAIL on some targets since we're just going to remove the
test next release with the removal -maltivec=be?


>> 	* gcc.target/powerpc/vsx-vector-6-le.c: Do not count xxlor's.

>> 	* gcc.target/powerpc/vsx-vector-6-le.p9.c: Likewise.

> 

> Please add a comment to the testcase why there is no count here.


Will do.

Peter
Segher Boessenkool Feb. 28, 2018, 11:01 p.m. | #3
On Wed, Feb 28, 2018 at 04:51:27PM -0600, Peter Bergner wrote:
> On 2/28/18 4:36 PM, Segher Boessenkool wrote:

> >> 	* gcc.target/powerpc/vec-setup-be-long.c: xfail.

> > 

> > "Add xfail for powerpc64le" or similar?

> 

> Doh!  I was going to say "why?" since we're xfailing it everywhere, but I


Because you can use a few more words in changelog entries ;-)

"Add xfail." is fine, too.  Just "xfail." is unclear.

> see I messed up that hunk, which should be "xfail {*-*-*}".  The test case

> currently only runs on powerpc64le*-*-linux* and we want to xfail it on

> powerpc64le*-*-linux*, so that leaves not really running it anywhere.

> Offline, I mentioned using:

> 

>   -/* { dg-do run { target { powerpc64le*-*-linux* } } } */

>   +/* { dg-do run { target { powerpc64le*-*-linux* } xfail { powerpc64le*-*-linux* } } } */

> 

> ...and you said we could just use "xfail {*-*-*}".  But thinking about

> it some more, doesn't "xfail {*-*-*}" add XFAILs on BE, AIX, etc. that

> never used to run the test because the target didn't allow it?


Yeah; keeping it as a separate dg-xfail-if statement works fine though
(is the above dg-do valid syntax even?  I have no idea).

> So should we go with my original idea above?  Or maybe we don't care

> that we XFAIL on some targets since we're just going to remove the

> test next release with the removal -maltivec=be?


It would be nice to have clean test results, which is all this patch is
about anyway, right?


Segher
Peter Bergner March 1, 2018, 3:59 a.m. | #4
On 2/28/18 5:01 PM, Segher Boessenkool wrote:
> On Wed, Feb 28, 2018 at 04:51:27PM -0600, Peter Bergner wrote:

>> Doh!  I was going to say "why?" since we're xfailing it everywhere, but I

> 

> Because you can use a few more words in changelog entries ;-)

> 

> "Add xfail." is fine, too.  Just "xfail." is unclear.


Ok.



>> So should we go with my original idea above?  Or maybe we don't care

>> that we XFAIL on some targets since we're just going to remove the

>> test next release with the removal -maltivec=be?

> 

> It would be nice to have clean test results, which is all this patch is

> about anyway, right?


Ok, this XFAILs the test case and shouldn't create any extra noise when
it shouldn't.  I also added the comment about no longer counting xxlor
insns.  Better?

Peter

	PR target/84534
	* gcc.target/powerpc/vec-setup-be-long.c: Add dg-xfail-run-if
        on powerpc64le*-*-linux*.
	* gcc.target/powerpc/vsx-vector-6-le.c: Do not count xxlor's.
	* gcc.target/powerpc/vsx-vector-6-le.p9.c: Likewise.

Index: gcc/testsuite/gcc.target/powerpc/vec-setup-be-long.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-setup-be-long.c	(revision 258038)
+++ gcc/testsuite/gcc.target/powerpc/vec-setup-be-long.c	(working copy)
@@ -1,4 +1,7 @@
+/* Per PR78303, we are deprecating usage of -maltivec=be on little endian,
+   so XFAIL this test until support is actually removed.  */
 /* { dg-do run { target { powerpc64le*-*-linux* } } } */
+/* { dg-xfail-run-if "PR78303 and PR84534" { powerpc64le*-*-linux* } } */
 /* { dg-require-effective-target vsx_hw } */
 /* Disable warnings to squelch deprecation message about -maltivec=be.  */
 /* { dg-options "-w -O2 -mvsx -maltivec=be" } */
Index: gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c	(revision 258038)
+++ gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c	(working copy)
@@ -9,7 +9,11 @@
 /* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
 /* { dg-final { scan-assembler-times "xvadddp" 1 } } */
 /* { dg-final { scan-assembler-times "xxlnor" 8 } } */
-/* { dg-final { scan-assembler-times "xxlor" 30 } } */
+/* We generate xxlor instructions for many reasons other than or'ing vector
+   operands or calling __builtin_vec_or(), which  means we cannot rely on
+   their usage counts being stable.  Therefore, we just ensure at least one
+   xxlor instruction was generated.  */
+/* { dg-final { scan-assembler "xxlor" } } */
 /* { dg-final { scan-assembler-times "xvcmpeqdp" 5 } } */
 /* { dg-final { scan-assembler-times "xvcmpgtdp" 8 } } */
 /* { dg-final { scan-assembler-times "xvcmpgedp" 6 } } */
Index: gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.p9.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.p9.c	(revision 258038)
+++ gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.p9.c	(working copy)
@@ -9,7 +9,11 @@
 /* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
 /* { dg-final { scan-assembler-times "xvadddp" 1 } } */
 /* { dg-final { scan-assembler-times "xxlnor" 7 } } */
-/* { dg-final { scan-assembler-times "xxlor" 20 } } */
+/* We generate xxlor instructions for many reasons other than or'ing vector
+   operands or calling __builtin_vec_or(), which  means we cannot rely on
+   their usage counts being stable.  Therefore, we just ensure at least one
+   xxlor instruction was generated.  */
+/* { dg-final { scan-assembler "xxlor" } } */
 /* { dg-final { scan-assembler-times "xvcmpeqdp" 5 } } */
 /* { dg-final { scan-assembler-times "xvcmpgtdp" 8 } } */
 /* { dg-final { scan-assembler-times "xvcmpgedp" 8 } } */
Segher Boessenkool March 2, 2018, 1:45 a.m. | #5
On Wed, Feb 28, 2018 at 09:59:27PM -0600, Peter Bergner wrote:
> >> So should we go with my original idea above?  Or maybe we don't care

> >> that we XFAIL on some targets since we're just going to remove the

> >> test next release with the removal -maltivec=be?

> > 

> > It would be nice to have clean test results, which is all this patch is

> > about anyway, right?

> 

> Ok, this XFAILs the test case and shouldn't create any extra noise when

> it shouldn't.  I also added the comment about no longer counting xxlor

> insns.  Better?

> 

> Peter

> 

> 	PR target/84534

> 	* gcc.target/powerpc/vec-setup-be-long.c: Add dg-xfail-run-if

>         on powerpc64le*-*-linux*.


(wrong indent: should be a tab and no extra spaces).

Okay for trunk, thanks!


Segher
Peter Bergner March 2, 2018, 2:57 a.m. | #6
On 3/1/18 7:45 PM, Segher Boessenkool wrote:
> On Wed, Feb 28, 2018 at 09:59:27PM -0600, Peter Bergner wrote:

>> 	PR target/84534

>> 	* gcc.target/powerpc/vec-setup-be-long.c: Add dg-xfail-run-if

>>         on powerpc64le*-*-linux*.

> 

> (wrong indent: should be a tab and no extra spaces).


Heh, that's just an artifact of me being lazy and cut/pasting the ChangeLog
entry into my mailer. :-)



> Okay for trunk, thanks!


Thanks, committed.

Peter

Patch

Index: gcc/testsuite/gcc.target/powerpc/vec-setup-be-long.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-setup-be-long.c	(revision 258038)
+++ gcc/testsuite/gcc.target/powerpc/vec-setup-be-long.c	(working copy)
@@ -1,4 +1,6 @@ 
-/* { dg-do run { target { powerpc64le*-*-linux* } } } */
+/* Per PR78303, we are deprecating usage of -maltivec=be on little endian,
+   so XFAIL this test until support is actually removed.  */
+/* { dg-do run xfail { powerpc64le*-*-linux* } } } */
 /* { dg-require-effective-target vsx_hw } */
 /* Disable warnings to squelch deprecation message about -maltivec=be.  */
 /* { dg-options "-w -O2 -mvsx -maltivec=be" } */
Index: gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c	(revision 258038)
+++ gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c	(working copy)
@@ -9,7 +9,7 @@ 
 /* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
 /* { dg-final { scan-assembler-times "xvadddp" 1 } } */
 /* { dg-final { scan-assembler-times "xxlnor" 8 } } */
-/* { dg-final { scan-assembler-times "xxlor" 30 } } */
+/* { dg-final { scan-assembler "xxlor" } } */
 /* { dg-final { scan-assembler-times "xvcmpeqdp" 5 } } */
 /* { dg-final { scan-assembler-times "xvcmpgtdp" 8 } } */
 /* { dg-final { scan-assembler-times "xvcmpgedp" 6 } } */
Index: gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.p9.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.p9.c	(revision 258038)
+++ gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.p9.c	(working copy)
@@ -9,7 +9,7 @@ 
 /* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
 /* { dg-final { scan-assembler-times "xvadddp" 1 } } */
 /* { dg-final { scan-assembler-times "xxlnor" 7 } } */
-/* { dg-final { scan-assembler-times "xxlor" 20 } } */
+/* { dg-final { scan-assembler "xxlor" } } */
 /* { dg-final { scan-assembler-times "xvcmpeqdp" 5 } } */
 /* { dg-final { scan-assembler-times "xvcmpgtdp" 8 } } */
 /* { dg-final { scan-assembler-times "xvcmpgedp" 8 } } */