This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[tree-ssa] CFG fix
- From: law at redhat dot com
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 11 Jun 2003 22:31:52 -0600
- Subject: [tree-ssa] CFG fix
- Reply-to: law at redhat dot com
first_exec_block is as far as I can tell a remnant of when empty statements
used to be shared.
>From looking at how it works and how it's used, I'm confident that it's
fundamentally incorrect and that we should instead get the block from
the first statement (empty or otherwise) associated with the given
tree node.
The problem is that by skipping empty statements, we can walk into a
different control structure. So if we've got something like this:
while (1)
{
{
i.2 = i;
i = i - 1;
retval.1 = i.2
};
if (retval.1 == 0)
{
goto <ULb690>;
};
(void)0 <P>
};
<ULb690>:;
}
When creating edges for the CFG, we would call first_exec_block with
the empty statement marked as <P> to determine where the fall-thru path
of the "if" should go. We skipped over the empty statement and ultimately
returned the block associated with label ULb690.
Thus the block which contained the empty statement <P> was considered
unreachable. Which wouldn't have caused a problem, except that block
contains the back edge to the top of the loop.
The lack of a back edge to the top of the loop caused us to not insert
a PHI node for "i" at the top of the loop. That in turn caused us to
not see updates for "i" through the loop and ultimately resulted in a
loop that never exited.
Yes, this problem could be fixed in a variety of other ways, such as making
the back edge in the loop explicit with a GOTO, or by further hackery in
tree-cfg.c. However, it's much simpler and cleaner to just get the block
from the empty statement itself (since they're no longer shared they have
annotations and thus have a pointer to their containing block).
This is the first (in a few) steps towards being able to eliminate
redundant or useless calls to "const" functions.
There's also a good chance this will fix one of the problems Jeff Sturm has
had with LOOP_EXPRs emitted by the Java front-end.
Bootstrapped, tested, no regressions. i686-pc-linux-gnu.
* tree-cfg.c (first_exec_block): Kill.
(make_edges): Use bb_for_stmt rather than first_exec_block.
(make_ctrl_stmt_edges, make_exit_edges): Likewise.
(make_loop_expr_edges, make_cond_expr_edges): Likewise.
(successor_block): Don't skip empty statements.
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-cfg.c,v
retrieving revision 1.1.4.104
diff -c -3 -p -r1.1.4.104 tree-cfg.c
*** tree-cfg.c 9 Jun 2003 22:37:01 -0000 1.1.4.104
--- tree-cfg.c 12 Jun 2003 04:17:05 -0000
*************** static void make_case_label_edges (basic
*** 100,106 ****
/* Various helpers. */
static basic_block successor_block (basic_block);
- static basic_block first_exec_block (tree *);
static tree *first_exec_stmt (tree *);
static basic_block switch_parent (basic_block);
static inline bool stmt_starts_bb_p (tree, tree);
--- 100,105 ----
*************** make_edges (void)
*** 862,868 ****
{
tree try_finally = VARRAY_TREE (try_finallys, i);
tree *finally_p = &TREE_OPERAND (try_finally, 1);
! basic_block finally_bb = first_exec_block (finally_p);
tree *finally_last_p;
basic_block last_bb;
int bb;
--- 861,867 ----
{
tree try_finally = VARRAY_TREE (try_finallys, i);
tree *finally_p = &TREE_OPERAND (try_finally, 1);
! basic_block finally_bb = bb_for_stmt (*finally_p);
tree *finally_last_p;
basic_block last_bb;
int bb;
*************** make_edges (void)
*** 906,912 ****
dummy_bitmap,
&finally_last_p);
! last_bb = first_exec_block (finally_last_p);
/* Find edges which exited the TRY block. For each of those
edges, we want to create a new edge from the FINALLY block
--- 905,911 ----
dummy_bitmap,
&finally_last_p);
! last_bb = bb_for_stmt (*finally_last_p);
/* Find edges which exited the TRY block. For each of those
edges, we want to create a new edge from the FINALLY block
*************** make_ctrl_stmt_edges (basic_block bb)
*** 1076,1087 ****
case TRY_FINALLY_EXPR:
VARRAY_PUSH_TREE (try_finallys, last);
if (first_exec_stmt (&TREE_OPERAND (last, 0)) == NULL)
! make_edge (bb, first_exec_block (&TREE_OPERAND (last, 1)), EDGE_ABNORMAL);
/* FALL THROUGH */
case TRY_CATCH_EXPR:
{
! basic_block target_bb = first_exec_block (&TREE_OPERAND (last, 0));
if (target_bb)
make_edge (bb, target_bb, EDGE_FALLTHRU);
--- 1075,1086 ----
case TRY_FINALLY_EXPR:
VARRAY_PUSH_TREE (try_finallys, last);
if (first_exec_stmt (&TREE_OPERAND (last, 0)) == NULL)
! make_edge (bb, bb_for_stmt (TREE_OPERAND (last, 1)), EDGE_ABNORMAL);
/* FALL THROUGH */
case TRY_CATCH_EXPR:
{
! basic_block target_bb = bb_for_stmt (TREE_OPERAND (last, 0));
if (target_bb)
make_edge (bb, target_bb, EDGE_FALLTHRU);
*************** make_ctrl_stmt_edges (basic_block bb)
*** 1091,1097 ****
case CATCH_EXPR:
{
! basic_block target_bb = first_exec_block (&CATCH_BODY (last));
if (target_bb)
make_edge (bb, target_bb, EDGE_FALLTHRU);
--- 1090,1096 ----
case CATCH_EXPR:
{
! basic_block target_bb = bb_for_stmt (CATCH_BODY (last));
if (target_bb)
make_edge (bb, target_bb, EDGE_FALLTHRU);
*************** make_ctrl_stmt_edges (basic_block bb)
*** 1101,1107 ****
case EH_FILTER_EXPR:
{
! basic_block target_bb = first_exec_block (&EH_FILTER_FAILURE (last));
if (target_bb)
make_edge (bb, target_bb, EDGE_ABNORMAL);
--- 1100,1106 ----
case EH_FILTER_EXPR:
{
! basic_block target_bb = bb_for_stmt (EH_FILTER_FAILURE (last));
if (target_bb)
make_edge (bb, target_bb, EDGE_ABNORMAL);
*************** make_exit_edges (basic_block bb)
*** 1160,1166 ****
for (t = stmt_ann (last)->reachable_exception_handlers;
t;
t = TREE_CHAIN (t))
! make_edge (bb, first_exec_block (&TREE_VALUE (t)), EDGE_ABNORMAL);
}
/* Some calls are known not to return. For such calls we need to
--- 1159,1165 ----
for (t = stmt_ann (last)->reachable_exception_handlers;
t;
t = TREE_CHAIN (t))
! make_edge (bb, bb_for_stmt (TREE_VALUE (t)), EDGE_ABNORMAL);
}
/* Some calls are known not to return. For such calls we need to
*************** make_exit_edges (basic_block bb)
*** 1197,1203 ****
t;
t = TREE_CHAIN (t))
make_edge (bb,
! first_exec_block (&TREE_VALUE (t)),
EDGE_ABNORMAL);
}
--- 1196,1202 ----
t;
t = TREE_CHAIN (t))
make_edge (bb,
! bb_for_stmt (TREE_VALUE (t)),
EDGE_ABNORMAL);
}
*************** make_exit_edges (basic_block bb)
*** 1215,1221 ****
t;
t = TREE_CHAIN (t))
make_edge (bb,
! first_exec_block (&TREE_VALUE (t)),
EDGE_ABNORMAL);
}
--- 1214,1220 ----
t;
t = TREE_CHAIN (t))
make_edge (bb,
! bb_for_stmt (TREE_VALUE (t)),
EDGE_ABNORMAL);
}
*************** make_loop_expr_edges (basic_block bb)
*** 1250,1256 ****
abort ();
#endif
! body_bb = first_exec_block (&LOOP_EXPR_BODY (entry));
if (body_bb)
make_edge (bb, body_bb, 0);
}
--- 1249,1255 ----
abort ();
#endif
! body_bb = bb_for_stmt (LOOP_EXPR_BODY (entry));
if (body_bb)
make_edge (bb, body_bb, 0);
}
*************** make_cond_expr_edges (basic_block bb)
*** 1279,1286 ****
#endif
/* Entry basic blocks for each component. */
! then_bb = first_exec_block (&COND_EXPR_THEN (entry));
! else_bb = first_exec_block (&COND_EXPR_ELSE (entry));
successor_bb = successor_block (bb);
if (then_bb)
--- 1278,1285 ----
#endif
/* Entry basic blocks for each component. */
! then_bb = bb_for_stmt (COND_EXPR_THEN (entry));
! else_bb = bb_for_stmt (COND_EXPR_ELSE (entry));
successor_bb = successor_block (bb);
if (then_bb)
*************** successor_block (basic_block bb)
*** 2640,2645 ****
--- 2639,2645 ----
basic_block succ_bb;
tree_stmt_iterator i;
tree last_stmt;
+ tree *container_p;
#if defined ENABLE_CHECKING
if (bb == NULL)
*************** successor_block (basic_block bb)
*** 2658,2666 ****
else
tsi_next (&i);
! succ_bb = first_exec_block (tsi_container (i));
! if (succ_bb)
! return succ_bb;
/* We couldn't find a successor for BB. This means that BB is the last
block inside a control structure or lexical scope. Use the
--- 2658,2670 ----
else
tsi_next (&i);
! container_p = tsi_container (i);
! if (container_p)
! {
! succ_bb = bb_for_stmt (*container_p);
! if (succ_bb)
! return succ_bb;
! }
/* We couldn't find a successor for BB. This means that BB is the last
block inside a control structure or lexical scope. Use the
*************** first_exec_stmt (tree *entry_p)
*** 2906,2928 ****
return NULL;
}
-
-
- /* Return the first basic block with executable statements in it, starting
- at ENTRY_P. */
-
- static basic_block
- first_exec_block (tree *entry_p)
- {
- tree *exec_p;
-
- if (entry_p == NULL)
- return NULL;
-
- exec_p = first_exec_stmt (entry_p);
- return (exec_p) ? bb_for_stmt (*exec_p) : NULL;
- }
-
/* Return the header block for the innermost switch statement containing
BB. Return NULL if BB is not inside a switch statement. */
--- 2910,2915 ----