Re: [patch] Some operands building cleanups

[Andrew MacLeod <amacleod at redhat dot com>]

> > It is really a bug, it happens to be a rare end-case and we just have
> > not triggered it. Someone will eventually trigger it and it will be a
> > nightmare to find.
> 
> actually, I hit it once; I just fixed the pass not to call update_stmt
> while iterating over immediate uses.
> 

You wouldn't have had to change the pass if they bug wasn't there :-)
It may not be possible to do this without a lot of hassle in some cases.

> > > Also, the p
> > 
> > Leaving potential bugs is always a bad idea, no matter how unlikely they
> > seem.
> 
> However, it seems to me that allowing changing immediate uses (by calling
> update_stmt) when iterating over them is very dangerous idea by itself.
> Even if we are very careful in update_stmt to preserve the old operands
> exactly, new operands appearing during the scan may cause problems (with
> the current implementation of the "safe" iterator, they won't be
> scanned, which is not neccessarily what the optimization intended).

its how it is defined. It works fine. If you add a new use, you already
know where it is an would presumably update it yourself. visiting new
ones is asking for an infinite loop. The whole point of the iterator is
to find the ones you don't know where they are.

> 
> I propose to add assert to update_stmt that we are not currently
> iterating over immediate uses (and fix the places that use update_stmt
> this way) -- I will prepare the patch if you agree.


I'd rather see the bug fixed than just add a limitation to the immediate
use iterator which then forces changes to optimizations which have been
written using the existing definition of how it works. 

I think visiting each stmt that has uses of a variable, making changes
to the stmt (which may cause an update), and going on to the next stmt
is perfectly reasonable and wouldn't want to prevent that.  

Andrew