Re: [PR64164] drop copyrename, integrate into expand

[Jeff Law <law at redhat dot com>]

On 04/03/2015 07:28 AM, Alexandre Oliva wrote:
> On Mar 31, 2015, Richard Biener <richard.guenther@gmail.com> wrote:
>
> > > +                     || !(SSA_NAME_IS_DEFAULT_DEF (var)
> > > +                          || (param_defaults
> > > +                              && bitmap_bit_p (param_defaults, part))))
>
> > This looks somewhat awkward to me ;)  Is it really important to allow
> > coalescing PARM_DECL-based SSA vars with sth else?
>
> It's a valid optimization.  I can't say it's really important, but if
> the only objection is to param_defaults, I'm getting rid of it.
I doubt it's terribly important, but I agree it's a valid optimization. 
 Do you have a testcase where it triggers?  Can we include that too so 
that if someone wants to remove this later for some reason or another 
we'd at least have a chance of seeing a regression.

ISTM it can only trigger when the PARM is tied to another object via a 
copy and the PARM and other object have non-overlapping lifetimes.  I'd 
expect that this may happen at PHIs where the PARM appears on the RHS 
and dies at that point -- the PARM and the PHI result are likely not 
going to conflict and thus may coalesce.


> > At least abnormal coalescing doesn't need to do that, so just walking
> > over the function decls parameters and making their default-def live
> > should be enough?
>
> It should.  We'd have to duplicate logic of parameters, including static
> chain and whatnot.  I figured this would make it more resilient to
> changes elsewhere.
This ties in a bit to my comment about whether or not we've got proper 
life information for PARMs.  I'd generally prefer to see us get the life 
information corrrect.


> > > +      else if (TREE_CODE (SSA_NAME_VAR (var2)) == VAR_DECL)
> > > +       leader = SSA_NAME_VAR (var2);
> > > +      else /* What else could it be?  */
> > > +       gcc_unreachable ();
> > > +
>
> > definitely comments missing in this spaghetti...
>
> I'm trying to remove the spaghetting now.
Good :-)

> > or seeing this, why coalesce default-defs at all?
>
> Why not?  (the referenced code is gone from my local tree, BTW)
>
> > Either they are param values or they have indetermined values (and
> > thus we can and do pick whatever is available at expansion time)?
>
> If they are param values, we want to have them available; if they
> aren't, whatever we coalesce with is good.
Agreed.  Didn't we recently change the coalescing code to allow 
coalescing non-PARM default defs more aggressively:

Author: glisse <glisse@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Nov 3 10:47:04 2014 +0000

    2014-11-03  Marc Glisse  <marc.glisse@inria.fr>

        PR tree-optimization/60770
    gcc/
        * tree-into-ssa.c (rewrite_update_stmt): Return whether the
        statement should be removed.
        (maybe_register_def): Likewise. Replace clobbers with default
        definitions.
        (rewrite_dom_walker::before_dom_children): Remove statement if
        rewrite_update_stmt says so.
        * tree-ssa-live.c: Include tree-ssa.h.
        (set_var_live_on_entry): Do not mark undefined variables as live.
        (verify_live_on_entry): Do not check undefined variables.
        * tree-ssa.h (ssa_undefined_value_p): New parameter for the case
        of partially undefined variables.
        * tree-ssa.c (ssa_undefined_value_p): Likewise.
        (execute_update_addresses_taken): Do not drop clobbers.

    gcc/testsuite/
        * gcc.dg/tree-ssa/pr60770-1.c: New file.

 > guess some statistics wouldn't be a bad idea.  Is there any specific
> testcase you're interested in, or something like a GCC bootstrap or
> somesuch?
Not from me.  bootstrap or .i files from gcc bootstrap would seem to be 
sufficient to me.

jeff