Re: [tree-ssa] empty_stmt_nodes, deleted statements, iterators, and gsi_step_bb

[Daniel Berlin <dberlin at dberlin dot org>]

> The drawback is that if a block has no statements, its not as
> convienient to add a statement.  ie, if we write a gsi_insert_after()
> routine, its likely to be used something like this:
*if*?

Diego's been promising one for a while, since SSAPRE needs it for 
adding to blocks with empty statements (i've presumed it would know 
what to do).

> gsi_stmt_interator i;
> i = gsi_start_bb (bb);
> ...
> n = new_stmt(...)
> gsi_insert_after (n, i)    /* or something like this.  */
>
> If there are nothing but empty_stmt_nodes in the block, gsi_start_bb
> will set the iterator to NULL. So when it comes time to add the
> statement, we don't even know what block we are adding this statement
> to, let alone where....
>
> The easiest thing to do to solve that problem would be to make the
> iterator have 2 words, the stmt pointer and the basic block it belongs
> to.
What makes you think this helps?

Because empty_stmt_node is unique, &empty_stmt_node is not the place 
you want to link the statement to anymore, because it's really a 
pointer to the unique empty_stmt_node (somewhere  in the global_trees 
array).
But that's what gets put in the head_tree_p/end_tree_p.
Thus, bb->head_tree_p/end_tree_p can't be replaced just by 
"*bb->head_tree_p = <whatever>"

You end up replacing what empty_stmt_node means if you do that.
I've done it.
Look at the SSAPRE code, search for "FIXME: This can't be fixed without 
the insertion machinery knowing what to do about it".

Trying to get it right is *incredibly* ugly unless you just stop making 
them unique, and thus, a special case.

Remember that you still have to find where the heck to actually link 
the statement into (remember the bb is on top of the trees, not in 
place of it, so just putting it in bb->head_tree_p in this case doesn't 
actually *do* anything to the code).

To do this without de-uniqifying, you actually need a parent_tree_p for 
bb's consisting of empty_stmt_node so you know where the actual 
empty_stmt_node is linked
I mentioned this to Diego a while ago, and he said he'd  worry  about 
it later.

> This would simplify many things I think, but it does have the
> drawback of passing a couple of words around as an iterator instead of
> just one. I doubt thats a huge deal, especially since a lot of routines
> which operate on interators are inlined. Perhaps there is a better
> option.
>
> What does everyone else think? Any other alternatives for tackling 
> this?
> what other issues are there going to be?
See above.
It doesn't help in doing insertion after/replacement of empty statement 
nodes, which is important.
The cleanest way to do that is to actually de-uniqify empty_stmt_node, 
rather than special casing them.
Whether this means adding a EMPTY_EXPR/EMPTY_STMT tree code to compare 
against (rather than pointer comparing empty_stmt_node), i'm not sure.

> Andrew
>
> PS We could also skip error_mark_nodes ther same way too I suppose,
> that's then remove the need to use is_exec_stmt() when traversing 
> within
> a block.
>
> PPS I also dont think its an issue that the non-bb versions of gsi_*
> work differently.... ? They give you an alternative which allows you to
> look at everything...