Hi!
On Tue, Jul 01, 2025 at 08:39:26PM +0200, Jakub Jelinek wrote:
> On Tue, Jul 01, 2025 at 12:04:04PM -0500, Segher Boessenkool wrote:
> > On Mon, Feb 17, 2025 at 02:28:50PM +0000, Alex Coplan wrote:
> > > After the recent alignment peeling enhancements in the vectorizer we
> > > started vectorizing the "checking" loops (that check for the right
> > > result) in gcc.target/powerpc/vsx-vectorize-*.c, thus skewing the
> > > expected counts of various scan-dump-times tests (causing them to FAIL).
> > > This adds #pragma GCC novector above the relevant loops to prevent them
> > > from being vectorized, thereby fixing the test failures.
> >
> > It is fundamentally incorrect to add a "novector" attribute to testcases
> > called "vsx-vectorize-*". Even if it is just to some part of the code;
> > if you want to only test some part of code, put it in a different file.
> >
> > If the testcases are less than stellar (and pretty much anything using
> > scan-assembler-times is!), fix *that*?
>
> All the 8 testcases have
> /* Taken from vect/vect-95.c. */
> comment and
> gcc.dg/vect/vect-95.c test uses that pragma already:
Yeah. But that has very unpredictable results *in the best case*.
And in any acceptable case any such thing will be documented to say what
it is for! No surprising thing is ever obvious.
> __attribute__ ((noinline))
> void bar (float *pd, float *pa, float *pb, float *pc)
> {
> int i;
>
> /* check results: */
> #pragma GCC novector
> for (i = 0; i < N; i++)
> {
> if (pa[i] != (pb[i] * pc[i]))
> abort ();
> if (pd[i] != 5.0)
> abort ();
> }
>
> return;
> }
>
> That said, unlike gcc.dg/vect/vect-95.c test, all these 8 are
> /* { dg-do compile } */
> test, so I fail to understand why they have definitions (rather than
> declarations) of bar and main.
Plain inertia?
No tests become good tests without effort. And tests that are not good
tests require constant maintenance!
> Or perhaps vsx-vectorize-*.c could
> be dg-do compile and with just bar declaration and main1 definition,
> and then have 8 further tests that include those and include the 2 headers,
> define bar and main and are dg-do run rather than dg-do compile and don't
> scan any dumps.
It isn't very clear what to decide to do, all those tests (like so very
many other tests) do not contain the by far most important piece of
information that should be there in any test: they do not say what they
intend to test!
So if anything happens (like, unrelated changes caused the instruction
counts here to change, a very common occurrence with
scan-assembler-times tests, those things are usually inherently
unstable), it won't clear to anyone what made the test fail, or how to
make it pass again.
Adding intrinsics (like pragmas) without considering how to make the
testcases actually better (instead of merley seemingly passing again)
is not the way forward.
Segher
