This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Adjust vect-widen-mult-const-[su]16.c for r226675
- From: Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenth at gcc dot gnu dot org>, venkataramanan dot kumar dot gnu at gmail dot com
- Date: Mon, 07 Dec 2015 11:21:36 -0600
- Subject: Re: [PATCH] Adjust vect-widen-mult-const-[su]16.c for r226675
- Authentication-results: sourceware.org; auth=none
- References: <1449258677 dot 3141 dot 7 dot camel at gnopaine> <CAFiYyc248sMnb2wNnOiefK6WyZPWyd8K7TV_WJ88q14iu1Okyg at mail dot gmail dot com>
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 } } } */
> >
> >
> >
> >
>