Re: [PR64164] drop copyrename, integrate into expand

[Jeff Law <law@redhat.com>]

On 04/03/2015 07:17 AM, Alexandre Oliva wrote:
>>> @@ -890,6 +900,36 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo)
>>> live_track_process_def (live, result, graph);
>>> }
>>>
>>> +      /* Pretend there are defs for params' default defs at the start
>>> +	 of the (post-)entry block.  We run after abnormal coalescing,
>>> +	 so we can't assume the leader variable is the default
>>> +	 definition, but because of SSA_NAME_VAR adjustments in
>>> +	 attempt_coalesce, we can assume that if there is any
>>> +	 PARM_DECL in the partition, it will be the leader's
>>> +	 SSA_NAME_VAR.  */
>
> This comment is outdated.  Since we no longer have abnormal coalescing
> before building the conflict graph, we can just test whether each
> SSA_NAME is a default def for a PARM_DECL and be done with it.
OK.  Please update the comment :-0

>
>> So the issue here is you want to iterate over the objects live at the
>> entry block, which would include any SSA_NAMEs which result from
>> PARM_DECLs.  I don't guess there's an easier way to do that other than
>> iterating over everything live in that initial block?
>
> We could iterate over all SSA_NAMEs, but that would probably be more
> costly.  There shouldn't be very many live variables at the function
> entry, so using the live bitmaps is likely to save us time, especially
> on functions with lots of SSA_NAMEs.
Agreed.  Iterating over the SSA_NAMEs was what came to mind when I 
pondered this a bit more and I'd rejected it for the same reason.

Can we get to the SSA_NAMEs associated with the PARM_DECLs from the 
function decl?  I can't think of a way off the top of my head, but if we 
could, then that'd avoid the iteration over the bitmap of live variables.

But then again, the bitmap of live variables ought to be small, 
particularly if we're not marking non-PARM default defs as live anymore 
(see patch reference in my prior message).

>> So the bulk of the changes into this routine are really about picking
>> a good leader, which presumably is how we're able to get the desired
>> effects on debuginfo that we used to get from tree-ssa-copyrename.c?
>
> This has nothing to do with debuginfo, I'm afraid.  We just had to keep
> track of parm and result decls to avoid coalescing them together, and
> parm decl default defs to promote them to leaders, because expand copies
> incoming REGs to pseudos in PARM_DECL's DECL_RTL.  We should fill that
> in with the RTL created for the default def for the PARM_DECL.  At the
> end, I believe we also copy the RESULT_DECL DECL_RTL to the actual
> return register or rtl.  I didn't want to tackle the reworking of these
> expanders to avoid problems out of copying incoming parms to one pseudo
> and then reading it from another, as I observed before I made this
> change.  I'm now tackling that, so that we can refrain from touching the
> base vars altogether, and not refrain from coalescing parms and results.
Hmmm, so the real issue here is the expansion setup of parms and 
results.  I hadn't pondered that aspect.  I'd encourage fixing the 
expansion code too if you can see a path for that.

Basically I just don't like special casing things like this -- 
coalescing should be driven by life information/conflict graph and a 
copy relationship between the two candidate objects.

Overall it looks like you're on the right path and we'll just need to 
iterate a bit more.

jeff