Adjust vect-widen-mult-const-[su]16.c for r226675
diff mbox

Message ID CAFiYyc248sMnb2wNnOiefK6WyZPWyd8K7TV_WJ88q14iu1Okyg@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener Dec. 7, 2015, 10:50 a.m. UTC
On Fri, Dec 4, 2015 at 8:51 PM, Bill Schmidt
<wschmidt@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)


help?

Richard.

> Thanks,
> Bill
>
>
> [gcc/testsuite]
>
> 2015-12-04  Bill Schmidt  <wschmidt@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 } } } */
>
>
>
>

Comments

Bill Schmidt Dec. 7, 2015, 5:21 p.m. UTC | #1
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.

Bill

On Mon, 2015-12-07 at 11:50 +0100, Richard Biener wrote:
> On Fri, Dec 4, 2015 at 8:51 PM, Bill Schmidt
> <wschmidt@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  <wschmidt@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 } } } */
> >
> >
> >
> >
>
Richard Biener Dec. 7, 2015, 6:17 p.m. UTC | #2
On December 7, 2015 6:21:36 PM GMT+01:00, Bill Schmidt <wschmidt@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
>> <wschmidt@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  <wschmidt@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 } } } */
>> >
>> >
>> >
>> >
>>
Bill Schmidt Dec. 7, 2015, 7:50 p.m. UTC | #3
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 <wschmidt@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
> >> <wschmidt@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  <wschmidt@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 } } } */
> >> >
> >> >
> >> >
> >> >
> >> 
> 
>

Patch
diff mbox

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;
            }
        }
     }