Message ID | 20101101192153.GA29412@tyan-ft48-01.lab.bos.redhat.com |
---|---|
State | New |
Headers | show |
On Mon, Nov 1, 2010 at 8:21 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > While debug stmts are never considered to need fake edges, it currently > (incorrectly) makes a difference whether a stmt that needs fake edges > was the last one or not. Fixed by disregarding any debug stmts > at the end of bb. > > Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk? Hmm, you mean that the stmt needing fake edges doesn't end the BB? So your patch fixes that by splitting the BB at the appropriate point? If so, it's ok. Thanks, Richard. > 2010-11-01 Jakub Jelinek <jakub@redhat.com> > > PR debug/46255 > * tree-cfg.c (gimple_flow_call_edges_add): Use gsi_last_nondebug_bb > instead of gsi_last_bb. > > * gcc.dg/pr46255.c: New test. > > --- gcc/tree-cfg.c.jj 2010-11-01 09:07:24.000000000 +0100 > +++ gcc/tree-cfg.c 2010-11-01 14:03:05.000000000 +0100 > @@ -6749,7 +6749,7 @@ gimple_flow_call_edges_add (sbitmap bloc > if (check_last_block) > { > basic_block bb = EXIT_BLOCK_PTR->prev_bb; > - gimple_stmt_iterator gsi = gsi_last_bb (bb); > + gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb); > gimple t = NULL; > > if (!gsi_end_p (gsi)) > @@ -6783,7 +6783,7 @@ gimple_flow_call_edges_add (sbitmap bloc > if (blocks && !TEST_BIT (blocks, i)) > continue; > > - gsi = gsi_last_bb (bb); > + gsi = gsi_last_nondebug_bb (bb); > if (!gsi_end_p (gsi)) > { > last_stmt = gsi_stmt (gsi); > --- gcc/testsuite/gcc.dg/pr46255.c.jj 2010-11-01 14:04:55.000000000 +0100 > +++ gcc/testsuite/gcc.dg/pr46255.c 2010-11-01 14:04:34.000000000 +0100 > @@ -0,0 +1,12 @@ > +/* PR debug/46255 */ > +/* { dg-do compile } */ > +/* { dg-options "-fcompare-debug -fprofile-generate -O" } */ > + > +int bar (void); > + > +void > +foo (int i) > +{ > + while (i) > + i = bar (); > +} > > Jakub >
On Mon, Nov 01, 2010 at 11:29:57PM +0100, Richard Guenther wrote: > On Mon, Nov 1, 2010 at 8:21 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > While debug stmts are never considered to need fake edges, it currently > > (incorrectly) makes a difference whether a stmt that needs fake edges > > was the last one or not. Fixed by disregarding any debug stmts > > at the end of bb. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk? > > Hmm, you mean that the stmt needing fake edges doesn't end the BB? Yes. > So your patch fixes that by splitting the BB at the appropriate point? No, that was what the code was doing before and that caused the -fcompare-debug differences, because without -g the bb was not split, with -g it was split. profile.c uses block_ends_with_call_p, which for gimple uses /* Return true if BB ends with a call, possibly followed by some instructions that must stay with the call. Return false, otherwise. */ static bool gimple_block_ends_with_call_p (basic_block bb) { gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb); return !gsi_end_p (gsi) && is_gimple_call (gsi_stmt (gsi)); } Having the fake edge gong not immediately from the call, but from the end of bb where the call is followed only by debug stmts, seems to work just fine, all it means is the profiling stuff gets emitted before or after those debug stmts which really result in no code being generated. Jakub
On Tue, Nov 2, 2010 at 12:07 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Mon, Nov 01, 2010 at 11:29:57PM +0100, Richard Guenther wrote: >> On Mon, Nov 1, 2010 at 8:21 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> > While debug stmts are never considered to need fake edges, it currently >> > (incorrectly) makes a difference whether a stmt that needs fake edges >> > was the last one or not. Fixed by disregarding any debug stmts >> > at the end of bb. >> > >> > Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk? >> >> Hmm, you mean that the stmt needing fake edges doesn't end the BB? > > Yes. > >> So your patch fixes that by splitting the BB at the appropriate point? > > No, that was what the code was doing before and that caused the > -fcompare-debug differences, because without -g the bb was not split, with > -g it was split. Hmm, I remember we drop DEBUG statements after calls that throw to be able to split the BB at the appropriate point. Do we? > profile.c uses block_ends_with_call_p, which for gimple uses > /* Return true if BB ends with a call, possibly followed by some > instructions that must stay with the call. Return false, > otherwise. */ > > static bool > gimple_block_ends_with_call_p (basic_block bb) > { > gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb); > return !gsi_end_p (gsi) && is_gimple_call (gsi_stmt (gsi)); > } > > Having the fake edge gong not immediately from the call, but > from the end of bb where the call is followed only by debug stmts, > seems to work just fine, all it means is the profiling stuff > gets emitted before or after those debug stmts which really result in > no code being generated. I guess as they are fake edges and only profiling uses it (but it's a CFG hook also implemented on RTL ... huh...) it probably doesn't matter. So, the patch is ok. It would be nice to make this localized though and remove the hookization of this functionality. Richard. > Jakub >
--- gcc/tree-cfg.c.jj 2010-11-01 09:07:24.000000000 +0100 +++ gcc/tree-cfg.c 2010-11-01 14:03:05.000000000 +0100 @@ -6749,7 +6749,7 @@ gimple_flow_call_edges_add (sbitmap bloc if (check_last_block) { basic_block bb = EXIT_BLOCK_PTR->prev_bb; - gimple_stmt_iterator gsi = gsi_last_bb (bb); + gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb); gimple t = NULL; if (!gsi_end_p (gsi)) @@ -6783,7 +6783,7 @@ gimple_flow_call_edges_add (sbitmap bloc if (blocks && !TEST_BIT (blocks, i)) continue; - gsi = gsi_last_bb (bb); + gsi = gsi_last_nondebug_bb (bb); if (!gsi_end_p (gsi)) { last_stmt = gsi_stmt (gsi); --- gcc/testsuite/gcc.dg/pr46255.c.jj 2010-11-01 14:04:55.000000000 +0100 +++ gcc/testsuite/gcc.dg/pr46255.c 2010-11-01 14:04:34.000000000 +0100 @@ -0,0 +1,12 @@ +/* PR debug/46255 */ +/* { dg-do compile } */ +/* { dg-options "-fcompare-debug -fprofile-generate -O" } */ + +int bar (void); + +void +foo (int i) +{ + while (i) + i = bar (); +}