From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id A906E3853555; Thu, 16 Jun 2022 13:33:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A906E3853555 From: "linkw at gcc dot 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 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: tree-optimization X-Bugzilla-Version: 13.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: linkw at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: linkw at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gcc-bugs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-bugs mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Jun 2022 13:33:34 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D105940 --- Comment #6 from Kewen Lin --- (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 > >=20 > > 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). >=20 > 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, direct= ly > ignore slp handlings, otherwise, go the normal slp path but won't start o= ver > 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 hackin= gs, 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) =3D=3D 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 s= till 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 analy= ses. 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 s= o. 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?=