Re: [PATCH, PR61554] ICE during CCP
On 2014/6/23 04:45 PM, Richard Biener wrote:
On Mon, Jun 23, 2014 at 7:32 AM, Chung-Lin Tang clt...@codesourcery.com
wrote:
Hi Richard,
In this change:
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01278.html
where substitute_and_fold() was changed to use a dom walker, the calls
to purge dead EH edges during the walk can alter the dom-tree, and have
chaotic results; the testcase in PR 61554 has some blocks traversed
twice during the walk, causing the segfault during CCP.
The patch records the to-be-purged-for-dead-EH blocks in a similar
manner like stmts_to_remove, and processes it after the walk. (another
possible method would be using a bitmap to record the BBs + calling
gimple_purge_all_dead_eh_edges...)
Oops.
Bootstrapped and tested on x86_64-linux, is this okay for trunk?
Can you please use a bitmap and use gimple_purge_all_dead_eh_edges
like tree-ssa-pre.c does?
Also please add the reduced testcase from the PR to the g++.dg/torture
Ok with that changes.
Thanks,
Richard.
Thanks for the review. Attached is what I committed. Testcase made by
Markus also added.
Thanks,
Chung-Lin
2014-06-24 Chung-Lin Tang clt...@codesourcery.com
PR tree-optimization/61554
* tree-ssa-propagate.c: Include bitmap.h.
(substitute_and_fold_dom_walker): Add 'bitmap need_eh_cleanup'
member,
properly update constructor/destructor.
(substitute_and_fold_dom_walker::before_dom_children):
Remove call to gimple_purge_dead_eh_edges, add bb-index to
need_eh_cleaup instead.
(substitute_and_fold): Call gimple_purge_all_dead_eh_edges on
need_eh_cleanup.
Index: tree-ssa-propagate.c
===
--- tree-ssa-propagate.c (revision 211927)
+++ tree-ssa-propagate.c (working copy)
@@ -29,6 +29,7 @@
#include function.h
#include gimple-pretty-print.h
#include dumpfile.h
+#include bitmap.h
#include sbitmap.h
#include tree-ssa-alias.h
#include internal-fn.h
@@ -1031,8 +1032,13 @@ class substitute_and_fold_dom_walker : public dom_
fold_fn (fold_fn_), do_dce (do_dce_), something_changed (false)
{
stmts_to_remove.create (0);
+ need_eh_cleanup = BITMAP_ALLOC (NULL);
}
-~substitute_and_fold_dom_walker () { stmts_to_remove.release (); }
+~substitute_and_fold_dom_walker ()
+{
+ stmts_to_remove.release ();
+ BITMAP_FREE (need_eh_cleanup);
+}
virtual void before_dom_children (basic_block);
virtual void after_dom_children (basic_block) {}
@@ -1042,6 +1048,7 @@ class substitute_and_fold_dom_walker : public dom_
bool do_dce;
bool something_changed;
vecgimple stmts_to_remove;
+bitmap need_eh_cleanup;
};
void
@@ -1144,7 +1151,7 @@ substitute_and_fold_dom_walker::before_dom_childre
/* If we cleaned up EH information from the statement,
remove EH edges. */
if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt))
- gimple_purge_dead_eh_edges (bb);
+ bitmap_set_bit (need_eh_cleanup, bb-index);
if (is_gimple_assign (stmt)
(get_gimple_rhs_class (gimple_assign_rhs_code (stmt))
@@ -1235,6 +1242,9 @@ substitute_and_fold (ssa_prop_get_value_fn get_val
}
}
+ if (!bitmap_empty_p (walker.need_eh_cleanup))
+gimple_purge_all_dead_eh_edges (walker.need_eh_cleanup);
+
statistics_counter_event (cfun, Constants propagated,
prop_stats.num_const_prop);
statistics_counter_event (cfun, Copies propagated,
Re: [PATCH, PR61554] ICE during CCP
On Mon, Jun 23, 2014 at 7:32 AM, Chung-Lin Tang clt...@codesourcery.com wrote: Hi Richard, In this change: https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01278.html where substitute_and_fold() was changed to use a dom walker, the calls to purge dead EH edges during the walk can alter the dom-tree, and have chaotic results; the testcase in PR 61554 has some blocks traversed twice during the walk, causing the segfault during CCP. The patch records the to-be-purged-for-dead-EH blocks in a similar manner like stmts_to_remove, and processes it after the walk. (another possible method would be using a bitmap to record the BBs + calling gimple_purge_all_dead_eh_edges...) Oops. Bootstrapped and tested on x86_64-linux, is this okay for trunk? Can you please use a bitmap and use gimple_purge_all_dead_eh_edges like tree-ssa-pre.c does? Also please add the reduced testcase from the PR to the g++.dg/torture Ok with that changes. Thanks, Richard. Thanks, Chung-Lin 2014-06-23 Chung-Lin Tang clt...@codesourcery.com PR tree-optimization/61554 * tree-ssa-propagate.c (substitute_and_fold_dom_walker): Add 'vecbasic_block bbs_to_purge_dead_eh_edges' member, properly update constructor/destructor. (substitute_and_fold_dom_walker::before_dom_children): Remove call to gimple_purge_dead_eh_edges, add bb to bbs_to_purge_dead_eh_edges instead. (substitute_and_fold): Call gimple_purge_dead_eh_edges for bbs recorded in bbs_to_purge_dead_eh_edges.
[PATCH, PR61554] ICE during CCP
Hi Richard,
In this change:
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01278.html
where substitute_and_fold() was changed to use a dom walker, the calls
to purge dead EH edges during the walk can alter the dom-tree, and have
chaotic results; the testcase in PR 61554 has some blocks traversed
twice during the walk, causing the segfault during CCP.
The patch records the to-be-purged-for-dead-EH blocks in a similar
manner like stmts_to_remove, and processes it after the walk. (another
possible method would be using a bitmap to record the BBs + calling
gimple_purge_all_dead_eh_edges...)
Bootstrapped and tested on x86_64-linux, is this okay for trunk?
Thanks,
Chung-Lin
2014-06-23 Chung-Lin Tang clt...@codesourcery.com
PR tree-optimization/61554
* tree-ssa-propagate.c (substitute_and_fold_dom_walker):
Add 'vecbasic_block bbs_to_purge_dead_eh_edges' member,
properly update constructor/destructor.
(substitute_and_fold_dom_walker::before_dom_children):
Remove call to gimple_purge_dead_eh_edges, add bb to
bbs_to_purge_dead_eh_edges instead.
(substitute_and_fold): Call gimple_purge_dead_eh_edges for
bbs recorded in bbs_to_purge_dead_eh_edges.
Index: tree-ssa-propagate.c
===
--- tree-ssa-propagate.c (revision 211874)
+++ tree-ssa-propagate.c (working copy)
@@ -1031,8 +1031,13 @@ class substitute_and_fold_dom_walker : public dom_
fold_fn (fold_fn_), do_dce (do_dce_), something_changed (false)
{
stmts_to_remove.create (0);
+ bbs_to_purge_dead_eh_edges.create (0);
}
-~substitute_and_fold_dom_walker () { stmts_to_remove.release (); }
+~substitute_and_fold_dom_walker ()
+{
+ stmts_to_remove.release ();
+ bbs_to_purge_dead_eh_edges.release ();
+}
virtual void before_dom_children (basic_block);
virtual void after_dom_children (basic_block) {}
@@ -1042,6 +1047,7 @@ class substitute_and_fold_dom_walker : public dom_
bool do_dce;
bool something_changed;
vecgimple stmts_to_remove;
+vecbasic_block bbs_to_purge_dead_eh_edges;
};
void
@@ -1144,7 +1150,7 @@ substitute_and_fold_dom_walker::before_dom_childre
/* If we cleaned up EH information from the statement,
remove EH edges. */
if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt))
- gimple_purge_dead_eh_edges (bb);
+ bbs_to_purge_dead_eh_edges.safe_push (bb);
if (is_gimple_assign (stmt)
(get_gimple_rhs_class (gimple_assign_rhs_code (stmt))
@@ -1235,6 +1241,14 @@ substitute_and_fold (ssa_prop_get_value_fn get_val
}
}
+ while (!walker.bbs_to_purge_dead_eh_edges.is_empty ())
+{
+ basic_block bb = walker.bbs_to_purge_dead_eh_edges.pop ();
+ gimple_purge_dead_eh_edges (bb);
+ if (dump_file dump_flags TDF_DETAILS)
+ fprintf (dump_file, Purge dead EH edges from bb %d\n, bb-index);
+}
+
statistics_counter_event (cfun, Constants propagated,
prop_stats.num_const_prop);
statistics_counter_event (cfun, Copies propagated,
