diff mbox

Fix ICE with dangling abnormal call edges

Message ID 201010172300.11179.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Oct. 17, 2010, 9 p.m. UTC
Hi,

bootstrapping the Ada compiler with -gnatn (inlining enabled) yields an ICE 
during CDDCE.  I've attached a reduced testcase, but it needs to be compiled 
within the build tree as the problem is related to __builtin_setjmp/longjmp.

The sequence of events is as follows: FRE replaces

  D.3227_33 = sinfo.entity (pref_1(ab));

[...]

<bb 25>:
  if (D.3228_35 == 2)
    goto <bb 26>;
  else
    goto <bb 30>;

<bb 26>:
  D.3231_37 = sinfo.entity (pref_1(ab));

[...]

<bb 30>:
  D.3235_43 = sinfo.entity (pref_1(ab));

with

<bb 25>:
  if (D.3228_35 == 2)
    goto <bb 26>;
  else
    goto <bb 30>;

<bb 26>:
  D.3231_37 = D.3227_33;

[...]

<bb 30>:
  D.3235_43 = D.3227_33;

leaving dangling outgoing abnormal edges in BB 26 and BB 30.

Later DCE removes the assignments:

<bb 23>:
  if (D.3228_35 == 2)
    goto <bb 24>;
  else
    goto <bb 28>;

<bb 24>:

[...]

<bb 28>:

leaving dangling empty BB 24 and 28, which confuses the CDDCE pass:

Eliminating unnecessary statements:
Deleting : if (D.3228_35 == 2)

as the statement isn't dead at all.


Fixed by purging the dead abnormal call edges at the end of PRE, similarly to 
what is done for dead EH edges.  I took this opportunity to clean up the 
existing gimple_purge_dead_abnormal_call_edges, as EH edges aren't abnormal 
edges any longer.

Bootstrapped/regtested on i586-suse-linux, OK for mainline?


2010-10-17  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-flow.h (gimple_purge_all_dead_abnormal_call_edges): Declare.
	* tree-cfg.c (gimple_purge_dead_abnormal_call_edges): Move around.
	Rewrite modelled on gimple_purge_dead_eh_edges.
	(gimple_purge_all_dead_abnormal_call_edges): New function.
	* tree-inline.c (expand_call_inline): Call gimple_purge_dead_eh_edges
	directly instead of through gimple_purge_dead_abnormal_call_edges.
	* tree-ssa-pre.c (need_ab_cleanup): New static variable.
	(eliminate): Set bit in need_ab_cleanup for the basic block if we
	removed AB side-effects from one of its statements.
	(init_pre): Initialize need_ab_cleanup.
	(fini_pre): Purge dead abnormal call edges and clean up the CFG if
	some bits are set in need_ab_cleanup.  Free need_ab_cleanup.

Comments

Richard Biener Oct. 18, 2010, 9:22 a.m. UTC | #1
On Sun, Oct 17, 2010 at 11:00 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> bootstrapping the Ada compiler with -gnatn (inlining enabled) yields an ICE
> during CDDCE.  I've attached a reduced testcase, but it needs to be compiled
> within the build tree as the problem is related to __builtin_setjmp/longjmp.
>
> The sequence of events is as follows: FRE replaces
>
>  D.3227_33 = sinfo.entity (pref_1(ab));
>
> [...]
>
> <bb 25>:
>  if (D.3228_35 == 2)
>    goto <bb 26>;
>  else
>    goto <bb 30>;
>
> <bb 26>:
>  D.3231_37 = sinfo.entity (pref_1(ab));
>
> [...]
>
> <bb 30>:
>  D.3235_43 = sinfo.entity (pref_1(ab));
>
> with
>
> <bb 25>:
>  if (D.3228_35 == 2)
>    goto <bb 26>;
>  else
>    goto <bb 30>;
>
> <bb 26>:
>  D.3231_37 = D.3227_33;
>
> [...]
>
> <bb 30>:
>  D.3235_43 = D.3227_33;
>
> leaving dangling outgoing abnormal edges in BB 26 and BB 30.
>
> Later DCE removes the assignments:
>
> <bb 23>:
>  if (D.3228_35 == 2)
>    goto <bb 24>;
>  else
>    goto <bb 28>;
>
> <bb 24>:
>
> [...]
>
> <bb 28>:
>
> leaving dangling empty BB 24 and 28, which confuses the CDDCE pass:
>
> Eliminating unnecessary statements:
> Deleting : if (D.3228_35 == 2)
>
> as the statement isn't dead at all.
>
>
> Fixed by purging the dead abnormal call edges at the end of PRE, similarly to
> what is done for dead EH edges.  I took this opportunity to clean up the
> existing gimple_purge_dead_abnormal_call_edges, as EH edges aren't abnormal
> edges any longer.
>
> Bootstrapped/regtested on i586-suse-linux, OK for mainline?

Ok.  I think that the indirect -> direct call folding needs a similar change
though.

Thanks,
Richard.

>
> 2010-10-17  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * tree-flow.h (gimple_purge_all_dead_abnormal_call_edges): Declare.
>        * tree-cfg.c (gimple_purge_dead_abnormal_call_edges): Move around.
>        Rewrite modelled on gimple_purge_dead_eh_edges.
>        (gimple_purge_all_dead_abnormal_call_edges): New function.
>        * tree-inline.c (expand_call_inline): Call gimple_purge_dead_eh_edges
>        directly instead of through gimple_purge_dead_abnormal_call_edges.
>        * tree-ssa-pre.c (need_ab_cleanup): New static variable.
>        (eliminate): Set bit in need_ab_cleanup for the basic block if we
>        removed AB side-effects from one of its statements.
>        (init_pre): Initialize need_ab_cleanup.
>        (fini_pre): Purge dead abnormal call edges and clean up the CFG if
>        some bits are set in need_ab_cleanup.  Free need_ab_cleanup.
>
>
> --
> Eric Botcazou
>
Eric Botcazou Oct. 18, 2010, 9:33 a.m. UTC | #2
> Ok.  I think that the indirect -> direct call folding needs a similar
> change though.

OK, I'll add it and retest, thanks.
diff mbox

Patch

Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 165574)
+++ tree-flow.h	(working copy)
@@ -450,9 +450,10 @@  extern void gather_blocks_in_sese_region
 					  VEC(basic_block,heap) **bbs_p);
 extern void add_phi_args_after_copy_bb (basic_block);
 extern void add_phi_args_after_copy (basic_block *, unsigned, edge);
-extern bool gimple_purge_dead_abnormal_call_edges (basic_block);
 extern bool gimple_purge_dead_eh_edges (basic_block);
 extern bool gimple_purge_all_dead_eh_edges (const_bitmap);
+extern bool gimple_purge_dead_abnormal_call_edges (basic_block);
+extern bool gimple_purge_all_dead_abnormal_call_edges (const_bitmap);
 extern tree gimplify_build1 (gimple_stmt_iterator *, enum tree_code,
 			     tree, tree);
 extern tree gimplify_build2 (gimple_stmt_iterator *, enum tree_code,
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 165574)
+++ tree-cfg.c	(working copy)
@@ -6824,39 +6824,6 @@  gimple_flow_call_edges_add (sbitmap bloc
   return blocks_split;
 }
 
-/* Purge dead abnormal call edges from basic block BB.  */
-
-bool
-gimple_purge_dead_abnormal_call_edges (basic_block bb)
-{
-  bool changed = gimple_purge_dead_eh_edges (bb);
-
-  if (cfun->has_nonlocal_label)
-    {
-      gimple stmt = last_stmt (bb);
-      edge_iterator ei;
-      edge e;
-
-      if (!(stmt && stmt_can_make_abnormal_goto (stmt)))
-	for (ei = ei_start (bb->succs); (e = ei_safe_edge (ei)); )
-	  {
-	    if (e->flags & EDGE_ABNORMAL)
-	      {
-		remove_edge (e);
-		changed = true;
-	      }
-	    else
-	      ei_next (&ei);
-	  }
-
-      /* See gimple_purge_dead_eh_edges below.  */
-      if (changed)
-	free_dominance_info (CDI_DOMINATORS);
-    }
-
-  return changed;
-}
-
 /* Removes edge E and all the blocks dominated by it, and updates dominance
    information.  The IL in E->src needs to be updated separately.
    If dominance info is not available, only the edge E is removed.*/
@@ -7010,6 +6977,8 @@  gimple_purge_dead_eh_edges (basic_block
   return changed;
 }
 
+/* Purge dead EH edges from basic block listed in BLOCKS.  */
+
 bool
 gimple_purge_all_dead_eh_edges (const_bitmap blocks)
 {
@@ -7029,6 +6998,59 @@  gimple_purge_all_dead_eh_edges (const_bi
     }
 
   return changed;
+}
+
+/* Purge dead abnormal call edges from basic block BB.  */
+
+bool
+gimple_purge_dead_abnormal_call_edges (basic_block bb)
+{
+  bool changed = false;
+  edge e;
+  edge_iterator ei;
+  gimple stmt = last_stmt (bb);
+
+  if (!cfun->has_nonlocal_label)
+    return false;
+
+  if (stmt && stmt_can_make_abnormal_goto (stmt))
+    return false;
+
+  for (ei = ei_start (bb->succs); (e = ei_safe_edge (ei)); )
+    {
+      if (e->flags & EDGE_ABNORMAL)
+	{
+	  remove_edge_and_dominated_blocks (e);
+	  changed = true;
+	}
+      else
+	ei_next (&ei);
+    }
+
+  return changed;
+}
+
+/* Purge dead abnormal call edges from basic block listed in BLOCKS.  */
+
+bool
+gimple_purge_all_dead_abnormal_call_edges (const_bitmap blocks)
+{
+  bool changed = false;
+  unsigned i;
+  bitmap_iterator bi;
+
+  EXECUTE_IF_SET_IN_BITMAP (blocks, 0, i, bi)
+    {
+      basic_block bb = BASIC_BLOCK (i);
+
+      /* Earlier gimple_purge_dead_abnormal_call_edges could have removed
+	 this basic block already.  */
+      gcc_assert (bb || changed);
+      if (bb != NULL)
+	changed |= gimple_purge_dead_abnormal_call_edges (bb);
+    }
+
+  return changed;
 }
 
 /* This function is called whenever a new edge is created or
Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 165574)
+++ tree-inline.c	(working copy)
@@ -4010,7 +4010,10 @@  expand_call_inline (basic_block bb, gimp
     }
 
   if (purge_dead_abnormal_edges)
-    gimple_purge_dead_abnormal_call_edges (return_block);
+    {
+      gimple_purge_dead_eh_edges (return_block);
+      gimple_purge_dead_abnormal_call_edges (return_block);
+    }
 
   /* If the value of the new expression is ignored, that's OK.  We
      don't warn about this for CALL_EXPRs, so we shouldn't warn about
Index: tree-ssa-pre.c
===================================================================
--- tree-ssa-pre.c	(revision 165574)
+++ tree-ssa-pre.c	(working copy)
@@ -484,10 +484,12 @@  static tree pretemp;
 static tree storetemp;
 static tree prephitemp;
 
-/* Set of blocks with statements that have had its EH information
-   cleaned up.  */
+/* Set of blocks with statements that have had their EH properties changed.  */
 static bitmap need_eh_cleanup;
 
+/* Set of blocks with statements that have had their AB properties changed.  */
+static bitmap need_ab_cleanup;
+
 /* The phi_translate_table caches phi translations for a given
    expression and predecessor.  */
 
@@ -4253,6 +4255,9 @@  eliminate (void)
 		      || TREE_CODE (rhs) != SSA_NAME
 		      || may_propagate_copy (rhs, sprime)))
 		{
+		  bool can_make_abnormal_goto
+		    = is_gimple_call (stmt)
+		      && stmt_can_make_abnormal_goto (stmt);
 		  gcc_assert (sprime != rhs);
 
 		  if (dump_file && (dump_flags & TDF_DETAILS))
@@ -4281,14 +4286,24 @@  eliminate (void)
 		  stmt = gsi_stmt (gsi);
 		  update_stmt (stmt);
 
-		  /* If we removed EH side effects from the statement, clean
+		  /* If we removed EH side-effects from the statement, clean
 		     its EH information.  */
 		  if (maybe_clean_or_replace_eh_stmt (stmt, stmt))
 		    {
 		      bitmap_set_bit (need_eh_cleanup,
 				      gimple_bb (stmt)->index);
 		      if (dump_file && (dump_flags & TDF_DETAILS))
-			fprintf (dump_file, "  Removed EH side effects.\n");
+			fprintf (dump_file, "  Removed EH side-effects.\n");
+		    }
+
+		  /* Likewise for AB side-effects.  */
+		  if (can_make_abnormal_goto
+		      && !stmt_can_make_abnormal_goto (stmt))
+		    {
+		      bitmap_set_bit (need_ab_cleanup,
+				      gimple_bb (stmt)->index);
+		      if (dump_file && (dump_flags & TDF_DETAILS))
+			fprintf (dump_file, "  Removed AB side-effects.\n");
 		    }
 		}
 	    }
@@ -4746,6 +4761,7 @@  init_pre (bool do_fre)
     }
 
   need_eh_cleanup = BITMAP_ALLOC (NULL);
+  need_ab_cleanup = BITMAP_ALLOC (NULL);
 }
 
 
@@ -4777,6 +4793,14 @@  fini_pre (bool do_fre)
 
   BITMAP_FREE (need_eh_cleanup);
 
+  if (!bitmap_empty_p (need_ab_cleanup))
+    {
+      gimple_purge_all_dead_abnormal_call_edges (need_ab_cleanup);
+      cleanup_tree_cfg ();
+    }
+
+  BITMAP_FREE (need_ab_cleanup);
+
   if (!do_fre)
     loop_optimizer_finalize ();
 }