Message ID | ba612ac9-6ed4-79ca-ec16-c11ed10d20db@redhat.com |
---|---|
State | New |
Headers | show |
On Fri, Jan 13, 2017 at 10:24 PM, Jeff Law <law@redhat.com> wrote: > > > Given a block with more than one dead store, one of which is the last > statement in the block, the existence debugging statements can change the > generated code which is of course bad. > > The problem is when I moved the code to delete the dead statement into its > own function, I passed in the statement rather than the iterator. As a > result dse_dom_walker::before_dom_children would not see the iterator in > the proper state and it would fail to walk all the statements if > dse_optimize_stmt deleted the last statement in the block. > > In a debug build that scenario was much less likely to trigger because of > the existence of debug statements. > > I'm installing this on the trunk now to fix the bootstrap issues. I'll try > to construct a testcase Monday. > > Bootstrapped and regression tested on x86_64-linux-gnu and ppc64-linux-gnu. > Installing on the trunk. > > Sorry for the breakage. This fixed the bootstrap issue I was seeing on aarch64-linux-gnu. Thanks, Andrew > > Jeff > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index 3c74b19..d07b1f5 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,14 @@ > +2017-01-13 Jeff Law <law@redhat.com> > + > + PR tree-optimization/33562 > + PR tree-optimization/61912 > + PR tree-optimization/77485 > + * tree-ssa-dse.c (delete_dead_call): Accept gsi rather than > + a statement. > + (delete_dead_assignment): Likewise. > + (dse_dom_walker::dse_optimize_stmt): Pass in the gsi rather than > + statement to delete_dead_call and delete_dead_assignment. > + > 2017-01-13 David Malcolm <dmalcolm@redhat.com> > > PR c/78304 > diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c > index 20cf3b4..2e6f8ff 100644 > --- a/gcc/tree-ssa-dse.c > +++ b/gcc/tree-ssa-dse.c > @@ -600,10 +600,11 @@ private: > void dse_optimize_stmt (gimple_stmt_iterator *); > }; > > -/* Delete a dead call STMT, which is mem* call of some kind. */ > +/* Delete a dead call at GSI, which is mem* call of some kind. */ > static void > -delete_dead_call (gimple *stmt) > +delete_dead_call (gimple_stmt_iterator *gsi) > { > + gimple *stmt = gsi_stmt (*gsi); > if (dump_file && (dump_flags & TDF_DETAILS)) > { > fprintf (dump_file, " Deleted dead call: "); > @@ -612,13 +613,12 @@ delete_dead_call (gimple *stmt) > } > > tree lhs = gimple_call_lhs (stmt); > - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); > if (lhs) > { > tree ptr = gimple_call_arg (stmt, 0); > gimple *new_stmt = gimple_build_assign (lhs, ptr); > unlink_stmt_vdef (stmt); > - if (gsi_replace (&gsi, new_stmt, true)) > + if (gsi_replace (gsi, new_stmt, true)) > bitmap_set_bit (need_eh_cleanup, gimple_bb (stmt)->index); > } > else > @@ -627,17 +627,18 @@ delete_dead_call (gimple *stmt) > unlink_stmt_vdef (stmt); > > /* Remove the dead store. */ > - if (gsi_remove (&gsi, true)) > + if (gsi_remove (gsi, true)) > bitmap_set_bit (need_eh_cleanup, gimple_bb (stmt)->index); > release_defs (stmt); > } > } > > -/* Delete a dead store STMT, which is a gimple assignment. */ > +/* Delete a dead store at GSI, which is a gimple assignment. */ > > static void > -delete_dead_assignment (gimple *stmt) > +delete_dead_assignment (gimple_stmt_iterator *gsi) > { > + gimple *stmt = gsi_stmt (*gsi); > if (dump_file && (dump_flags & TDF_DETAILS)) > { > fprintf (dump_file, " Deleted dead store: "); > @@ -649,9 +650,8 @@ delete_dead_assignment (gimple *stmt) > unlink_stmt_vdef (stmt); > > /* Remove the dead store. */ > - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); > basic_block bb = gimple_bb (stmt); > - if (gsi_remove (&gsi, true)) > + if (gsi_remove (gsi, true)) > bitmap_set_bit (need_eh_cleanup, bb->index); > > /* And release any SSA_NAMEs set in this statement back to the > @@ -717,7 +717,7 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator > *gsi) > } > > if (store_status == DSE_STORE_DEAD) > - delete_dead_call (stmt); > + delete_dead_call (gsi); > return; > } > > @@ -760,7 +760,7 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator > *gsi) > && !gimple_clobber_p (use_stmt)) > return; > > - delete_dead_assignment (stmt); > + delete_dead_assignment (gsi); > } > } > >
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 3c74b19..d07b1f5 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,14 @@ +2017-01-13 Jeff Law <law@redhat.com> + + PR tree-optimization/33562 + PR tree-optimization/61912 + PR tree-optimization/77485 + * tree-ssa-dse.c (delete_dead_call): Accept gsi rather than + a statement. + (delete_dead_assignment): Likewise. + (dse_dom_walker::dse_optimize_stmt): Pass in the gsi rather than + statement to delete_dead_call and delete_dead_assignment. + 2017-01-13 David Malcolm <dmalcolm@redhat.com> PR c/78304 diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c index 20cf3b4..2e6f8ff 100644 --- a/gcc/tree-ssa-dse.c +++ b/gcc/tree-ssa-dse.c @@ -600,10 +600,11 @@ private: void dse_optimize_stmt (gimple_stmt_iterator *); }; -/* Delete a dead call STMT, which is mem* call of some kind. */ +/* Delete a dead call at GSI, which is mem* call of some kind. */ static void -delete_dead_call (gimple *stmt) +delete_dead_call (gimple_stmt_iterator *gsi) { + gimple *stmt = gsi_stmt (*gsi); if (dump_file && (dump_flags & TDF_DETAILS)) { fprintf (dump_file, " Deleted dead call: "); @@ -612,13 +613,12 @@ delete_dead_call (gimple *stmt) } tree lhs = gimple_call_lhs (stmt); - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); if (lhs) { tree ptr = gimple_call_arg (stmt, 0); gimple *new_stmt = gimple_build_assign (lhs, ptr); unlink_stmt_vdef (stmt); - if (gsi_replace (&gsi, new_stmt, true)) + if (gsi_replace (gsi, new_stmt, true)) bitmap_set_bit (need_eh_cleanup, gimple_bb (stmt)->index); } else @@ -627,17 +627,18 @@ delete_dead_call (gimple *stmt) unlink_stmt_vdef (stmt); /* Remove the dead store. */ - if (gsi_remove (&gsi, true)) + if (gsi_remove (gsi, true)) bitmap_set_bit (need_eh_cleanup, gimple_bb (stmt)->index); release_defs (stmt); } } -/* Delete a dead store STMT, which is a gimple assignment. */ +/* Delete a dead store at GSI, which is a gimple assignment. */ static void -delete_dead_assignment (gimple *stmt) +delete_dead_assignment (gimple_stmt_iterator *gsi) { + gimple *stmt = gsi_stmt (*gsi); if (dump_file && (dump_flags & TDF_DETAILS)) { fprintf (dump_file, " Deleted dead store: "); @@ -649,9 +650,8 @@ delete_dead_assignment (gimple *stmt) unlink_stmt_vdef (stmt); /* Remove the dead store. */ - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); basic_block bb = gimple_bb (stmt); - if (gsi_remove (&gsi, true)) + if (gsi_remove (gsi, true)) bitmap_set_bit (need_eh_cleanup, bb->index); /* And release any SSA_NAMEs set in this statement back to the @@ -717,7 +717,7 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi) } if (store_status == DSE_STORE_DEAD) - delete_dead_call (stmt); + delete_dead_call (gsi); return; } @@ -760,7 +760,7 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi) && !gimple_clobber_p (use_stmt)) return; - delete_dead_assignment (stmt); + delete_dead_assignment (gsi); } }