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