[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315 --- Comment #15 from CVS Commits --- The master branch has been updated by Richard Biener : https://gcc.gnu.org/g:84935c9822183ce403bb361c5f405711b9a808c6 commit r11-408-g84935c9822183ce403bb361c5f405711b9a808c6 Author: Richard Biener Date: Wed Apr 15 12:09:01 2020 +0200 tree-optimization/33315 - common stores during sinking This implements commoning of stores to a common successor in a simple ad-hoc way. I've decided to put it into the code sinking pass since, well, it sinks stores. It's still separate since it does not really sink code into less executed places. It's ad-hoc since it does not perform any dataflow or alias analysis but simply only considers trailing stores in a block, iteratively though. If the stores are from different values a PHI node is inserted to merge them. gcc.dg/tree-ssa/split-path-7.c shows that path splitting will eventually undo this very transform, I've decided to not bother with it and simply disable sinking for the particular testcase. Doing this transform is good for code size when the stores are from constants, once we have to insert PHIs the situation becomes less clear but it's a transform we do elsewhere as well (cselim for one), and reversing the transform should be easy. 2020-05-15 Richard Biener PR tree-optimization/33315 * tree-ssa-sink.c: Include tree-eh.h. (sink_stats): Add commoned member. (sink_common_stores_to_bb): New function implementing store commoning by sinking to the successor. (sink_code_in_bb): Call it, pass down TODO_cleanup_cfg returned. (pass_sink_code::execute): Likewise. Record commoned stores in statistics. * gcc.dg/tree-ssa/ssa-sink-13.c: New testcase. * gcc.dg/tree-ssa/ssa-sink-14.c: Likewise. * gcc.dg/tree-ssa/split-path-7.c: Disable sinking.
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315 Richard Biener changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #16 from Richard Biener --- Fixed on trunk. Individual missed cases should be tracked by separate bugreports.
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315 --- Comment #14 from Richard Biener --- (In reply to Richard Biener from comment #13) > (In reply to Richard Biener from comment #12) > > Created attachment 48279 [details] > > patch > > > > Patch forward ported to current trunk. > > Surprisingly small fallout: > > FAIL: gcc.dg/tree-ssa/split-path-7.c scan-tree-dump-times split-paths > "Duplicating join block" 0 > > I've thought the approach is too simplistic but I'll do some statistics > over bootstrap in stage1 and push it if it looks useful - we certainly > have more "hackish" optimization pieces. stage3 gcc/ files show (first column is the number of edges we sink from, second column the number of stores we sink): 2: 3195 3: 193 4: 130 5: 55 6: 28 7: 4 8: 9 9: 3 10: 3 12: 1 13: 6 16: 2 18: 1 22: 2 so it seems worth it.
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315 --- Comment #13 from Richard Biener --- (In reply to Richard Biener from comment #12) > Created attachment 48279 [details] > patch > > Patch forward ported to current trunk. Surprisingly small fallout: FAIL: gcc.dg/tree-ssa/split-path-7.c scan-tree-dump-times split-paths "Duplicating join block" 0 I've thought the approach is too simplistic but I'll do some statistics over bootstrap in stage1 and push it if it looks useful - we certainly have more "hackish" optimization pieces.
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315 --- Comment #12 from Richard Biener --- Created attachment 48279 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48279&action=edit patch Patch forward ported to current trunk.
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315
Jeffrey A. Law changed:
What|Removed |Added
CC||law at redhat dot com
--- Comment #11 from Jeffrey A. Law ---
We're closer now than before:
test ()
{
int i;
[local count: 1073741824]:
i_8 = num;
if (i_8 == 1)
goto ; [34.00%]
else
goto ; [66.00%]
[local count: 365072220]:
MEM[(int *)&a] = 0;
MEM[(int *)&a + 4B] = 0;
[local count: 1073741824]:
MEM[(int *)&a + 8B] = 0;
return;
[local count: 73016]:
MEM[(int *)&a] = 0;
MEM[(int *)&a + 4B] = 0;
goto ; [100.00%]
}
I wonder if the block hashing stuff from ICF would allow us to determine that
bb3 and bb5 are equivalent. That in turn would allow us to recognize the test
in bb2 isn't actually needed.
It was always my hope that the ICF infrastructure could be used in this way.
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315
Richard Biener changed:
What|Removed |Added
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot
gnu.org
--- Comment #10 from Richard Biener ---
I've posted a patch for this in July (but it had fallout, as expected...). IL
after that fix:
test ()
{
int i;
[100.00%]:
i_8 = num;
MEM[(int *)&a] = 0;
MEM[(int *)&a + 4B] = 0;
MEM[(int *)&a + 8B] = 0;
return;
}
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315 Richard Biener changed: What|Removed |Added Status|RESOLVED|NEW Blocks||16996 Resolution|FIXED |--- --- Comment #9 from Richard Biener --- There are still two stores to a[0] and two to a[1]. Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16996 [Bug 16996] [meta-bug] code size improvements
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315
Martin Sebor changed:
What|Removed |Added
Status|NEW |RESOLVED
CC||msebor at gcc dot gnu.org
Resolution|--- |FIXED
--- Comment #8 from Martin Sebor ---
With today's top of trunk (GCC 7.0) I see the following in the optimized dump.
The redundant stores seem to have been eliminated (they are still present in
GCC 6). Therefore resolving as fixed (please reopen if I missed something).
test ()
{
int i;
[100.00%]:
i_8 = num;
if (i_8 == 1)
goto ; [34.00%]
else
goto ; [66.00%]
[34.00%]:
MEM[(int *)&a] = 0;
goto ; [100.00%]
[34.00%]:
MEM[(int *)&a + 4B] = 0;
[100.00%]:
MEM[(int *)&a + 8B] = 0;
return;
[66.00%]:
MEM[(int *)&a] = 0;
if (i_8 == 2)
goto ; [51.52%]
else
goto ; [48.48%]
[32.00%]:
MEM[(int *)&a + 4B] = 0;
goto ; [100.00%]
}
[Bug tree-optimization/33315] stores not commoned by sinking
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315
Richard Biener changed:
What|Removed |Added
CC||rguenth at gcc dot gnu.org
Component|middle-end |tree-optimization
Summary|If condition not getting|stores not commoned by
|eliminated |sinking
--- Comment #7 from Richard Biener ---
comment#4 is long fixed (in DOM). In the original testcase the redundant
conditons are eliminated but we retain
test ()
{
int i;
:
i_8 = num;
if (i_8 == 1)
goto ;
else
goto ;
:
MEM[(int *)&a] = 0;
MEM[(int *)&a + 4B] = 0;
goto ;
:
MEM[(int *)&a + 4B] = 0;
:
MEM[(int *)&a + 8B] = 0;
return;
:
MEM[(int *)&a] = 0;
if (i_8 == 2)
goto ;
else
goto ;
:
MEM[(int *)&a + 4B] = 0;
goto ;
because nothing sinks the common stores and tail-merging does only obfuscate
the CFG. So this didn't really have sth to do with PR23286. But the
complaint that the if condition is not eliminated is no longer true.
transmogrifying bug.
