On Mon, 2015-12-07 at 19:17 +0100, Richard Biener wrote:
> On December 7, 2015 6:21:36 PM GMT+01:00, Bill Schmidt
> <wschm...@linux.vnet.ibm.com> wrote:
> >Hi Richi,
> >
> >I was afraid this would break X86. Unfortunately, your proposed patch
> >didn't change any output for me. Still seeing 6 and 8 instances of
> >"pattern recognized", unfortunately.
>
>
> Hmm, can you open a PR and attach vectorizer dumps?
PR68776. Thanks!
Bill
>
> Thanks,
> Richard.
> >Bill
> >
> >On Mon, 2015-12-07 at 11:50 +0100, Richard Biener wrote:
> >> On Fri, Dec 4, 2015 at 8:51 PM, Bill Schmidt
> >> <wschm...@linux.vnet.ibm.com> wrote:
> >> > Since r226675, we have been seeing these failures:
> >> >
> >> > FAIL: gcc.dg/vect/vect-widen-mult-const-s16.c -flto
> >-ffat-lto-objects
> >> > scan-tree-dump-times vect "pattern recognized" 2
> >> > FAIL: gcc.dg/vect/vect-widen-mult-const-s16.c scan-tree-dump-times
> >vect
> >> > "pattern recognized" 2
> >> > FAIL: gcc.dg/vect/vect-widen-mult-const-u16.c -flto
> >-ffat-lto-objects
> >> > scan-tree-dump-times vect "pattern recognized" 2
> >> > FAIL: gcc.dg/vect/vect-widen-mult-const-u16.c scan-tree-dump-times
> >vect
> >> > "pattern recognized" 2
> >> >
> >> > Comparing the vect-details dumps from r226674 to r226675, I see
> >these as
> >> > the reason:
> >> >
> >> > 63a64,66
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:16:3:
> >note: vect_recog_mult_pattern: detected:
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:16:3:
> >note: patt_47 = _6 << 2;
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:16:3:
> >note: pattern recognized: patt_47 = _6 << 2;
> >> > 70a74,76
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:16:3:
> >note: vect_recog_mult_pattern: detected:
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:16:3:
> >note: patt_40 = _6 << 1;
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:16:3:
> >note: pattern recognized: patt_40 = _6 << 1;
> >> >
> >> > 747a754,756
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:31:3:
> >note: vect_recog_mult_pattern: detected:
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:31:3:
> >note: patt_47 = _6 << 2;
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:31:3:
> >note: pattern recognized: patt_47 = _6 << 2;
> >> > 754a764,766
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:31:3:
> >note: vect_recog_mult_pattern: detected:
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:31:3:
> >note: patt_40 = _6 << 1;
> >> >>
> >/home/wschmidt/gcc/gcc-mainline-base/gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c:31:3:
> >note: pattern recognized: patt_40 = _6 << 1;
> >> >
> >> > These seems precisely what's expected, given the nature of the
> >patch,
> >> > which is looking for these opportunities. So it's likely that we
> >should
> >> > just change
> >> >
> >> > /* { dg-final { scan-tree-dump-times "pattern recognized" 2
> >> > "vect" { target vect_widen_mult_hi_to_si_pattern } } } */
> >> >
> >> > to
> >> >
> >> > /* { dg-final { scan-tree-dump-times "pattern recognized" 6
> >> > "vect" { target vect_widen_mult_hi_to_si_pattern } } } */
> >> >
> >> > and similarly for the unsigned case. The following patch does
> >this.
> >> > However, I wanted to run this by Venkat since this was apparently
> >not
> >> > detected when his patch went in. This doesn't appear to be a
> >> > target-specific issue, and most targets support
> >> > vect_widen_mult_hi_to_si_pattern, so I'm not sure why this wasn't
> >fixed
> >> > with the original patch. Will this change break on any other
> >targets
> >> > for some reason?
> >> >
> >> > Tested on powerpc64le-unknown-linux-gnu. Ok for trunk?
> >>
> >> Hmm. That will FAIL on x86_64 though because it can handle
> >multiplication
> >> natively. I think the pattern recognition is simply bogus as it
> >fails to detect
> >> the stmt is already part of the widen-mult pattern? In fact, pattern
> >> recognition
> >> looping over all pattern functions even if one already matched on the
> >very
> >> same stmt looks bogus to me.
> >>
> >> Does the (untested)
> >>
> >> Index: gcc/tree-vect-patterns.c
> >> ===================================================================
> >> --- gcc/tree-vect-patterns.c (revision 231357)
> >> +++ gcc/tree-vect-patterns.c (working copy)
> >> @@ -3791,7 +3791,7 @@ vect_mark_pattern_stmts (gimple *orig_st
> >> This function also does some bookkeeping, as explained in the
> >documentation
> >> for vect_recog_pattern. */
> >>
> >> -static void
> >> +static bool
> >> vect_pattern_recog_1 (vect_recog_func_ptr vect_recog_func,
> >> gimple_stmt_iterator si,
> >> vec<gimple *> *stmts_to_replace)
> >> @@ -3809,7 +3809,7 @@ vect_pattern_recog_1 (vect_recog_func_pt
> >> stmts_to_replace->quick_push (stmt);
> >> pattern_stmt = (* vect_recog_func) (stmts_to_replace, &type_in,
> >&type_out);
> >> if (!pattern_stmt)
> >> - return;
> >> + return false;
> >>
> >> stmt = stmts_to_replace->last ();
> >> stmt_info = vinfo_for_stmt (stmt);
> >> @@ -3831,13 +3831,13 @@ vect_pattern_recog_1 (vect_recog_func_pt
> >> /* Check target support */
> >> type_in = get_vectype_for_scalar_type (type_in);
> >> if (!type_in)
> >> - return;
> >> + return false;
> >> if (type_out)
> >> type_out = get_vectype_for_scalar_type (type_out);
> >> else
> >> type_out = type_in;
> >> if (!type_out)
> >> - return;
> >> + return false;
> >> pattern_vectype = type_out;
> >>
> >> if (is_gimple_assign (pattern_stmt))
> >> @@ -3853,7 +3853,7 @@ vect_pattern_recog_1 (vect_recog_func_pt
> >> if (!optab
> >> || (icode = optab_handler (optab, vec_mode)) ==
> >CODE_FOR_nothing
> >> || (insn_data[icode].operand[0].mode != TYPE_MODE
> >(type_out)))
> >> - return;
> >> + return false;
> >> }
> >>
> >> /* Found a vectorizable pattern. */
> >> @@ -3892,6 +3892,8 @@ vect_pattern_recog_1 (vect_recog_func_pt
> >>
> >> vect_mark_pattern_stmts (stmt, pattern_stmt, NULL_TREE);
> >> }
> >> +
> >> + return true;
> >> }
> >>
> >>
> >> @@ -4005,8 +4007,9 @@ vect_pattern_recog (vec_info *vinfo)
> >> for (j = 0; j < NUM_PATTERNS; j++)
> >> {
> >> vect_recog_func = vect_vect_recog_func_ptrs[j];
> >> - vect_pattern_recog_1 (vect_recog_func, si,
> >> - &stmts_to_replace);
> >> + if (vect_pattern_recog_1 (vect_recog_func, si,
> >> + &stmts_to_replace))
> >> + break;
> >> }
> >> }
> >> }
> >> @@ -4026,8 +4029,9 @@ vect_pattern_recog (vec_info *vinfo)
> >> for (j = 0; j < NUM_PATTERNS; j++)
> >> {
> >> vect_recog_func = vect_vect_recog_func_ptrs[j];
> >> - vect_pattern_recog_1 (vect_recog_func, si,
> >> - &stmts_to_replace);
> >> + if (vect_pattern_recog_1 (vect_recog_func, si,
> >> + &stmts_to_replace))
> >> + break;
> >> }
> >> }
> >> }
> >>
> >> help?
> >>
> >> Richard.
> >>
> >> > Thanks,
> >> > Bill
> >> >
> >> >
> >> > [gcc/testsuite]
> >> >
> >> > 2015-12-04 Bill Schmidt <wschm...@linux.vnet.ibm.com>
> >> >
> >> > * gcc.dg/vect/vect-widen-mult-const-s16.c: Change number of
> >> > occurrences of "pattern recognized" to 6.
> >> > * gcc.dg/vect/vect-widen-mult-const-u16.c: Likewise.
> >> >
> >> >
> >> > Index: gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c
> >> > ===================================================================
> >> > --- gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c
> >(revision 231278)
> >> > +++ gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-s16.c
> >(working copy)
> >> > @@ -56,5 +56,5 @@ int main (void)
> >> >
> >> > /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect"
> >{ target vect_widen_mult_hi_to_si } } } */
> >> > /* { dg-final { scan-tree-dump-times
> >"vect_recog_widen_mult_pattern: detected" 2 "vect" { target
> >vect_widen_mult_hi_to_si_pattern } } } */
> >> > -/* { dg-final { scan-tree-dump-times "pattern recognized" 2 "vect"
> >{ target vect_widen_mult_hi_to_si_pattern } } } */
> >> > +/* { dg-final { scan-tree-dump-times "pattern recognized" 6 "vect"
> >{ target vect_widen_mult_hi_to_si_pattern } } } */
> >> >
> >> > Index: gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-u16.c
> >> > ===================================================================
> >> > --- gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-u16.c
> >(revision 231278)
> >> > +++ gcc/testsuite/gcc.dg/vect/vect-widen-mult-const-u16.c
> >(working copy)
> >> > @@ -73,4 +73,4 @@ int main (void)
> >> >
> >> > /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 3 "vect"
> >{ target vect_widen_mult_hi_to_si } } } */
> >> > /* { dg-final { scan-tree-dump-times
> >"vect_recog_widen_mult_pattern: detected" 2 "vect" { target
> >vect_widen_mult_hi_to_si_pattern } } } */
> >> > -/* { dg-final { scan-tree-dump-times "pattern recognized" 2 "vect"
> >{ target vect_widen_mult_hi_to_si_pattern } } } */
> >> > +/* { dg-final { scan-tree-dump-times "pattern recognized" 8 "vect"
> >{ target vect_widen_mult_hi_to_si_pattern } } } */
> >> >
> >> >
> >> >
> >> >
> >>
>
>
