Re: [PATCH] [PATCH, rs6000] Fix pr79941 (v2)

[Will Schmidt <will_schmidt at vnet dot ibm dot com>]

On Thu, 2017-03-09 at 17:24 -0600, Segher Boessenkool wrote:
> Hi Will,
> 
> On Thu, Mar 09, 2017 at 10:52:52AM -0600, Will Schmidt wrote:
> > Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those
> > entries were added to the intrinsics-to-be-folded list where the generic
> > multiplies should have been instead.  Test coverage in place was for the
> > generic multiplies, and this was missed by my testing.
> > 
> > The mul[eo]* unsigned operations were missing entries in builtin_function_type()
> > to indicate the overloaded arguments were unsigned.  This is corrected here,
> > and those operations now fold accurately to the desired instruction.
> > 
> > Testcases have been added for the mule/mulo operations, as well as a dg-do
> > run test based on the testcase in the PR.  I'll note that the variables in that
> > test case are now marked as volatile so they are not entirely optimized out during
> > the compile.
> > 
> > OK for trunk?
> > regtest is currently running on ppc64*.
> 
> 
> 
> >      PR target/79941
> >      * config/rs6000/rs6000.c (builtin_function_type): Add VMUL*UH
> >      entries to the case statement that marks unsigned arguments to
> >      overloaded functions.
> 
> UH and UB?

Yes.  I've updated that to read "Add VMUL*U[HB]" in my local copy.

> 
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c
> > @@ -0,0 +1,38 @@
> > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with char
> > +   inputs produce the right results.  */
> > +
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> > +/* { dg-options "-maltivec -O2 " } */
> 
>                                  ^ this space looks off

It is. fixed locally in both *mule-char.c and *mule-short.c testcases.


> 
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c
> > @@ -0,0 +1,61 @@
> > +/* PR target/79941 */
> > +
> > +/* { dg-do run } */
> > +/* { dg-require-effective-target powerpc_vsx_ok } */
> > +/* { dg-options "-mvsx -O2 -save-temps" } */
> 
> I think that -save-temps is a leftover from testing?  It shouldn't be there?

Because this is a "dg-do run", and I am also doing a scan-assembler
stanza at the end, I needed that to preserve the intermediate.   I can
rework things if this is not considered proper.. :-) 

> 
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c
> > @@ -0,0 +1,37 @@
> > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with short
> > +   inputs produce the right results.  */
> > +
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> > +/* { dg-options "-maltivec -O2 " } */
> 
>                                  ^ another

yup, got it updated locally now, thanks.  

> 
> Okay with those nits fixed, but let's hear if Mike remembers anything first.

yup.

> 
> Thanks,
> 
> 
> Segher
>