[PATCH] Fix vec-splati-runnable.c test.

[PATCH] Fix vec-splati-runnable.c test.

[Michael Meissner]

[PATCH] Fix vec-splati-runnable.c test.

I noticed that the vec-splati-runnable.c did not have an abort after one
of the tests.  If the test was run with optimization, the optimizer could
delete some of the tests and throw off the count.

I have bootstraped this on LE power9 and BE power8 systems.  There were no
regressions in the tests.  Can I check this into the trunk?

I do not expect to back port this to GCC 11 unless we will be back porting the
future patches that add support for the XXSPLITW, XXSPLTIDP, and XXSPLTI32DX
instructions.

gcc/testsuite/
2021-05-18  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.target/powerpc/vec-splati-runnable.c: Run test with -O2
	optimization.  Do not check what XXSPLTIDP generates if the value
	is undefined.
---
 .../gcc.target/powerpc/vec-splati-runnable.c  | 29 ++++++-------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
index e84ce77a21d..a135279b1d7 100644
--- a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
+++ b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
@@ -1,7 +1,7 @@
 /* { dg-do run { target { power10_hw } } } */
 /* { dg-do link { target { ! power10_hw } } } */
 /* { dg-require-effective-target power10_ok } */
-/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
+/* { dg-options "-mdejagnu-cpu=power10 -save-temps -O2" } */
 #include <altivec.h>
 
 #define DEBUG 0
@@ -12,6 +12,8 @@
 
 extern void abort (void);
 
+volatile vector double vresult_d_undefined;
+
 int
 main (int argc, char *argv [])
 {
@@ -85,25 +87,12 @@ main (int argc, char *argv [])
 #endif
   }
 
-  /* This test will generate a "note" to the user that the argument
-     is subnormal.  It is not an error, but results are not defined.  */
-  vresult_d = (vector double) { 2.0, 3.0 };
-  expected_vresult_d = (vector double) { 6.6E-42f, 6.6E-42f };
-
-  vresult_d = vec_splatid (6.6E-42f);
-
-  /* Although the instruction says the results are not defined, it does seem
-     to work, at least on Mambo.  But no guarentees!  */
-  if (!vec_all_eq (vresult_d,  expected_vresult_d)) {
-#if DEBUG
-    printf("ERROR, vec_splati (6.6E-42f)\n");
-    for(i = 0; i < 2; i++)
-      printf(" vresult_d[%i] = %e, expected_vresult_d[%i] = %e\n",
-	     i, vresult_d[i], i, expected_vresult_d[i]);
-#else
-    ;
-#endif
-  }
+  /* This test will generate a "note" to the user that the argument is
+     subnormal.  It is not an error, but results are not defined.  Because this
+     is undefined, we cannot check that any value is correct.  Just store it in
+     a volatile variable so the XXSPLTIDP instruction gets generated and the
+     warning message printed. */
+  vresult_d_undefined = vec_splatid (6.6E-42f);
 
   /* Vector splat immediate */
   vsrc_a_int = (vector int) { 2, 3, 4, 5 };
-- 
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

Re: [PATCH] Fix vec-splati-runnable.c test.

[will schmidt]

On Tue, 2021-05-18 at 16:49 -0400, Michael Meissner wrote:
> [PATCH] Fix vec-splati-runnable.c test.
> 

hi,


> I noticed that the vec-splati-runnable.c did not have an abort after one
> of the tests.  If the test was run with optimization, the optimizer could
> delete some of the tests and throw off the count.
> 


> I have bootstraped this on LE power9 and BE power8 systems.  There were no
> regressions in the tests.  Can I check this into the trunk?
> 
> I do not expect to back port this to GCC 11 unless we will be back porting the
> future patches that add support for the XXSPLITW, XXSPLTIDP, and XXSPLTI32DX
> instructions.
> 
> gcc/testsuite/
> 2021-05-18  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	* gcc.target/powerpc/vec-splati-runnable.c: Run test with -O2
> 	optimization.  Do not check what XXSPLTIDP generates if the value
> 	is undefined.
> ---
>  .../gcc.target/powerpc/vec-splati-runnable.c  | 29 ++++++-------------
>  1 file changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> index e84ce77a21d..a135279b1d7 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> @@ -1,7 +1,7 @@
>  /* { dg-do run { target { power10_hw } } } */
>  /* { dg-do link { target { ! power10_hw } } } */
>  /* { dg-require-effective-target power10_ok } */
> -/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
> +/* { dg-options "-mdejagnu-cpu=power10 -save-temps -O2" } */
>  #include <altivec.h>
> 
>  #define DEBUG 0
> @@ -12,6 +12,8 @@
> 
>  extern void abort (void);
> 
> +volatile vector double vresult_d_undefined;
> +
>  int
>  main (int argc, char *argv [])
>  {
> @@ -85,25 +87,12 @@ main (int argc, char *argv [])
>  #endif
>    }
> 
> -  /* This test will generate a "note" to the user that the argument
> -     is subnormal.  It is not an error, but results are not defined.  */
> -  vresult_d = (vector double) { 2.0, 3.0 };
> -  expected_vresult_d = (vector double) { 6.6E-42f, 6.6E-42f };
> -
> -  vresult_d = vec_splatid (6.6E-42f);
> -
> -  /* Although the instruction says the results are not defined, it does seem
> -     to work, at least on Mambo.  But no guarentees!  */
> -  if (!vec_all_eq (vresult_d,  expected_vresult_d)) {
> -#if DEBUG
> -    printf("ERROR, vec_splati (6.6E-42f)\n");
> -    for(i = 0; i < 2; i++)
> -      printf(" vresult_d[%i] = %e, expected_vresult_d[%i] = %e\n",
> -	     i, vresult_d[i], i, expected_vresult_d[i]);
> -#else
> -    ;
> -#endif
> -  }
> +  /* This test will generate a "note" to the user that the argument is
> +     subnormal.  It is not an error, but results are not defined.  Because this
> +     is undefined, we cannot check that any value is correct.  Just store it in

as in undefined-behavior..?

> +     a volatile variable so the XXSPLTIDP instruction gets generated and the
> +     warning message printed. */
> +  vresult_d_undefined = vec_splatid (6.6E-42f);


This does not look like it adds an abort() call as I would have
expected per the patch description. 

So this looks like it still calls vec_splatid(), but instead assigns
result to a variable name vresult_d_undefined.   Also removes some
DEBUG code, which is fine.  So just the vec_all_eq() call is removed?  
I'm not certain I see how that will change the results, just the -O2
optimization makes the difference?
I may be missing something...


Thanks,
-Will

> 
>    /* Vector splat immediate */
>    vsrc_a_int = (vector int) { 2, 3, 4, 5 };
> -- 
> 2.31.1
> 

Re: [PATCH] Fix vec-splati-runnable.c test.

[Michael Meissner]

On Thu, May 20, 2021 at 02:31:59PM -0500, will schmidt wrote:
> On Tue, 2021-05-18 at 16:49 -0400, Michael Meissner wrote:
> > [PATCH] Fix vec-splati-runnable.c test.
> > 
> 
> hi,
> 
> 
> > I noticed that the vec-splati-runnable.c did not have an abort after one
> > of the tests.  If the test was run with optimization, the optimizer could
> > delete some of the tests and throw off the count.
> > 
> 
> 
> > I have bootstraped this on LE power9 and BE power8 systems.  There were no
> > regressions in the tests.  Can I check this into the trunk?
> > 
> > I do not expect to back port this to GCC 11 unless we will be back porting the
> > future patches that add support for the XXSPLITW, XXSPLTIDP, and XXSPLTI32DX
> > instructions.
> > 
> > gcc/testsuite/
> > 2021-05-18  Michael Meissner  <meissner@linux.ibm.com>
> > 
> > 	* gcc.target/powerpc/vec-splati-runnable.c: Run test with -O2
> > 	optimization.  Do not check what XXSPLTIDP generates if the value
> > 	is undefined.
> > ---
> >  .../gcc.target/powerpc/vec-splati-runnable.c  | 29 ++++++-------------
> >  1 file changed, 9 insertions(+), 20 deletions(-)
> > 
> > diff --git a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> > index e84ce77a21d..a135279b1d7 100644
> > --- a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> > @@ -1,7 +1,7 @@
> >  /* { dg-do run { target { power10_hw } } } */
> >  /* { dg-do link { target { ! power10_hw } } } */
> >  /* { dg-require-effective-target power10_ok } */
> > -/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
> > +/* { dg-options "-mdejagnu-cpu=power10 -save-temps -O2" } */
> >  #include <altivec.h>
> > 
> >  #define DEBUG 0
> > @@ -12,6 +12,8 @@
> > 
> >  extern void abort (void);
> > 
> > +volatile vector double vresult_d_undefined;
> > +
> >  int
> >  main (int argc, char *argv [])
> >  {
> > @@ -85,25 +87,12 @@ main (int argc, char *argv [])
> >  #endif
> >    }
> > 
> > -  /* This test will generate a "note" to the user that the argument
> > -     is subnormal.  It is not an error, but results are not defined.  */
> > -  vresult_d = (vector double) { 2.0, 3.0 };
> > -  expected_vresult_d = (vector double) { 6.6E-42f, 6.6E-42f };
> > -
> > -  vresult_d = vec_splatid (6.6E-42f);
> > -
> > -  /* Although the instruction says the results are not defined, it does seem
> > -     to work, at least on Mambo.  But no guarentees!  */
> > -  if (!vec_all_eq (vresult_d,  expected_vresult_d)) {
> > -#if DEBUG
> > -    printf("ERROR, vec_splati (6.6E-42f)\n");
> > -    for(i = 0; i < 2; i++)
> > -      printf(" vresult_d[%i] = %e, expected_vresult_d[%i] = %e\n",
> > -	     i, vresult_d[i], i, expected_vresult_d[i]);
> > -#else
> > -    ;
> > -#endif
> > -  }
> > +  /* This test will generate a "note" to the user that the argument is
> > +     subnormal.  It is not an error, but results are not defined.  Because this
> > +     is undefined, we cannot check that any value is correct.  Just store it in
> 
> as in undefined-behavior..?

It is undefined what value is put into the registers if you specify a denomral
value.

> 
> > +     a volatile variable so the XXSPLTIDP instruction gets generated and the
> > +     warning message printed. */
> > +  vresult_d_undefined = vec_splatid (6.6E-42f);
> 
> 
> This does not look like it adds an abort() call as I would have
> expected per the patch description. 

Originally the code did add an abort test.  Then I discovered the hardware
generates something different than what the simulator showed.  So I had to
remove the test for equality of what is loaded.  Sorry about that.

> 
> So this looks like it still calls vec_splatid(), but instead assigns
> result to a variable name vresult_d_undefined.   Also removes some
> DEBUG code, which is fine.  So just the vec_all_eq() call is removed?  
> I'm not certain I see how that will change the results, just the -O2
> optimization makes the difference?
> I may be missing something...

The original code was run at -O0.  If you enabled optimization, the optimizer
would delete setting the value because it did nothing.

	  /* This test will generate a "note" to the user that the argument
	     is subnormal.  It is not an error, but results are not defined.  */
	  vresult_d = (vector double) { 2.0, 3.0 };
	  expected_vresult_d = (vector double) { 6.6E-42f, 6.6E-42f };

	  vresult_d = vec_splatid (6.6E-42f);

	  /* Although the instruction says the results are not defined, it does seem
	     to work, at least on Mambo.  But no guarentees!  */
	  if (!vec_all_eq (vresult_d,  expected_vresult_d)) {
	#if DEBUG
	    printf("ERROR, vec_splati (6.6E-42f)\n");
	    for(i = 0; i < 2; i++)
	      printf(" vresult_d[%i] = %e, expected_vresult_d[%i] = %e\n",
		     i, vresult_d[i], i, expected_vresult_d[i]);
	#else
	    ;
	#endif
	  }

This of course throws off the insn counts.  I discovered this in preparing the
next set of patches that will add XXSPLTIW, XXSPLTIDP, and XXSPLTI32DX support.

My gut feel is that the built-in should not accept those values.  But that
should be done as a separate bug report.  I don't know if anybody is using the
buil-in, and trying to load those values.

-- 
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

Ping #2: [PATCH] Fix vec-splati-runnable.c test.

[Michael Meissner]

Ping patch again.

Original patch (Fix vec-splati-runnable.c test)

| Date: Tue, 18 May 2021 16:49:58 -0400
| Subject: [PATCH] Fix vec-splati-runnable.c test.
| Message-ID: <20210518204958.GA17351@ibm-toto.the-meissners.org>
| https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570684.html

Note as Will points out I need to change the check-in message to eliminate
adding abort () since in the current version of the test, the abort has been
changed to storing the value in a global volatile variable.

-- 
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