[PATCH] CSE removes COPY.

[Quentin Colombet]

Hi Danil,

> 1. The reason why CSE ignored COPY is PerformTrivialCoalescing.
> If COPY insert into HASH and perform PerformTrivialCoalescing: MI->eraseFromParent()
HASH will be broken.

That is not the only reason. With your patch, most of the single used copies will be coalesced by this method. This method does not use the profitability model of the register coalescer and thus different pairs may be coalesced.
The problem is that this may create over constrained, in terms of register allocation, live-ranges (since they are longer) and may produce more spill code or expensive, not coalescable, copies.

At the very least, you will have to provide performance numbers for your patch.

That said, if we switch the default of cse-ignore-copies to true (instead of false in your current patch), your patch shouldn’t change the current behavior AFAICT. (A diff of the llvm-testsuite binaires with and without your patch may be a good check.)
That way, the patch could be more easily tested by the community and you could already use it for your target.

How does that sound?

> 2) cross-regclass copy:

The peephole optimizer has an optimization that rewrites cross-register bank copies. It does not know how to handle your case, but it could be extended to do that.

> 3) subreg copy

Your patch does not handle the subreg copy per say, but the scheme seems indeed natural. Maybe add a comment about that when we abort on subregs in PerformTrivialCoalescing.
BTW, you do not need to modify the test case.

> 4) problem with CodeGen/X86/inline-asm-fpstack.ll:

This is another argument why we should disable that optimization by default. Until we understand what is going on, we do not know the impact of this.

Finally, please run clang-format. You have some weird spacing and some (at least one) ^M characters.

Thanks,
Quentin

http://reviews.llvm.org/D3948