Re: update address taken: don't drop clobbers

[Jeff Law <law at redhat dot com>]

On 10/15/14 08:35, Marc Glisse wrote:
> > Would that extra pass be acceptable?
> >
> > Otherwise, what do you think should be responsible for cleaning up the
> > dead assignments?
>
> Does anyone have an opinion on which side needs to be improved? As a
> reminder:
>
> - we have a va_list with its address taken by va_start/va_end.
> - fab lowers va_start/va_end and the list doesn't have its address taken
> anymore.
> - update_address_taken replaces the clobber: list =v {}; with an
> assignment of an undefined value: list_6 = list_2(D);
> - uninit warns about this.
>
> Some possible directions:
> - "prematurely" optimize in update_address_taken so we don't generate
> the useless assignment.
> - add a dce pass before uninit.
I tend to land on the side of minimizing false positives, so the comment 
about PR18501 is a "don't care" to me.  If the optimizers remove a dead 
assignment and we no longer warn about a potential uninitialized use in 
the dead assignment, then I consider that good.  Not everyone agrees 
with that way of thinking, obviously.

So my inclination would be to evaluate independent of the pr18501 
issues.  ie, what's the compile-time cost vs runtime benefit of running 
DCE here.  I'm guessing there's little runtime benefit for this 
particular case.

So my next line of thinking would be can we arrange to conditionally run 
DCE?  ie, have update_address_taken signal that it did something that 
has a reasonable chance of exposing dead code and only run DCE in that 
case.  Obviously this only helps if it rarely signals :-)  I don't think 
we have any infrastructure for this right now.

Finally I'd look at how difficult it would be to have 
update_address_taken cleanup after itself.   If the LHS is in SSA form, 
then if we find it has no uses, can we just remove the assignment 
completely?

jeff