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