On Tue, Mar 25, 2025 at 03:33:59PM -0500, Peter Bergner wrote:
> On 3/25/25 1:42 AM, jeevitha wrote:
> > gcc/testsuite/
> > PR testsuite/119382
> > * gcc.target/powerpc/vsx-builtin-7.c: Add '-fno-ipa-icf' to dg-options.
> >
> > diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c
> > b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c
> > index 5095d5030fd..78e4e23d102 100644
> > --- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c
> > @@ -1,6 +1,6 @@
> > /* { dg-do compile { target { powerpc*-*-* } } } */
> > /* { dg-skip-if "" { powerpc*-*-darwin* } } */
> > -/* { dg-options "-O2 -mdejagnu-cpu=power7 -fno-inline-functions" } */
> > +/* { dg-options "-O2 -mdejagnu-cpu=power7 -fno-inline-functions
> > -fno-ipa-icf" } */
> > /* { dg-require-effective-target powerpc_vsx } */
> >
> > /* Test simple extract/insert/slat operations. Make sure all types are
>
> I REALLY dislike these BIG built-in test cases that test for all possible
> built-ins and then do multiple insn scans.
Yup. Very generally, every testcase should say exactly what it is
testing for (in the header comments, for example). The "what" it is
testing for and the "how" it is testing for it are related in an obvious
way, for very simple testcases, but not for more complex ones.
> Much better would be having
> many small test cases that test for one specific thing, rather than testing
> lots of things.
Yup. Easier to debug, too, when they start misbehaving. Easier to edit
etc. even! We all now how to run trivial cp commands I hope :-)
> As shown here, they're pretty fragile to changes in the compiler.
Pretty much all scan-assembler-times tests are very suspect, not to say
plain wrong.
> That said, I'm not sure it's really worth splitting this older Power7
> test case up, so I guess adding -fno-ipa-icf is probably the best/easiest
> of all of the bad options.
It is probably less work the next time one of those tests starts failing
to *start* with splitting the test up :-)
> Segher, any reason you can give on why we shouldn't go the easy route to
> "fix" (yes, these are air-quotes) this by using -fno-ipa-icf?
One reason is that that option should not make any difference whatsoever
for a well-written testcase: a testcases that wants to test what insns
are generated for particular code, damn well should be written in such a
way that it is very unlikely the compiler will ever generate different
code for it. Another reason is I had to look up what that option with
the cryptical name does, what that names stands for. And finally, will
we be doing more maintenance on this later? Testcase maintenance is
wasted work, work that does not scale even, so it is important to write
testcases so that maintenance isn't needed, and if it becomes necessary
anyway to improve it so that it will not be needed so much in the
future.
Segher
