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 } } } */ > > > >