Trying to insert a COPY in foldMemoryOperandImpl()


I am working on a patch for the SystemZ backend that aims to make three-address instructions the default, since those instructions are not more expensive than their two-address counterparts, and this change helps the register allocator to reduce spilling and register moves.

However, the reg/memory instructions that can be used to fold a reload only accept a single register (corresponding to folding the RHS operand of the 2-address register instruction).

Since in many cases the 3-address instructions end up with a register allocation that could be used with the corresponding 2-address instruction (dst and LHS assigned to the same phys-reg), it would be nice to still fold the memory operand in these cases where RHS is spilled. This is a bit more involved than doing this for a 2-address instruction since the dst-reg and LHS reg have different virtual registers.

My progress so far can be found at A short example here is when %9 is spilled:

%20:gr64bit = AGRK %19:gr64bit, %9:gr64bit

By passing the VirtRegMap to foldMemoryOperandImpl(), it can be checked that %20 and %19 are allocated to the same phys-reg, which means that the reload of %9 can be folded into a single SystemZ instruction.

The problem at this point is that RA can still change its allocation for instance by evicting one of %20 and %19 and reassign that register to a different one. Therefore I had the idea of inserting a COPY before the AGRK, which would typically be deleted by MachineCopyProp as an identity copy after regalloc is done:

=> %9 ends up on the stack =>

%20:gr64bit = COPY %19:gr64bit
%20:gr64bit = AG %20:gr64bit(tied-def 0), %stack.0

My problem now is what to do about updating the LiveIntervals of %20 and %19.

I am not even sure if inserting a COPY here in the middle of the register allocation during spilling is feasible, and if so, what is the best way to do the updating?

Since the caller in this case (InlineSpiller) inserts any new instructions into SlotIndexes, I thought it would be logical for it to also handle the LiveInterval updating. It however does not do this currently (see Phabricator post for suggestion).

An alternative approach may be to tell InlineSpiller that the Target is taking care of the updating, and that way this simple case of inserting a COPY in the SystemZ backend could be handled by the backend.

So, in summary, does this patch seem reasonable, and how should the LiveIntervals best be updated? Any examples?

Thanks for any help,