Re: [PATCH v2] PR92398: Fix testcase failure of pr72804.c

[Segher Boessenkool]

Hi!

On Mon, Nov 18, 2019 at 04:21:07PM +0800, luoxhu wrote:
> On 2019/11/15 18:17, Segher Boessenkool wrote:
> > On Thu, Nov 14, 2019 at 09:12:32PM -0600, Xiong Hu Luo wrote:
> >> P9LE generated instruction is not worse than P8LE.
> >> mtvsrdd;xxlnot;stxv vs. not;not;std;std.
> >> Update the test case to fix failures.
> > 
> > So this no longer runs it for p7, and it also doesn't run it for cpus
> > after p9 anymore.  Could you change it to be a test for p9 and above,
> > and one for before p9?  Does that work?
> Since one test file could not support multiple dg do-compile or dg-options,
> it seems not feasible to build both power7, power8 and power9 option in one
> source file, I added check_effective_target_power[7,8,9] and
> add_options_for_power[7,8,9] in target-supports.exp for each cpu configure.

You can add a selector to the scan-assembler tests, I meant.

There are two ways to run tests: in one way you use just what -mcpu=
etc. you happen to get; in the other way, you force particular options.
With the first way, you will automatically also test everything on newer
cpus.  With the second way, you also test (for example) p8 code gen when
actually testing on a p9 system.  Both approaches have their uses.

In this case, I would use the first approach, but okay :-)

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr72804.h
> @@ -0,0 +1,17 @@
> +/* This test code is included into pr72804.p8.c and pr72804.p9.c

And p7.

> +   The two files have the tests for the number of instructions generated for
> +   P8LE versus P9LE.  */

Three files :-)

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr72804.p7.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O2" } */
> +/* { dg-add-options power7 } */
> +
> +/* { dg-final { scan-assembler-times "not " 4 {xfail be} } } */
> +/* { dg-final { scan-assembler-times "std " 2  } } */
> +/* { dg-final { scan-assembler-times "ld " 2 } } */
> +/* { dg-final { scan-assembler-not "lxvd2x" {xfail be} } } */
> +/* { dg-final { scan-assembler-not "stxvd2x" {xfail be} } } */
> +/* { dg-final { scan-assembler-not "xxpermdi" } } */
> +/* { dg-final { scan-assembler-not "mfvsrd" } } */
> +/* { dg-final { scan-assembler-not "mfvsrd" } } */

So this is exactly the same as p8.  Hmm.

> --- a/gcc/testsuite/gcc.target/powerpc/pr72804.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr72804.p9.c
> @@ -1,25 +1,16 @@
>  /* { dg-do compile { target { lp64 } } } */
> -/* { dg-skip-if "" { powerpc*-*-darwin* } } */
>  /* { dg-require-effective-target powerpc_vsx_ok } */
> -/* { dg-options "-O2 -mvsx -fno-inline-functions --param 
> max-inline-insns-single-O2=200" } */
> +/* { dg-options "-O2 -mvsx" } */
> +/* { dg-add-options power9 } */

-mvsx is superfluous if you have -mcpu=power9 already.
You shouldn't test powerpc_vsx_ok if you force a CPU that has it.

> -/* { dg-final { scan-assembler-times "not " 4 } } */
> -/* { dg-final { scan-assembler-times "std " 2 } } */
> +/* { dg-final { scan-assembler-times "not " 2 { target power9 } } } */
> +/* { dg-final { scan-assembler-not "std " { target power9 } } } */
>  /* { dg-final { scan-assembler-times "ld " 2 } } */
>  /* { dg-final { scan-assembler-not "lxvd2x" } } */
>  /* { dg-final { scan-assembler-not "stxvd2x" } } */
>  /* { dg-final { scan-assembler-not "xxpermdi" } } */
>  /* { dg-final { scan-assembler-not "mfvsrd" } } */
>  /* { dg-final { scan-assembler-not "mfvsrd" } } */

You now force power9 for this test, so you shouldn't test for it anymore.

> +proc check_effective_target_power7 {  } {
> +  return [check_no_compiler_messages_nocache power7 assembly {
> + #if !(!defined(_ARCH_PWR8) && defined(_ARCH_PWR7))
> + #error !_ARCH_PWR7
> + #endif
> +  } "-mdejagnu-cpu=power7"]
> +}

So this test for p7 exactly (not power8 or up).  Okay.  Add a comment
saying that?

> +proc add_options_for_power7 { flags } {
> +  if { [istarget powerpc*-*-*]  } {
> +return "$flags -mdejagnu-cpu=power7"
> +  }
> +  return "$flags"
> +}

Is this better than just adding -mdejagnu-cpu=power7 directly?


Segher

Re: [PATCH v2] PR92398: Fix testcase failure of pr72804.c

[luoxhu]

Hi,

On 2019/11/15 18:17, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Nov 14, 2019 at 09:12:32PM -0600, Xiong Hu Luo wrote:
>> P9LE generated instruction is not worse than P8LE.
>> mtvsrdd;xxlnot;stxv vs. not;not;std;std.
>> Update the test case to fix failures.
> 
> So this no longer runs it for p7, and it also doesn't run it for cpus
> after p9 anymore.  Could you change it to be a test for p9 and above,
> and one for before p9?  Does that work?
Since one test file could not support multiple dg do-compile or dg-options,
it seems not feasible to build both power7, power8 and power9 option in one
source file, I added check_effective_target_power[7,8,9] and
add_options_for_power[7,8,9] in target-supports.exp for each cpu configure.

> 
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr72804.p8.c
>> @@ -0,0 +1,16 @@
>> +/* { dg-do compile { target lp64 } } */
>> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */
>> +/* { dg-require-effective-target powerpc_vsx_ok } */
>> +/* { dg-options "-O2 -mvsx -mdejagnu-cpu=power8" } */
> 
> -mvsx is implied by power8 (or power7, already).
> 
> You don't need the test for Darwin: Darwin never has VSX, already (and
> if it somehow would ever get support for it, we'd want this test to run
> for it).
OK.

> 
>> +/* { dg-final { scan-assembler-times "std " 0 } } */
> 
> We usually write this as scan-assembler-not, but this works just fine
> as well.  Whichever you prefer.
Updated.

> 
> 
> Segher
> 

V2 patch as below, Thanks:


P9LE generated instruction is not worse than P8LE.
mtvsrdd;xxlnot;stxv vs. not;not;std;std.
Update the test case to fix failures.

gcc/testsuite/ChangeLog:

2019-11-15  Luo Xiong Hu  

testsuite/pr92398
* gcc.target/powerpc/pr72804.h: New.
* gcc.target/powerpc/pr72804.p7.c: New.
* gcc.target/powerpc/pr72804.p8.c: New.
* gcc.target/powerpc/pr72804.c: Rename to ...
* gcc.target/powerpc/pr72804.p9.c: ... this one.
* lib/target-supports.exp (check_effective_target_power7): New.
(add_options_for_power7): New.
(check_effective_target_power8): New.
(add_options_for_power9): New.
(check_effective_target_power9): New.
(add_options_for_power8): New.
---
 gcc/testsuite/gcc.target/powerpc/pr72804.h| 17 +++
 gcc/testsuite/gcc.target/powerpc/pr72804.p7.c | 16 +++
 gcc/testsuite/gcc.target/powerpc/pr72804.p8.c | 16 +++
 .../powerpc/{pr72804.c => pr72804.p9.c}   | 23 +++---
 gcc/testsuite/lib/target-supports.exp | 45 +++
 5 files changed, 101 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr72804.h
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr72804.p7.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr72804.p8.c
 rename gcc/testsuite/gcc.target/powerpc/{pr72804.c => pr72804.p9.c} (52%)

diff --git a/gcc/testsuite/gcc.target/powerpc/pr72804.h 
b/gcc/testsuite/gcc.target/powerpc/pr72804.h
new file mode 100644
index 000..8a5ea93cc17
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr72804.h
@@ -0,0 +1,17 @@
+/* This test code is included into pr72804.p8.c and pr72804.p9.c
+   The two files have the tests for the number of instructions generated for
+   P8LE versus P9LE.  */
+
+__int128_t
+foo (__int128_t *src)
+{
+  return ~*src;
+}
+
+void
+bar (__int128_t *dst, __int128_t src)
+{
+  *dst =  ~src;
+}
+
+
diff --git a/gcc/testsuite/gcc.target/powerpc/pr72804.p7.c 
b/gcc/testsuite/gcc.target/powerpc/pr72804.p7.c
new file mode 100644
index 000..f78a8557c9f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr72804.p7.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options power7 } */
+
+/* { dg-final { scan-assembler-times "not " 4 {xfail be} } } */
+/* { dg-final { scan-assembler-times "std " 2  } } */
+/* { dg-final { scan-assembler-times "ld " 2 } } */
+/* { dg-final { scan-assembler-not "lxvd2x" {xfail be} } } */
+/* { dg-final { scan-assembler-not "stxvd2x" {xfail be} } } */
+/* { dg-final { scan-assembler-not "xxpermdi" } } */
+/* { dg-final { scan-assembler-not "mfvsrd" } } */
+/* { dg-final { scan-assembler-not "mfvsrd" } } */
+
+/* Source code for the test in pr72804.h */
+#include "pr72804.h"
diff --git a/gcc/testsuite/gcc.target/powerpc/pr72804.p8.c 
b/gcc/testsuite/gcc.target/powerpc/pr72804.p8.c
new file mode 100644
index 000..1efc0691809
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr72804.p8.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options power8 } */
+
+/* { dg-final { scan-assembler-times "not " 4 {xfail be} } } */
+/* { dg-final { scan-assembler-times "std " 2 {xfail be} } } */
+/* { dg-final { scan-assembler-times "ld " 2 } } */
+/* { dg-final { scan-assembler-not "lxvd2x" } } */
+/* { d