[llvm-commits] [llvm] r107529 - in /llvm/trunk: include/llvm/CodeGen/MachineInstr.h include/llvm/Target/Target.td include/llvm/Target/TargetOpcodes.h lib/CodeGen/LowerSubregs.cpp utils/TableGen/CodeGenTarget.cpp

[Chris Lattner]

On Jul 4, 2010, at 12:08 PM, Jakob Stoklund Olesen wrote:

>>> Add a new target independent COPY instruction and code to lower it.
>> 
>> Very interesting!  This means that the early code generator won't need to call the virtual method to determine if something is a copy?
> 
> Yes. I have clocked X86::isMoveInstr at around 5% CPU for both the fast register allocator and the coalescer.

Yeah, I can imagine it is expensive.  It is also annoying for target authors and is a poor abstraction.  It also makes it hard to read -print-machineinstr dumps on targets like ppc.

> There is a longer rationale here: http://lists.cs.uiuc.edu/pipermail/llvmdev/2010-June/032448.html

Ah, thanks I missed that in the post-wwdc surge :)

> 
>> This will also make targets that forget to implement the 'is copy' hook generate better code by default.
> 
> Yes. You also get foldMemoryOperand(COPY) --> storeToStackSlot / loadFromStackSlot for free. That is the only folding PowerPC does, for instance.

Right.  It is also non-obvious to target authors that they need to add this.

> I also intend to change the copyRegToReg hook to only allow physical register arguments, and eliminate the register class arguments.

Ok.  By virtue of taking vregs, the new COPY instruction will have regclass info.  I assume that COPY will also allow physregs, how will it (or will it?) represent regclass info for physregs?

>> This sounds like a great plan to me.  What is the migration plan for existing targets and existing codegen passes?  Do they need to do anything?
> 
> First I am going to emit EXTRACT_SUBREG as COPY so that instruction becomes selection DAG only. This is fairly easy since both are lowered by LowerSubregs. The problem is dealing with the FIXME in SimpleRegisterCoalescing::JoinIntervals regarding physical register live intervals.
> 
> Second, INSERT_SUBREG is converted to COPY by TwoAddressInstructionPass. It is still needed while the machine code is in SSA form.
> 
> Then it is a simple issue of replacing copyRegToReg with COPY instructions, and TII::isMoveInstr() with MI->isCopy.
> 
> Existing codegen passes need to use isCopy() instead of isExtractSubreg/isInsertSubreg/isMoveInstr. I am migrating  them by first adding isCopy, and later removing the other checks.
> 
> Targets need a new copyRegToReg implementation that takes no register class arguments but requires physical registers. I will start by providing a new version that falls back to the old one, so no target changes will be necessary initially.

This plan sounds great to me!

>> Do you think the "isMove" target hook can go away completely?
> 
> Yes, I think so. It depends on whether it is needed after LowerSubregs. A quick grep says that it isn't.

Nice.  Thanks for working on this,

-Chris