[PATCH] D126807: [PowerPC] Fix LQ-STQ instructions to use correct offset and base

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:496
+    // This is a spill/restore of a quadword.
+    if ((Opcode == PPC::RESTORE_QUADWORD) || (Opcode == PPC::SPILL_QUADWORD)) {
+      LLVM_DEBUG(dbgs() << "Memory Operand: " << InstrInfo->getName(Opcode)
----------------
lkail wrote:
> This looks redundant. Test cases can pass without this change.
It would be difficult to write a test case that ensures all registers are allocated and we need to rely on this in order to guarantee that the register scavenger is able to spill a register to be able to scavenge one.


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:1589
+  // emitting the X-Form pseudo.
+  if (NewOpcode == PPC::LQX_PSEUDO || NewOpcode == PPC::STQX_PSEUDO) {
+    assert(is64Bit && "Quadword loads/stores only supported in 64-bit mode");
----------------
lkail wrote:
> Expanding two `PPCCustomInserterPseudo` instructions here looks odd.
> 
> I would expect these two instructions expanded after `prologepilog`, in `postrapseudos`. This is making us expand these two instructions twice in backend code. It's more adequate to make them `PPCPostRAExpPseudo` rather than `PPCCustomInserterPseudo`.
This code is the only place we convert D-Form instructions to X-Form instructions post-RA. Expanding them here makes sense because:
1. We create them here to begin with
2. Producing X-Forms is what this portion of the code already does
3. We have the register scavenger here and a slot saved for the scavenger to ensure it can always find a free GPR. If we do this expansion somewhere later, there is no guarantee that the scavenger we acquire will be able to scavenge a register.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126807