------- 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED], gcc.gnu.org} Free Software Evangelist [EMAIL PROTECTED], gnu.org} -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20640