On Sat, Sep 22, 2012 at 4:08 AM, Dehao Chen <de...@google.com> wrote:
> Hi,
>
> The problem is due to a bug when move_block_to_fn. edge->goto_block
> should be updated even when its locus is unknown. This patch also
> fixes the way to reset block for expr.
I think the machinery is much more fragile (which is probably why
when in SSA form the whole thing is invoked with a NULL_TREE BLOCK).
>From looking at how the block tree is mangled:
/* Rewire BLOCK_SUBBLOCKS of orig_block. */
if (orig_block)
{
tree block;
gcc_assert (BLOCK_SUBBLOCKS (DECL_INITIAL (dest_cfun->decl))
== NULL_TREE);
BLOCK_SUBBLOCKS (DECL_INITIAL (dest_cfun->decl))
= BLOCK_SUBBLOCKS (orig_block);
for (block = BLOCK_SUBBLOCKS (orig_block);
block; block = BLOCK_CHAIN (block))
BLOCK_SUPERCONTEXT (block) = DECL_INITIAL (dest_cfun->decl);
BLOCK_SUBBLOCKS (orig_block) = NULL_TREE;
}
I think we want a simpler version of your fix:
> Bootstrapped and pass all gcc regression tests.
>
> Is it okay for trunk?
>
> Thanks,
> Dehao
>
> gcc/ChangeLog:
> tree-cfg.c (move_stmt_op): Reset the expr block only
> when necessary.
> (move_block_to_fn): Reset the edge's goto block even
> when the goto locus is unknown.
>
> Index: tree-cfg.c
> ===================================================================
> --- tree-cfg.c (revision 191614)
> +++ tree-cfg.c (working copy)
> @@ -6013,7 +6013,9 @@ move_stmt_op (tree *tp, int *walk_subtrees, void *
>
> if (EXPR_P (t))
> {
> - if (TREE_BLOCK (t))
> + if (p->orig_block == NULL_TREE
> + || TREE_BLOCK (t) == p->orig_block
> + || TREE_BLOCK (t) == NULL_TREE)
> TREE_SET_BLOCK (t, p->new_block);
Only replace TREE_BLOCK of t if it was equal to p->orig_block or if
p->orig_block is NULL_TREE _and_ TREE_BLOCK of t is not NULL_TREE.
> }
> else if (DECL_P (t) || TREE_CODE (t) == SSA_NAME)
> @@ -6315,7 +6317,7 @@ move_block_to_fn (struct function *dest_cfun, basi
> }
>
> FOR_EACH_EDGE (e, ei, bb->succs)
> - if (!IS_UNKNOWN_LOCATION (e->goto_locus))
> + if (e->goto_locus)
Please compare against UNKNOWN_LOCATION (which is zero, right)?
Ok with that changes.
Thanks,
Richard.
> {
> tree block = LOCATION_BLOCK (e->goto_locus);
> if (d->orig_block == NULL_TREE
