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

Reply via email to