[PATCH] D80163: [X86][VARARG] Avoid spilling xmm registers for va_start.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ExpandPseudo.cpp:531
+
+    GuardedBlock->addLiveIn(
+        SaveVarargXmmRegsInstr->getOperand(OpndIdx).getReg());
----------------
craig.topper wrote:
> Why do we only add them ass liveins this late? Shouldn't they have been livein when the block was created in the custom inserter?
Things were done that way(fill liveins when the block was created 
in the custom inserter) in the first version of the fix.
But it creates new liveness rule for physical registers
(newly added block should have physical registers in its liveins 
but it is not allowed before regalloc):

https://reviews.llvm.org/D69372?id=227838#change-TJhqJRPUjpDM

So there was a request to do things in other way, not requiring 
to change liveness rule:

https://reviews.llvm.org/D69372#1734756

The solution done in that patch creates a block with SAVE_VARARG_XMM_REGS
pseudo instruction. That pseudo instruction defines xmm registers 
(used by varargs). After register allocator, SAVE_VARARG_XMM_REGS pseudo 
instruction is expanded to the real copy instructions and put xmm registers 
into the block liveins. 


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31654
+
+  for (unsigned OpndIdx = 3, RegIdx = 0;
+       OpndIdx + 1 < SrcPseudoInstr.getNumOperands(); OpndIdx++, RegIdx++)
----------------
craig.topper wrote:
> Is RegIdx used here?
no. will delete it.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31657
+    MIB.addReg(SrcPseudoInstr.getOperand(OpndIdx).getReg(),
+               RegState::InternalRead);
+
----------------
craig.topper wrote:
> I'm not familiar with InternalRead. The documentation says it means it was defined by the same instruction. Is that case here?
that is how the solution works: SAVE_VARARG_XMM_REGS pseudo declares 
itself as an instruction which reads the registers defined by itself. And later 
(when SAVE_VARARG_XMM_REGS is expanded) the correct definitions 
would be put into the liveins.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80163/new/

https://reviews.llvm.org/D80163