Hi!
On Fri, Aug 13, 2021 at 10:34:46AM +0800, Kewen.Lin wrote:
> on 2021/8/12 下午11:10, Segher Boessenkool wrote:
> >> + && VECTOR_UNIT_ALTIVEC_OR_VSX_P (in_vmode))
> >> + {
> >> + machine_mode exp_mode = DImode;
> >> + machine_mode exp_vmode = V2DImode;
> >> + enum rs6000_builtins vname = RS6000_BUILTIN_COUNT;
> >
> > "name"? This should be "bif" or similar?
>
> Updated with name.
No, I meant "name" has no meaning other than it is wrong here :-)
It is an enum for which builtin to use here. It has nothing to do with
a name. So it could be "enum rs6000_builtins bif" or whatever you want;
short variable names are *good*, for many reasons, but they should not
egregiously lie :-)
> >> +/* { dg-do run } */
> >> +/* { dg-require-effective-target lp64 } */
> >
> > Same here. I suppose this uses builtins that do not exist on 32-bit?
>
> Yeah, those bifs which are guarded with lp64 in their cases are only
> supported on 64-bit environment.
It is a pity we cannot use "powerpc64" here (that selector does not test
what you would/could/should hope it tests... Maybe someone can fix it
some day? The only real blocker to that is fixing up the current users
of it, the rest is easy).
Expanding a bit... You would expect (well, I do! I did patches
expecting this several times) this to mean "powerpc64_ok", but it in
fact means "powerpc64_hw". Maybe we should have selectors with those
two names, and get rid of the current "powerpc64"?
> >> +#define CHECK(name)
> >> \
> >> + __attribute__ ((optimize (1))) void check_##name ()
> >> \
> >
> > What is the attribute for, btw? It seems fragile, but perhaps I do not
> > understand the intention.
>
> It's to stop compiler from optimizing check functions with vectorization,
> since the test point is to compare the results between scalar and vectorized
> version.
So, add a comment for this as well please.
In general, in testcases you can do the dirtiest things, no problems at
all, just document what you do why :-)
> Thanks, v2 has been attached by addressing Bill's and your comments. :)
Looks good. Just fix that "name" thing, and it is okay for trunk.
Thanks!
Segher
