From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 221A73894C1B; Wed, 3 Mar 2021 08:03:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 221A73894C1B From: "rguenther at suse dot de" To: gcc-bugs@gcc.gnu.org Subject: [Bug tree-optimization/94092] Code size and performance degradations after -ftree-loop-distribute-patterns was enabled at -O[2s]+ Date: Wed, 03 Mar 2021 08:03:57 +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: 10.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: rguenther at suse dot de X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: aoliva 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: Wed, 03 Mar 2021 08:03:58 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D94092 --- Comment #12 from rguenther at suse dot de --- On Wed, 3 Mar 2021, bina2374 at gmail dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D94092 >=20 > --- Comment #11 from Mel Chen --- > (In reply to Mel Chen from comment #10) > > (In reply to Richard Biener from comment #9) > > > (In reply to Mel Chen from comment #8) > > > > Sorry for using the bad example to describe the problem I am facing= . Let me > > > > clarify my question with a more precise example. > > > >=20 > > > > void array_mul(int N, int *C, short *A, short *B) { > > > > int i, j; > > > > for (i =3D 0; i < N; i++) { > > > > C[i] =3D 0; // Will be transformed to __builtin_memset > > > > for (j =3D 0; j < N; j++) { > > > > C[i] +=3D (int)A[i * N + j] * (int)B[j]; > > > > } > > > > } > > > > } > > > >=20 > > > > If I compile the case with -O2 -fno-tree-loop-distribute-patterns, = the store > > > > operation 'C[i] =3D 0' can be eliminated by dead store elimination = (dse3). But > > > > without -fno-tree-loop-distribute-patterns, it will be transformed = to memset > > > > by loop distribution (ldist) because ldist executes before dse3. Fi= nally the > > > > memset will not be eliminated. > > > >=20 > > > > Another point is if there are other operations in the same level lo= op as the > > > > store operation, is it really beneficial to do loop distribution an= d then > > > > convert to builtin function? > > >=20 > > > Sure, it shows a cost modeling issue given that usually loop distribu= tion > > > merges partitions which touch the same memory stream (but IIRC maybe = only > > > for loads). But more to the point we're missing to eliminate the dea= d store > > > which should be appearant at least after PRE - LIM2 applied store mot= ion > > > but only PRE elides the resulting load of C[i]. Usually DCE and DSE = come in > > > pairs but after PRE we have DCE, CDDCE w/o accompaning DSE only with = the > > > next DSE only happening after loop distribution. > > >=20 > > > Which means we should eventually do > > >=20 > > > diff --git a/gcc/passes.def b/gcc/passes.def > > > index e9ed3c7bc57..be3a9becde0 100644 > > > --- a/gcc/passes.def > > > +++ b/gcc/passes.def > > > @@ -254,6 +254,7 @@ along with GCC; see the file COPYING3. If not see > > > NEXT_PASS (pass_sancov); > > > NEXT_PASS (pass_asan); > > > NEXT_PASS (pass_tsan); > > > + NEXT_PASS (pass_dse); > > > NEXT_PASS (pass_dce); > > > /* Pass group that runs when 1) enabled, 2) there are loops > > > in the function. Make sure to run pass_fix_loops before > >=20 > > Yes, doing DSE before ldist is a simple and effective way. > > This patch has been verified to be work on coremark. Not only improved > > performance, but also code size. >=20 > The test case gcc.dg/tree-ssa/ldist-33.c is failure after I added DSE. >=20 > /* { dg-do compile { target size32plus } } */ > /* { dg-options "-O2 -ftree-loop-distribution -ftree-loop-distribute-patt= erns > -fdump-tree-ldist-details" } */ >=20 > #define N (1024) > double a[N][N], b[N][N], c[N][N]; >=20 > void > foo (void) > { > unsigned i, j, k; >=20 > for (i =3D 0; i < N; ++i) > for (j =3D 0; j < N; ++j) > { > c[i][j] =3D 0.0; > for (k =3D 0; k < N; ++k) > c[i][j] +=3D a[i][k] * b[k][j]; > } > } >=20 > /* { dg-final { scan-tree-dump "Loop nest . distributed: split to 1 loops= and 1 > library" "ldist" } } */ >=20 > It is similar to the example I showed earlier. DSE eliminated 'c[i][j] = =3D 0.0' > so no loop will be split. My question is how to handle this test case? Add > -fno-tree-dse into dg-options, modify this test case, delete this test ca= se, or > others. I'd say disable store-motion with a comment (-fno-tree-loop-im, we might also finally add a separate switch to control the store-motion part of GIMPLE invariant motion). I think the testcase should show we can recognize the memset in a non-innermost loop (IIRC the above mimics what bwaves does).=