Re: update address taken: don't drop clobbers

[Jeff Law <law@redhat.com>]

On 07/06/14 08:23, Marc Glisse wrote:
> What is the lifetime of an SSA_NAME with a default definition? The way
> we handle it now, we start from the uses and go back to all blocks that
> can reach one of the uses, since there is no defining statement where we
> could stop
Right, that's exactly what I would expect given the typical definition 
of liveness.


  (intuitively we should stop where the clobber was, if not
> earlier). This huge lifetime makes it very likely for such an SSA_NAME
> to conflict with others. And if an abnormal phi is involved, and thus we
> must coalesce, there is a problem.
So this suggests another approach.  Could we just assume that it's 
always safe to coalesce with a default definition?


>
> The patch attached (it should probably use ssa_undefined_value_p with a
> new extra argument to say whether we care about partially-undefined)
> makes the lifetime of undefined local variables empty and lets the
> original patch work with:
>        def = get_or_create_ssa_default_def (cfun, sym);
> instead of creating a new variable.
Not a terrible suggestion either and accomplishes the same thing as the 
mental model I had of special casing this stuff in the coalescing code.

I guess this is the first thing we probably should sort out.  Do we want 
to handle this is the conflict, coalescing or lifetime analysis.

We've certainly had similar hacks in the code which built the conflict 
matrix for the register allocator in the past (old local-alloc/global 
variant, pre IRA).   By not recoding those conflicts, coalescing just 
magically works.

By handling it when we build the lifetimes, they'll magically coalesce 
because there's no conflicts as well.  It may help other code which 
utilizes lifetimes as well.  But a part of me doesn't like it because it 
is different than the traditionally formulated life analysis.

The question I havent thought much about is would the coalescing code do 
the right thing if we have one of these undefined SSA_NAMEs that 
coalesce into two different partitions.

ie, let's say we have two PHI nodes in different blocks.  Each PHI has 
one or more source/dest operations that are marked as 
SSA_NAME_OCCURS_IN_ABNORMAL_PHI.  Furthermore, they all reference a 
single RHS undefined SSA_NAME.

When we coalesce the undefined SSA_NAME with all the others in the first 
PHI, doesn't that mean that we then have to coalesce all the SSA_NAMES 
in both PHIs together (because that undefined SSA_NAME appears on the 
RHS on the 2nd PHI too).

Does this then argue that each use of an undefined SSA_NAME should be a 
unique SSA_NAME?

Ugh, I wonder if I just opened a can of worms here...

>
> However, I am not convinced reusing the same variable is necessarily the
> best thing. For warnings, we can create a new variable with the same
> name (with .1 added by gcc at the end) and copy the location info (I am
> not doing that yet), so little is lost. A new variable expresses more
> clearly that the value it holds is random crap. If I reuse the same
> variable, the SRA patch doesn't warn because SRA explicitly sets
> TREE_NO_WARNING (this can probably be changed, but that's starting to be
> a lot of changes). Creating a new variable is also more general. When
> reading *ssa_name after *ssa_name={v}{CLOBBER}; or after free(ssa_name);
> we have no variable to reuse so we will need to create a new undefined
> variable, and if a variable is global or a PARM_DECL, its default
> definition is not an undefined value (though it will probably happen in
> a different pass, so it doesn't have to behave the same).
My concern about using another variable was primarily that it was being 
used to hide a deeper issue.


>
> (Side note: we don't seem to have any code to notice that:
> a=phi<undef,b>
> b=phi<undef,a>
> means both phis can be replaced with undefined variables.)
Various passes will consider the undef value to be whatever value is 
convenient for optimization.  So for the first, we'd consider the undef 
value to be the same as "b" because that allows the PHI to be propagated 
away.

Obviously when there are many different values in the RHS, it's a lot 
less likely that we'll be able to propagate away the PHI.

Jeff