[Bug tree-optimization/20640] [4.0 Regression] ICE on NULL PHI_ARG_DEF

["aoliva at redhat dot com" <gcc-bugzilla at gcc dot gnu dot org>]

------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-31 08:28 -------
Subject: Re: [PR tree-optimization/20640] add phi args to dests of dce-redirected edges

On Mar 31, 2005, Alexandre Oliva <aoliva@redhat.com> wrote:

> I have a gut feeling that we'll always get a PHI arg from one of the
> previous successors of the src of the redirected edge, but I can't
> quite prove it.  What do you think?

A few seconds after posting the patch, ssa-dce-3.c failed, showing my
gut feeling was wrong.  Oh well...

On Mar 30, 2005, Jeffrey A Law <law@redhat.com> wrote:

>> This code is triggered rarely, I would expect it to be even rarer still
>> for POST_DOM_BB to have PHI nodes.  You could probably just ignore dead
>> control statements where the post dominator has PHI nodes and I doubt
>> it would ever be noticed.

It is noticed if we take the same path as the no-post_dom_bb,
infinite-loop case, because then the instruction that remains may
still reference variables that were deleted.

This patch, however, arranges for us to turn the edge into a
fall-through edge to its original destination should the immediate
post dominator be found to have PHI nodes.

I've also tweaked the code so as to remove all dead phi nodes before
removing all other statements, thereby improving the odds of
redirecting a dead control stmt to the post dominator.

Interestingly, the resulting code was no different for some cases I
inspected (the testcase included in the patch and ssa-dce-3.c).
That's because the edge dest bb ends up becoming a simple forwarding
block that is later removed.

As for infinite loops, I'm wondering if we should somehow try to
remove the condition from the loop.  If the conditional branch is
found to be dead, it's quite possible that the set of that variable is
dead as well, and so we'd be keeping a reference to a deleted
variable.  I couldn't actually exercise such an error, and I'm not
convinced it's possible, but I thought I'd bring this up.  Thoughts?

Anyway, how does this patch look?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR tree-optimization/20640
	* tree-ssa-dce.c (remove_dead_stmt): Don't redirect edge to
	post-dominator if it has phi nodes.
	(eliminate_unnecessary_stmts): Remove dead phis in all blocks
	before dead statements.

Index: gcc/tree-ssa-dce.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dce.c,v
retrieving revision 2.37
diff -u -p -r2.37 tree-ssa-dce.c
--- gcc/tree-ssa-dce.c 12 Mar 2005 20:53:18 -0000 2.37
+++ gcc/tree-ssa-dce.c 31 Mar 2005 07:56:42 -0000
@@ -637,7 +637,10 @@ eliminate_unnecessary_stmts (void)
     {
       /* Remove dead PHI nodes.  */
       remove_dead_phis (bb);
+    }
 
+  FOR_EACH_BB (bb)
+    {
       /* Remove dead statements.  */
       for (i = bsi_start (bb); ! bsi_end_p (i) ; )
 	{
@@ -724,6 +727,7 @@ remove_dead_stmt (block_stmt_iterator *i
   if (is_ctrl_stmt (t))
     {
       basic_block post_dom_bb;
+
       /* The post dominance info has to be up-to-date.  */
       gcc_assert (dom_computed[CDI_POST_DOMINATORS] == DOM_OK);
       /* Get the immediate post dominator of bb.  */
@@ -737,6 +741,15 @@ remove_dead_stmt (block_stmt_iterator *i
 	  return;
 	}
 
+      /* If the post dominator block has PHI nodes, we might be unable
+	 to compute the right PHI args for them.  Since the control
+	 statement is unnecessary, all edges can be regarded as
+	 equivalent, but we have to get rid of the condition, since it
+	 might reference a variable that was determined to be
+	 unnecessary and thus removed.  */
+      if (phi_nodes (post_dom_bb))
+	post_dom_bb = EDGE_SUCC (bb, 0)->dest;
+
       /* Redirect the first edge out of BB to reach POST_DOM_BB.  */
       redirect_edge_and_branch (EDGE_SUCC (bb, 0), post_dom_bb);
       PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR tree-optimization/20640
	* gcc.dg/torture/tree-loop-1.c: New.

Index: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
===================================================================
RCS file: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
diff -N gcc/testsuite/gcc.dg/torture/tree-loop-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/gcc.dg/torture/tree-loop-1.c 30 Mar 2005 05:28:22 -0000
@@ -0,0 +1,21 @@
+/* PR tree-optimization/20640 */
+
+/* After unrolling the loop, we'd turn some conditional branches into
+   unconditional ones, but branch redirection would fail to compute
+   the PHI args for the PHI nodes in the replacement edge
+   destination, so they'd remain NULL causing crashes later on.  */
+
+/* { dg-do compile } */
+
+static int a = 0;
+extern int foo (void);
+extern int *bar (void) __attribute__ ((__const__));
+
+void
+test (int x)
+{
+  int b = 10;
+  while (foo () == -1 && *bar () == 4 && b > 0)
+    --b;
+  a = x;
+}

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20640