rs6000: Fix broken gcc.target/powerpc/fold-vec-st-*.c test cases [PR93913]

Message ID 348e0d67-30e6-2da5-5352-3bf109769ab8@bergner.org
State New
Headers show
Series
  • rs6000: Fix broken gcc.target/powerpc/fold-vec-st-*.c test cases [PR93913]
Related show

Commit Message

Peter Bergner Feb. 25, 2020, 4:49 p.m.
POWER9 added the stxv instruction, which is d-form store.  When we compile
the FAILing test cases in PR93913 when -mcpu=power9 is the default, then
we may sometimes get stxv rather than stvx generated.  The fix here is
to allow both mnemonics when counting.

Ok for trunk and GCC 9, where these are FAILing now?

Peter

	PR target/93913
	* gcc.target/powerpc/fold-vec-st-char.c (scan-assembler-times): Allow
	stxv instruction as well.
	* gcc.target/powerpc/fold-vec-st-float.c: Likewise.
	* gcc.target/powerpc/fold-vec-st-int.c: Likewise.
	* gcc.target/powerpc/fold-vec-st-short.c: Likewise.

Comments

Segher Boessenkool Feb. 25, 2020, 10:25 p.m. | #1
On Tue, Feb 25, 2020 at 10:49:16AM -0600, Peter Bergner wrote:
> POWER9 added the stxv instruction, which is d-form store.  When we compile

> the FAILing test cases in PR93913 when -mcpu=power9 is the default, then

> we may sometimes get stxv rather than stvx generated.  The fix here is

> to allow both mnemonics when counting.


Well, you now get an extra mask instruction (rldicr for example) as well,
right?  While that mask usually isn't needed.

> -/* { dg-final { scan-assembler-times {\mstvx\M} 14 } } */

> +/* { dg-final { scan-assembler-times {\mstvx|stxv\M} 14 } } */


That checks if either the string
  \mstvx
or the string
  stxv\M
is part of the generated code.

You want
  {\m(stvx|stxv)\M}
or
  {\mstvx\M|\mstxv\M}
or
  {\mst(vx|xv)\M}
or one of many more variants (maybe you want to allow stxvx as well, for
example).


Segher
Peter Bergner Feb. 26, 2020, 5:29 p.m. | #2
On 2/25/20 4:25 PM, Segher Boessenkool wrote:
> Well, you now get an extra mask instruction (rldicr for example) as well,

> right?  While that mask usually isn't needed.


I believe the mask is implicit by the pattern used by the vec_st()
builtin, which normally gets mapped to the Altivec stvx insn, so
when we use the d-form stxv insn, we have to make explicit that
and:.

I still think using vec_st() or any of the other vector store
builtins is less preferable than just dereferencing a pointer.  



>> -/* { dg-final { scan-assembler-times {\mstvx\M} 14 } } */

>> +/* { dg-final { scan-assembler-times {\mstvx|stxv\M} 14 } } */

> 

> That checks if either the string

>   \mstvx

> or the string

>   stxv\M


Oops, good catch.


> You want

>   {\m(stvx|stxv)\M}


Is the patch ok with this variant?  I think this is more readable to
me than the other variants you gave...at least to me. :-)

Peter
Peter Bergner Feb. 26, 2020, 6:16 p.m. | #3
On 2/26/20 11:29 AM, Peter Bergner wrote:
>> You want

>>   {\m(stvx|stxv)\M}


As we discussed offline, the regex above double counts everything,
so I went with {\m(?:stvx|stxv|stxvx)\M} which you pointed me to.
Pushed to master.  I'll push to the gcc-9 branch tomorrow after
Bill's regression scripts confirm it's fixed on master.

Thanks!

Peter

Patch

diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-char.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-char.c
index 4c4582780c2..61cd9462346 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-char.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-char.c
@@ -91,4 +91,4 @@  testst_cst7 (vector bool char vbc1, int i1, signed char * vscp)
 	return vec_st(vbc1, 36, vscp);
 }
 
-/* { dg-final { scan-assembler-times {\mstvx\M} 14 } } */
+/* { dg-final { scan-assembler-times {\mstvx|stxv\M} 14 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-float.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-float.c
index 5a8fc6670a6..408555b0b62 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-float.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-float.c
@@ -31,4 +31,4 @@  testst_cst2 (vector float vf1, int i1, float * fp)
 	return vec_st(vf1, 24, fp);
 }
 
-/* { dg-final { scan-assembler-times {\mstvx\M}  4 } } */
+/* { dg-final { scan-assembler-times {\mstvx|stxv\M} 4 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-int.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-int.c
index 4db35f0dd94..4bb504d6caf 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-int.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-int.c
@@ -80,5 +80,5 @@  testst_cst7 (vector bool int vbi1, int i1, signed int * vsip)
 	return vec_st(vbi1, 36, vsip);
 }
 
-/* { dg-final { scan-assembler-times {\mstvx\M}  14 } } */
+/* { dg-final { scan-assembler-times {\mstvx|stxv\M} 14 } } */
 
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-short.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-short.c
index ba8397eef26..b1f58d865cf 100644
--- a/gcc/testsuite/gcc.target/powerpc/fold-vec-st-short.c
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-st-short.c
@@ -80,4 +80,4 @@  testst_cst7 (vector bool short vbs1, int i1, signed short * vssp)
 	return vec_st(vbs1, 36, vssp);
 }
 
-/* { dg-final { scan-assembler-times {\mstvx\M} 14} } */
+/* { dg-final { scan-assembler-times {\mstvx|stxv\M} 14} } */