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

--- Comment #6 from Kewen Lin <linkw at gcc dot gnu.org> ---
(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?

Reply via email to