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;
vec<gimple> 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",
