[Bug tree-optimization/105940] suggested_unroll_factor applying place looks wrong

linkw at gcc dot gnu.org gcc-bugzilla@gcc.gnu.org
Fri Jun 17 10:56:59 GMT 2022


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105940

--- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #6)
> (In reply to Kewen Lin from comment #4)
> > (In reply to Richard Biener from comment #2)
> > > (In reply to Kewen Lin from comment #1)
> > > > Created attachment 53126 [details]
> > > > move_applying
> > > 
> > > LGTM (maybe the suggested unroll factor should be only applied if the
> > > suggestion was from a matching with/without SLP analysis, or in fact
> > > vect_analyze_loop_1 should communicate that down - disabling SLP when
> > > the one suggesting unrolling did the re-analysis).
> > 
> > Oops, just noticed the nice suggestion.  Will make a follow up patch for
> > this.
> > It would looks like:
> >   when working out suggested unroll factor, save slp decision into one
> > passed down variable from vect_analyze_loop_1.
> >   when applying suggested unroll factor, if the save slp is false, directly
> > ignore slp handlings, otherwise, go the normal slp path but won't start over
> > for slp off.
> 
> I tested one patch which was bootstrapped and regress-tested on x86, aarch64
> and powerpc64le, but found some failures on SPEC2017 run with rs6000
> hackings, the reason to fail is that we can save reduction chain in
> vect_is_simple_reduction for further analysis in SLP detection, if we
> aggressively skip SLP related analyses in vect_analyze_loop_2, there is ICE
> in vectorizable_reduction
> 
>     if (REDUC_GROUP_FIRST_ELEMENT (stmt_info))
>       gcc_assert (slp_node
>                   && REDUC_GROUP_FIRST_ELEMENT (stmt_info) == stmt_info);
> 
> There seem to be some alternatives:
>   1) check if applying_suggested_uf && !slp_done_for_suggested_uf initially
> in vect_analyze_loop_2, if yes, pass slp disabled information down to
> vect_is_simple_reduction, not to save reduction chain for later slp analysis
> (not existed).
>   2) before we are going to do the slp related analyses (vect_analyze_slp
> etc.), check if applying_suggested_uf && !slp_done_for_suggested_uf, if yes,
> dissolve LOOP_VINFO_REDUCTION_CHAINS(loop_info).
>   3) for the case applying_suggested_uf && !slp_done_for_suggested_uf, we
> still let it go through slp related analyses but not applying suggested
> unroll factor, only applying for its retry without slp.
> 
> 3) is simple but seems to be the worst since we still do some useless
> analyses.
> 1) can save the reduction chain handlings, seems to be the best, not sure if
> it's too intrusive, and if there are some similar needs (in future) to do so.
> 2) is similar to 1), it's to add one common place to undo those previous
> analysis results which are only for slp analyses and can cause unexpected
> result if we don't undo it.
> 
> Any suggestions on this?

Sent out one tested patch for 1) at
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596788.html


More information about the Gcc-bugs mailing list