On Tue, 2017-03-07 at 22:52 +0100, Jakub Jelinek wrote:
> On Tue, Mar 07, 2017 at 03:36:38PM -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.
> >
> > Thusly, remove those entries from the folding list. At the same time, I
> > am adding a testcase to provide some basic coverage for those ops.
> >
> > Functional gimple folding for those operations will be showing up at
> > a later time.
> >
> > OK for trunk?
> > regtest is currently running on ppc64*.
> >
> >
> > gcc:
> > 2017-03-07 Will Schmidt <will_schm...@vnet.ibm.com>
> >
> > PR middle-end/79941
> > * config/rs6000/rs6000.c (gimplify_init_constructor): Remove multiply
> > even and multiply odd (vmule/vmulo) intrinsics from the multiply
> > folding
> > sequence.
> >
> > testsuite:
> > 2017-03-07 Will Schmidt <will_schm...@vnet.ibm.com>
> >
> > PR middle-end/79941
> > * gcc.target/powerpc/fold-vec-mult-even_odd_misc.c: New test.
> > ---
> > gcc/config/rs6000/rs6000.c | 31 -----------
> > .../powerpc/fold-vec-mult-even_odd_misc.c | 58
> > ++++++++++++++++++++
> > 2 files changed, 58 insertions(+), 31 deletions(-)
> > create mode 100644
> > gcc/testsuite/gcc.target/powerpc/fold-vec-mult-even_odd_misc.c
> >
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index 25b10f1..ce0ece5 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -16852,37 +16852,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator
> > *gsi)
> > gsi_replace (gsi, g, true);
> > return true;
> > }
>
Hi Jakub,
> Doesn't the ALTIVEC_BUILTIN_VMUL[EO]S[BH] folding work properly already?
Perhaps by accident. :-)
I have yet to review the generated assembly for the mul[eo] code, so I
don't have a good feel for the health of the generated code at the
moment.
And I freely admit that I am still learning this code, and the VMUL[EO]*
bits were only upstreamed due to error on my part.
If there is any time pressure at all, I would prefer reverting this
chunk entirely. Doing those intrinsics properly is in my queue, but may
or may not end up being during stage4.
> Perhaps you could just comment out the VMUL[EO]U[BH] folding for now,
> or depending on whether there is S or U set uns flag and punt if TYPE_SIGN
> (TREE_TYPE
> (TREE_TYPE (arg0))) is not equal to that (similarly for arg1 and lhs).
> Or VIEW_CONVERT_EXPR it if not the right sign. Or tweak the builtin
> prototype for VMUL[EO]U[BH] so that it uses unsigned vector arguments and
> result.
Thanks for this analysis... this will help.. :-)
Thanks,
-Will
>
> > - /* Even element flavors of vec_mul (signed). */
> > - case ALTIVEC_BUILTIN_VMULESB:
> > - case ALTIVEC_BUILTIN_VMULESH:
> > - /* Even element flavors of vec_mul (unsigned). */
> > - case ALTIVEC_BUILTIN_VMULEUB:
> > - case ALTIVEC_BUILTIN_VMULEUH:
> > - {
> > - arg0 = gimple_call_arg (stmt, 0);
> > - arg1 = gimple_call_arg (stmt, 1);
> > - lhs = gimple_call_lhs (stmt);
> > - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_EVEN_EXPR, arg0,
> > arg1);
> > - gimple_set_location (g, gimple_location (stmt));
> > - gsi_replace (gsi, g, true);
> > - return true;
> > - }
> > - /* Odd element flavors of vec_mul (signed). */
> > - case ALTIVEC_BUILTIN_VMULOSB:
> > - case ALTIVEC_BUILTIN_VMULOSH:
> > - /* Odd element flavors of vec_mul (unsigned). */
> > - case ALTIVEC_BUILTIN_VMULOUB:
> > - case ALTIVEC_BUILTIN_VMULOUH:
> > - {
> > - arg0 = gimple_call_arg (stmt, 0);
> > - arg1 = gimple_call_arg (stmt, 1);
> > - lhs = gimple_call_lhs (stmt);
> > - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_ODD_EXPR, arg0,
> > arg1);
> > - gimple_set_location (g, gimple_location (stmt));
> > - gsi_replace (gsi, g, true);
> > - return true;
> > - }
> > -
> > default:
> > break;
> > }
>
> Jakub
>
