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 <meiss...@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: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
