On Wed, 2021-01-27 at 18:24 -0600, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Oct 26, 2020 at 04:22:32PM -0500, will schmidt wrote:
> > Per PR91903, GCC ICEs when we attempt to pass a variable
> > (or out of range value) into the vec_ctf() builtin. Per
> > investigation, the parameter checking exists for this
> > builtin with the int types, but was missing for
> > the long long types.
> >
> > This patch adds the missing CODE_FOR_* entries to the
> > rs6000_expand_binup_builtin to cover that scenario.
> > This patch also updates some existing tests to remove
> > calls to vec_ctf() and vec_cts() that contain negative
> > values.
> > --- a/gcc/testsuite/gcc.target/powerpc/builtins-1.fold.h
> > +++ b/gcc/testsuite/gcc.target/powerpc/builtins-1.fold.h
> > @@ -212,14 +212,14 @@ int main ()
> > extern vector unsigned long long u9; u9 = vec_mergeo (u3, u4);
> >
> > extern vector long long l8; l8 = vec_mul (l3, l4);
> > extern vector unsigned long long u6; u6 = vec_mul (u3, u4);
> >
> > - extern vector double dh; dh = vec_ctf (la, -2);
> > + extern vector double dh; dh = vec_ctf (la, 2);
> > extern vector double di; di = vec_ctf (ua, 2);
> > extern vector int sz; sz = vec_cts (fa, 0x1F);
> > - extern vector long long l9; l9 = vec_cts (dh, -2);
> > + extern vector long long l9; l9 = vec_cts (dh, 2);
>
> I think removing the negative inputs here reduces test coverage? Why
> did you change them, it isn't immediately clear to me?
The vec_ctf() and vec_cts() builtins accept a const int parameter which
should be in the range of 0..31. The PR was initially
written/described as an ICE when a variable was passed into the
builtin, and part of debug/fixups revealed that the testcase negative
values were also invalid.
I'll clarify that in the commit message.
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr91903.c
> > @@ -0,0 +1,74 @@
> > +/* { dg-do compile */
> > +/* { dg-require-effective-target p8vector_hw } */
>
> Compile tests should use p8vector_ok, instead. (We do not care what
> kind of hardware the system under test is: we can run this on a
> cross-
> compiler just fine, after all!)
ok
>
> > +/* { dg-skip-if "" { powerpc*-*-darwin* } } */
>
> Please skip this line. If the test does not work for Darwin Iain can
> easily disable it, but if you do, no one will find out if it does
> work.
ok, sounds good.
>
> Okay for trunk with those things fixed, and the -2 thing looked at.
> Thanks!
>
Thanks for the review. :-)
>
> Segher
