Re: [tree-ssa] bsi_insert routines

[Andrew MacLeod <amacleod at redhat dot com>]

On Sun, 2003-03-16 at 02:51, Daniel Berlin wrote:
> 
> 
> On Sat, 15 Mar 2003, Andrew MacLeod wrote:
> 
> > On Fri, 2003-03-14 at 18:05, Daniel Berlin wrote:
> >
> > > Errr, why are *you* updating the SSA more than calling modify_stmt?
> > >
> > > Calling get_stmt_operands in these functions means that one has to make
> > > modifications to the statement *before* inserting it.  If you make them
> > > after, you have to reupdate the statement again anyway.
> >
> > So you want to insert a stmt, and *then* change it? Why wouldn't you
> > create the stmt you want, and then insert it? If you don't do it that
> > way, how can anything be taken care of for you?  What changes are you
> > planning to make after inserting it?
> 
> Update the reaching def, which might not be able to be known before the
> insert.  This can happen in PRE + strength reduction.
> 

Don't get me wrong, Im not against taking the call to
get_stmt_operands() out, especially if it doesnt actually buy us
anything. 

My question is, are there any SSA related updates that the insert
routines should be aware of or need to do. Clearly, I will modify them
to do the basic block changes required, but on top of that, do we leave
everything else up to the optimizers?  I just want to make sure that we
funnel as much stuff as possible through common routines instead of
having every optimization do their changes their own way. 


> > Why do they have to be in pre order? we can update the BB and SSA web on
> > the fly stmt by stmt....  Anything that isnt resolved can be resolved
> > when the corresponding stmt is inserted.
> 
> But if it has to be resolved then anyway, *why call get_stmt_operands in
> the insertion functions*?
> Just mark the statement modified, and let the optimizers do it.
> 
> >  uses of non-existant defs and
> > goto's of non-existant labels aren't hard to deal with.
> 
> Of course they aren't hard, but you've missed the point again.


No, my point was to force the optimizations to pay attention to what
they are doing when they insert something by providing a central
consistant point through which SSA modifications happened. The original
interface way back when called for insert, delete, and replace. So if
you inserted a stmt, and then you wanted to change it, you notified the
system of the change by giving the new stmt to replace_stmt().

Now, that all said, perhaps it buys us nothing to do that for SSA
operand related things, such as whether new PHI nodes are needed, etc.
Perhaps it makes more sense to have a seperate routine or set of
routines for handling the cases which have side effects like that. Ie,
update_phi_node (def) when you've gone and added code which may affect
phi node insertions.

Perhaps the SSA builder is fast enough now that rebuilding SSA when its
too out of date is a better solution now. (Jeff/Diego?) As long as we
can go out of ssa and back in quickly enough...

I just want to make sure we establish a consistent interface that makes
sense instead of just updating things our own way in the locations we
need it. In particular, Im thinking forward to when large blocks of code
are replicated or added, such as in loop transformations. 

> 
> The SSA info will be out of date if it's modified outside of using
> bsi_insert*.

the original theory is that it would be modified in replace_stmt(),
which would be a member of the insert family. 

> This already happens in CCP (which does non-insertions), and can happen in
> any optimization that needs to modify something due to another insertion
> occurring (which can happen  whenever non-preorder DT order insertion occurs).
> 
> Watch:
> 
> a = 5
> b = 6
> c = a + b
> c = a + b
> 
> 
> Let's say i insert the reload here first, and thus, we get:
> 
> 
> a = 5
> b = 6
> c = a + b
> c = <no idea yet>
> 

Don't you know you are going to be making it a temp 
(ie, c = some.temp.9) , or are you inserting an invalid tree as a stmt,
or is it a tree which has arbitrary op[ernads which you are going to
change later?



> 
> Errr, remind me again why bsi_insert calling modify_stmt on the new
> statements, and having the optimizers use get_stmt_operands, *like they do
> now*, is not a good solution?
> It works, it's good, it doesn't waste any time (it won't update
> non-modified statements anyway).
> 

Again, im not suggesting the insert routines *have* to call
get_stmt_operands() after an insert. Im made them do so because I dont
think we should be inserting invalid trees into the stmt chain, and
get_stmt_operands() basically fills in the operand cache, which is going
to happen when the stmt is used anyway.
 
  My original intention was to actually do something with the operands
to keep some required SSA bits up to date next, but if there is no need
for that, then we dont have to make the call. Unless doing so forces a
certain level of responsibility on the clients that we decide is a good
thing. :-)

> I'm just trying to get you to remove the get_stmt_operands call in
> bsi_insert since it's not actually going to save any time, or make
> anything easier for optimizers.
>


So what this boils down to for me is a question of if and how we police
changes to the tree when its in SSA form.
- Do we simply allow arbitrary trees to be inserted
- Do we funnel any stmt change through a replace() routine, or do we
just plug new tree elements in where we want.
- Does this even effect the SSA operands anyway..

I can certainly be easily convinced to remove the call if its the
sensible thing to do. I wasn't expecting a large number of inserts that
didnt mean anything significant when they were actually inserted, so
filling the stmt operand cache seemed reasonable.  It looks like I wont
actually be using the bits myself in those routines, so maybe it does
make sense to remove it.

Andrew