diff mbox

[PR61554] ICE during CCP

Message ID 53A7BBDA.5010207@codesourcery.com
State New
Headers show

Commit Message

Chung-Lin Tang June 23, 2014, 5:32 a.m. UTC
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  <cltang@codesourcery.com>

	PR tree-optimization/61554
	* tree-ssa-propagate.c (substitute_and_fold_dom_walker):
	Add 'vec<basic_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.

Comments

Richard Biener June 23, 2014, 8:45 a.m. UTC | #1
On Mon, Jun 23, 2014 at 7:32 AM, Chung-Lin Tang <cltang@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  <cltang@codesourcery.com>
>
>         PR tree-optimization/61554
>         * tree-ssa-propagate.c (substitute_and_fold_dom_walker):
>         Add 'vec<basic_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.
diff mbox

Patch

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