Hi,
On Thu, 3 Nov 2011, Richard Guenther wrote:
> > + /* Add clobbers for all variables that go out of scope. */
> > + for (t = BIND_EXPR_VARS (bind_expr); t ; t = DECL_CHAIN (t))
> > + {
> > + if (TREE_CODE (t) == VAR_DECL
> > + && !is_global_var (t)
>
> I think you want to use auto_var_in_fn () here.
I don't think so. It accepts LABEL_DECLs (and others, though I'm only
interested in VAR_DECLs here), and barring the check on DECL_CONTEXT
(which I don't need) it's exactly equivalent to !is_global_var.
> > + && !DECL_HARD_REGISTER (t)
> > + && !TREE_THIS_VOLATILE (t)
>
> Any reason for that?
We must not introduce new writes (which the clobbers are) to
volatile storage. And HARD_REGs obviously never will be placed on the
stack.
> > + && !DECL_HAS_VALUE_EXPR_P (t)
> > + /* Only care for variables that have to be in memory. Others
> > + will be rewritten into SSA names, hence moved to the
> > top-level. */
> > + && needs_to_live_in_memory (t))
>
> Looking at the predicate this will exclude non-address-taken aggregates.
> We talked about this as an optimization, but that would need a real
> computation of a life problem for those.
Hmm? The whole thing would work exactly the same for address-taken and
non-address-taken aggregates.
> Thus, does this mean that non-address-taken aggregates won't get their
> stack slots shared right now?
Correct. I considered them unimportant enough to not deserve the special
treatment. There aren't many things you can do with non-address-taken
aggregates that SRA wouldn't decompose.
> > + if (!decl_to_stack_part)
> > + decl_to_stack_part = pointer_map_create ();
> > +
> > v = &stack_vars[stack_vars_num];
> > + * (size_t *)pointer_map_insert (decl_to_stack_part, decl) =
> > stack_vars_num;
>
> Use uintptr_t.
Oh? Since when can we use that? It's used nowhere else in GCC.
> > + if (gimple_assign_single_p (stmt)
> > + && TREE_CLOBBER_P (gimple_assign_rhs1 (stmt)))
>
> Can you introduce a gimple_clobber_p (stmt) predicate for this?
Yes.
> (not sure if I like "clobber" too much, that sounds like a may-kill
> while this should have the semantic of a true kill operation, thus,
> maybe change s/clobber/kill/ throughout the patch?)
Ah, bikeshedding :) A clobber is indeed a may-kill. The indeterminate
value stored into it may as well be the original content (without that
it's a must-kill). Like for a kill, which also seems to imply an
indeterminate value. From that perspective also a kill is a may-kill, or
IOW "clobber" and "kill" are equivalent. I don't like the notion of
"killing" variables or storage, they remain in existence, just the
contents aren't, I really prefer clobber.
> > Index: tree-ssa-sccvn.c
> > ===================================================================
> > --- tree-ssa-sccvn.c.orig 2011-11-02 21:18:45.000000000 +0100
> > +++ tree-ssa-sccvn.c 2011-11-02 21:19:51.000000000 +0100
> > @@ -1422,6 +1422,7 @@ vn_reference_lookup_3 (ao_ref *ref, tree
> > else if (is_gimple_reg_type (vr->type)
> > && gimple_assign_single_p (def_stmt)
> > && gimple_assign_rhs_code (def_stmt) == CONSTRUCTOR
> > + && !TREE_CLOBBER_P (gimple_assign_rhs1 (def_stmt))
>
> Does that really matter? We can as well assume zeros for the new content.
No, we cannot. If we assume so then a second store also storing zeros
into it might be removed because it's "already filled with zeros", where
at runtime this doesn't hold and the second store would have been
significant:
for (i<N) { aggr.i=0; use(&aggr); aggr <= clobber }
-->
aggr.i=0; for (i<N) { use(&aggr); aggr <= clobber }
Ciao,
Michael.
