public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "linkw at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/105940] suggested_unroll_factor applying place looks wrong
Date: Thu, 16 Jun 2022 13:33:34 +0000	[thread overview]
Message-ID: <bug-105940-4-bLRxsSbrUE@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-105940-4@http.gcc.gnu.org/bugzilla/>

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?

  parent reply	other threads:[~2022-06-16 13:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-13  5:59 [Bug tree-optimization/105940] New: " linkw at gcc dot gnu.org
2022-06-13  6:07 ` [Bug tree-optimization/105940] " linkw at gcc dot gnu.org
2022-06-13  6:08 ` linkw at gcc dot gnu.org
2022-06-13  8:25 ` rguenth at gcc dot gnu.org
2022-06-14  5:58 ` cvs-commit at gcc dot gnu.org
2022-06-14  8:46 ` linkw at gcc dot gnu.org
2022-06-15 10:13 ` cvs-commit at gcc dot gnu.org
2022-06-16 13:33 ` linkw at gcc dot gnu.org [this message]
2022-06-16 13:35 ` linkw at gcc dot gnu.org
2022-06-17 10:56 ` linkw at gcc dot gnu.org
2022-06-20 12:45 ` cvs-commit at gcc dot gnu.org
2022-06-23  1:59 ` cvs-commit at gcc dot gnu.org
2022-06-23  2:25 ` cvs-commit at gcc dot gnu.org
2022-06-23  3:20 ` linkw at gcc dot gnu.org
2022-06-23  6:28 ` rguenther at suse dot de

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-105940-4-bLRxsSbrUE@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).