| Message ID | 50507644.7090503@st.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 12, 2012 at 4:47 AM, Christian Bruel <christian.bruel@st.com> wrote: > The problem stems from tree-ssa-tail-merge that breaks bb->count, The > CFG looks like > > 2 > / \ > / 6 > 5 (0) | > | 3 <----- > | / \ | > | 7 (1) 8 - > | / > 4 (1) > > (in parenthesis the bb->count from gcov) > > 2 > / \ > / 6 > / | > | 3 <-- > | / | | > 5 (0) 8 -- > | > | > 4 (1) > > so 5 and 4 are now in different partitions, producing an assertion > because there is no EDGE_CROSSING between them. > > I can see 3 solutions to this > > 1) merge the BB counts in tree-ssa-tail-merge.c, so 5 is in the same > partition that 4 This looks correct as we already do that for the frequency. > > 2) don't tail-merge blocks that belong to different partitions. I don't think you detect this on the tree level as the partitioning has not happened yet. Thanks, Andrew Pinski > > 3) add a EDGE_CROSSING flag on the edge between 4 and 5. > > 1) fixes the problem, so 5 and 4 are now in the same partition. The fix > is quite trivial, as with attached. > > the other solution 2) is more conservative, and also fixes the problem. > > I don't think 3) is necessary. > > more ideas ? > > thanks, > > Christian > > > On 09/11/2012 06:21 PM, Jakub Jelinek wrote: >> On Tue, Sep 11, 2012 at 05:40:30PM +0200, Steven Bosscher wrote: >>> On Tue, Sep 11, 2012 at 5:31 PM, Christian Bruel <christian.bruel@st.com> wrote: >>>> Actually, the edge is fairly simple. I have >>>> >>>> BB5 (BB_COLD_PARTITION) -> BB10 (BB_HOT_PARTITION) -> EXIT >>>> >>>> and BB10 has no other incoming edges. and we are duplicating it. >>> >>> That is wrong, should never happen. Is there a test case to play with? >>> It'd be good to have a PR for this. >> >> Isn't that the standard case when !HAVE_return ? Then you can have only a >> single return through epilogue, and when the epilogue is in the hot >> partition, even if cold code is returning, it needs to jump to the epilogue. >> >> Jakub >>
>> 1) fixes the problem, so 5 and 4 are now in the same partition. The fix >> is quite trivial, as with attached. > > That looks obviously correct to me. I can't approve it, but I'd have > committed it as obvious. > Thanks, I'll make the formal request, after checking forthe unexpected side effects > > >> I don't think 3) is necessary. > > I think it *is* necessary. From cfg-flags.def: > /* Edge crosses between hot and cold sections, when we do partitioning. > This flag is only used for the RTL CFG. */ > DEF_EDGE_FLAG(CROSSING, 11) > > Edge 5-4 is a crossing edge, so EDGE_CROSSING should be set. OK, I'll try to investigate this while it's hot and before the symptom disappears after committing the bb->count... that would still be latent somewhere. thanks Christian > > Ciao! > Steven >
Index: tree-ssa-tail-merge.c =================================================================== --- tree-ssa-tail-merge.c (revision 191129) +++ tree-ssa-tail-merge.c (working copy) @@ -1478,6 +1478,8 @@ bb2->frequency = BB_FREQ_MAX; bb1->frequency = 0; + bb2->count += bb1->count; + /* Do updates that use bb1, before deleting bb1. */ release_last_vdef (bb1); same_succ_flush_bb (bb1);
The problem stems from tree-ssa-tail-merge that breaks bb->count, The CFG looks like 2 / \ / 6 5 (0) | | 3 <----- | / \ | | 7 (1) 8 - | / 4 (1) (in parenthesis the bb->count from gcov) 2 / \ / 6 / | | 3 <-- | / | | 5 (0) 8 -- | | 4 (1) so 5 and 4 are now in different partitions, producing an assertion because there is no EDGE_CROSSING between them. I can see 3 solutions to this 1) merge the BB counts in tree-ssa-tail-merge.c, so 5 is in the same partition that 4 2) don't tail-merge blocks that belong to different partitions. 3) add a EDGE_CROSSING flag on the edge between 4 and 5. 1) fixes the problem, so 5 and 4 are now in the same partition. The fix is quite trivial, as with attached. the other solution 2) is more conservative, and also fixes the problem. I don't think 3) is necessary. more ideas ? thanks, Christian On 09/11/2012 06:21 PM, Jakub Jelinek wrote: > On Tue, Sep 11, 2012 at 05:40:30PM +0200, Steven Bosscher wrote: >> On Tue, Sep 11, 2012 at 5:31 PM, Christian Bruel <christian.bruel@st.com> wrote: >>> Actually, the edge is fairly simple. I have >>> >>> BB5 (BB_COLD_PARTITION) -> BB10 (BB_HOT_PARTITION) -> EXIT >>> >>> and BB10 has no other incoming edges. and we are duplicating it. >> >> That is wrong, should never happen. Is there a test case to play with? >> It'd be good to have a PR for this. > > Isn't that the standard case when !HAVE_return ? Then you can have only a > single return through epilogue, and when the epilogue is in the hot > partition, even if cold code is returning, it needs to jump to the epilogue. > > Jakub >