[PATCH] CSE removes COPY.

[Quentin Colombet]

On May 29, 2014, at 3:37 PM, Andrew Trick <atrick at apple.com> wrote:

> You're proposing removing cross-class copies, which is not necessarilly a good idea. I expect that if we have a cross-class copy in MI then it's there for a reason. Note that we should remove cross class copies like this:
> 
> r1 = a1
> a2 = r1
> 
> I don't think we're handling the above case yet but we should fix that. Maybe as part of CSE or maybe earlier. Quentin, do you know if we have a bug on this?

I think
r1 = a1
a2 = r1
should already be supported by the peephole optimizer.
Indeed, it should generate:
r1 = a1
a2 = a1

What is not supported and needed in this case is:
r1 = a1
r2 = a1

Indeed, when we try to find a more suitable source for the copy r2 = a1, we follow the use-def chain of a1 and fail to see that r1 would be available. Though that would require more work because we would have to check for dominance, because it was implicitly correct when we follow the use-def chain.

> 
> Regarding subregister copies, I don't think this is a good fix and I think the FIXME is still relevant.
I agree that this patch does not fix the sub register copies but works around the problem.
That’s why I recommended that at least we have a comment on how it works around that.

> This fix happens to work when the use of the subreg copies gets CSE'd. But what if it doesn't. Then we have a subreg copy with multiple uses and a large live range. Subreg copies are conceptually part of the use's operand. We should really eliminate them completely, but if we can't then the should be single-use copies with minimal live ranges so that they have the same liveness as they would being part of the operand.
> 
> Quentin understands these issue better, so I'll defer to him. I would not oppose a subtarget option to allow cse'ing copies until we have a better way to handle your cases (-machine-cse-copies). However, when the flag is disabled, the logic should remain as it was, and the FIXME should not be removed. Also please make sure the cse-add-with-overflow.ll test still tests for subreg copy coalescing, not subreg copy CSE as you have done.

Double on that (in case that was not clear from my previous comment :)).

Q.
> 
> http://reviews.llvm.org/D3948
> 
>