On Mon, Nov 20, 2017 at 02:16:59PM -0600, Will Schmidt wrote:
> > > --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int-fwrapv.c
> > > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int-fwrapv.c
> > > @@ -1,11 +1,11 @@
> > > /* Verify that overloaded built-ins for vec_abs with int
> > > inputs produce the right results. */
> > >
> > > /* { dg-do compile } */
> > > /* { dg-require-effective-target powerpc_altivec_ok } */
> > > -/* { dg-options "-maltivec -O2 -fwrapv" } */
> > > +/* { dg-options "-maltivec -O2 -fwrapv -mcpu=power8" } */
> >
> > Is this testcase really testing something specific to power8 codegen?
>
> Yes, in contrast to power9 codegen.
Ah, so there is a separate testcase for power9. Okay.
Maybe it should say so in the filename, or in a comment at least.
It is very helpful if testcases say what they try to test.
> > Making all these testcases use -mcpu=power8 means they won't be
> tested
> > with any other settings. (Also, does that work if the user puts another
> > -mcpu= in RUNTESTFLAGS).
>
> possibly not. I did (do?) have a do-not-override option in place
> initially for a couple of the tests, but didn't seem to need it during
> my sniff-testing. I can try a few runs with manually specifying -mcpu
> flags and revisit.
> /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" }
> { "-mcpu=power8" } } */
Such a line is needed afaics, yeah.
> > I'm all for making the testresults cleaner, but let's not do that by
> > (effectively) disabling all failing tests ;-)
>
> No, thats really not my intent here. I'm not really convinced thats
> what I've done here either. I've added .p9 versions for most of what
> I've touched, with the intent to continue to have good coverage.
I missed that :-) Great, thanks.
> Adding the -mcpu=foo option to dg-options shouldn't be disabling the
> test..?
It means this test won't test codegen for any later cpu. So if there
is no separate test for later cpus, you lose test coverage.
Thanks,
Segher
