Message ID | 504F2DA8.3050208@st.com |
---|---|
State | New |
Headers | show |
> Does this restriction look right to you ? (regression tests are still > running on x86 and sh) Please generate your patches with diff -up (or svn diff -x -up). > + && (BB_PARTITION (e->src) == BB_PARTITION (e->dest)) No need for parentheses around this check. The shrink wrapping code appears to be dealing with partitioning, or at least there are BB_COPY_PARTITIONs further down. So I can't tell whether this fix is correct. Can you show in more detail what happens? (A dotty graph is always helpful ;-) Ciao! Steven
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. My hypothesis, is that with a gcov based profile, we should never have such partitioning on the edges, BB10 should be COLD as well. My suggestion was to avoid shrink-wrapping failing on the block duplication for this case, but that would hide the real cause. I now prefer to understand why BB10 is HOT in the first place... if this is a correct assumption that it should not be. Thanks Christian On 09/11/2012 02:46 PM, Steven Bosscher wrote: >> Does this restriction look right to you ? (regression tests are still >> running on x86 and sh) > > Please generate your patches with diff -up (or svn diff -x -up). > >> + && (BB_PARTITION (e->src) == BB_PARTITION (e->dest)) > > No need for parentheses around this check. > > The shrink wrapping code appears to be dealing with partitioning, or > at least there are BB_COPY_PARTITIONs further down. So I can't tell > whether this fix is correct. Can you show in more detail what happens? > (A dotty graph is always helpful ;-) > > Ciao! > Steven >
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. Ciao! Steven
On 09/11/2012 05:40 PM, 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? Thanks for the confirmation. The case happens on SH only when applying the simple_return patch [PR target/54546] on the bb-reorder test from the testsuite. > It'd be good to have a PR for this. I'll update the PR above with what I find, lets see if this turns out to be target independent. thanks Christian > > Ciao! > Steven >
when running a cfg dump, I get many messages like: Invalid sum of incoming frequencies 1667, should be 3334 So it looks like a profile information was not correctly propagated somewhere. which could lead to such partitioning incoherency. I have no idea for the moment if this is local problem or not, just want to share that in case someone as an input on this. Cheers Christian On 09/11/2012 05:40 PM, 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. > > Ciao! > Steven >
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
Index: function.c =================================================================== --- function.c (revision 191177) +++ function.c (working copy) @@ -6063,6 +6063,7 @@ FOR_EACH_EDGE (e, ei, tmp_bb->preds) if (single_succ_p (e->src) && !bitmap_bit_p (&bb_on_list, e->src->index) + && (BB_PARTITION (e->src) == BB_PARTITION (e->dest)) && can_duplicate_block_p (e->src)) { edge pe;