[PowerPC] Exploiting to use mtvsrdd instruction when save called-saved GPR register to VSR registers

[qiucf]

Summary

Make an exploiting to use mtvsrdd instruction when save called-saved GPR registers to VSR registers.
Because mtvsrdd can save two GPRs together at one time and make the space/time more efficient.

[jsji]

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

2265–2266

The comment are comparing to old implementation? This is unnecessary?

2273

No need to check hasP9Vector() here, we already early exit above when !Subtarget.hasP9Vector() in line 2107.

2283

Similar to above, we only enable spill to VSR for P9, so no need to check here.

2308

Can this be a lamda or function?

2313

Please add comments about the mapping from GPR to VSR?

2380

Do we want to save the two GPRs in the order of endian order? or maybe fixed litten endian?

2390

Never use assert(0...), that's what llvm_unreachable is for.

2550

Maybe the order here should be align with the restore order?

2560

Never use assert(0), that's what llvm_unreachable is for.

llvm/lib/Target/PowerPC/PPCFrameLowering.h

32

several GPRs? 1 or 2?

llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir

16 ↗

(On Diff #201814)

Please add comments about checking points.

23 ↗

(On Diff #201814)

It looks weird to have x14 before x15, although they should be the same.

[qiucf]

Rebase and address some basic comments

[qiucf]

Address comments

[qiucf]

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

2380

Here it always uses big-endian element order to save and restore, so it doesn't matter?

[nemanjai]

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

2259

This name is strange. We are not spilling VSR's here. Perhaps LastVSRUsedForSpill?

2310

I think an assert to ensure we are not trying to put more than two GPRs in a VSR should be added here.

2369

Please avoid asserts without text explaining the assert.

llvm/lib/Target/PowerPC/PPCFrameLowering.h

33

I am curious why this maps from unsigned. Why not from Register?
Also, I think it makes sense to map to std::pair since there will be (at most) two GPR's.

Finally, I think the name is confusing. I understand that it is mapping a VSR to GPRs, but since this optimization is moving stuff between registers, it can easily be misunderstood to mean that we are moving a VSR to GPRs. I think a better name would be something like VSRContainsGPRs.

[qiucf]

Address comments from Nemanja

llvm/lib/Target/PowerPC/PPCFrameLowering.h

33

Ah, I just used unsigned to align with class CalleeSavedInfo definition:

class CalleeSavedInfo {
  Register Reg;
  union {
    int FrameIdx;
    unsigned DstReg;
  };
  //...
};

[qiucf]

Ping..

[jsji]

LGTM, please wait a few days in case @nemanjai has more comments. Thanks.

[nemanjai]

LGTM.

llvm/lib/Target/PowerPC/PPCFrameLowering.h

class PPCFrameLowering: public TargetFrameLowering {
   const PPCSubtarget &Subtarget;
   const unsigned ReturnSaveOffset;
   const unsigned TOCSaveOffset;
   const unsigned FramePointerSaveOffset;
   const unsigned LinkageSize;
   const unsigned BasePointerSaveOffset;
   const unsigned CRSaveOffset;
 
+  // Map each group of one or two GPRs to corresponding VSR for spilling.
+  // TODO: Use local table in methods to avoid this mutable member.
+  mutable DenseMap<unsigned, std::pair<Register, Register>> VSRContainingGPRs;
+
   /**
    * Find register[s] that can be used in function prologue and epilogue
    *
    * Find register[s] that can be use as scratch register[s] in function
    * prologue and epilogue to save various registers (Link Register, Base
    * Pointer, etc.). Prefer R0/R12, if available. Otherwise choose whatever
    * register[s] are available.
    *

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

bool PPCFrameLowering::assignCalleeSavedSpillSlots(
   BitVector BVAllocatable = TRI->getAllocatableSet(MF);
   BitVector BVCalleeSaved(TRI->getNumRegs());
   const PPCRegisterInfo *RegInfo = Subtarget.getRegisterInfo();
   const MCPhysReg *CSRegs = RegInfo->getCalleeSavedRegs(&MF);
   for (unsigned i = 0; CSRegs[i]; ++i)
     BVCalleeSaved.set(CSRegs[i]);
 
   for (unsigned Reg : BVAllocatable.set_bits()) {
-    // Set to 0 if the register is not a volatile VF/F8 register, or if it is
+    // Set to 0 if the register is not a volatile VSX register, or if it is
     // used in the function.
-    if (BVCalleeSaved[Reg] ||
-        (!PPC::F8RCRegClass.contains(Reg) &&
-         !PPC::VFRCRegClass.contains(Reg)) ||
-        (MF.getRegInfo().isPhysRegUsed(Reg)))
+    if (BVCalleeSaved[Reg] || !PPC::VSRCRegClass.contains(Reg) ||
+        MF.getRegInfo().isPhysRegUsed(Reg))
       BVAllocatable.reset(Reg);
   }
 
   bool AllSpilledToReg = true;
+  unsigned LastVSRUsedForSpill = 0;
   for (auto &CS : CSI) {
     if (BVAllocatable.none())
       return false;
 
     unsigned Reg = CS.getReg();
-    if (!PPC::G8RCRegClass.contains(Reg) && !PPC::GPRCRegClass.contains(Reg)) {
+
+    if (!PPC::G8RCRegClass.contains(Reg)) {
       AllSpilledToReg = false;
       continue;
     }
 
+    // For P9, we can reuse LastVSRUsedForSpill to spill two GPRs
+    // into one VSR using the mtvsrdd instruction.
+    if (LastVSRUsedForSpill != 0) {
+      CS.setDstReg(LastVSRUsedForSpill);
+      BVAllocatable.reset(LastVSRUsedForSpill);
+      LastVSRUsedForSpill = 0;
+      continue;
+    }
+
     unsigned VolatileVFReg = BVAllocatable.find_first();
     if (VolatileVFReg < BVAllocatable.size()) {
       CS.setDstReg(VolatileVFReg);
-      BVAllocatable.reset(VolatileVFReg);
+      LastVSRUsedForSpill = VolatileVFReg;
     } else {
       AllSpilledToReg = false;
     }
   }
   return AllSpilledToReg;
 }
 
 bool PPCFrameLowering::spillCalleeSavedRegisters(
     MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
     ArrayRef<CalleeSavedInfo> CSI, const TargetRegisterInfo *TRI) const {
 
   MachineFunction *MF = MBB.getParent();
   const PPCInstrInfo &TII = *Subtarget.getInstrInfo();
   PPCFunctionInfo *FI = MF->getInfo<PPCFunctionInfo>();
   bool MustSaveTOC = FI->mustSaveTOC();
   DebugLoc DL;
   bool CRSpilled = false;
   MachineInstrBuilder CRMIB;
+  BitVector Spilled(TRI->getNumRegs());
+
+  VSRContainingGPRs.clear();
+
+  // Map each VSR to GPRs to be spilled with into it. Single VSR can contain one
+  // or two GPRs, so we need table to record information for later save/restore.
+  llvm::for_each(CSI, [&](const CalleeSavedInfo &Info) {
+    if (Info.isSpilledToReg()) {
+      auto &SpilledVSR =
+          VSRContainingGPRs.FindAndConstruct(Info.getDstReg()).second;
+      assert(SpilledVSR.second == 0 &&
+             "Can't spill more than two GPRs into VSR!");
+      if (SpilledVSR.first == 0)
+        SpilledVSR.first = Info.getReg();
+      else
+        SpilledVSR.second = Info.getReg();
+    }
+  });
 
   for (unsigned i = 0, e = CSI.size(); i != e; ++i) {
     unsigned Reg = CSI[i].getReg();
 
     // CR2 through CR4 are the nonvolatile CR fields.
     bool IsCRField = PPC::CR2 <= Reg && Reg <= PPC::CR4;
 
     // Add the callee-saved register as live-in; it's killed at the spill.
if (IsCRField) {
         MBB.insert(MI, CRMIB);
         MBB.insert(MI, addFrameReference(BuildMI(*MF, DL, TII.get(PPC::STW))
                                          .addReg(PPC::R12,
                                                  getKillRegState(true)),
                                          CSI[i].getFrameIdx()));
       }
     } else {
       if (CSI[i].isSpilledToReg()) {
-        NumPESpillVSR++;
-        BuildMI(MBB, MI, DL, TII.get(PPC::MTVSRD), CSI[i].getDstReg())
-          .addReg(Reg, getKillRegState(true));
+        unsigned Dst = CSI[i].getDstReg();
+
+        if (Spilled[Dst])
+          continue;
+
+        if (VSRContainingGPRs[Dst].second != 0) {
+          assert(Subtarget.hasP9Vector() &&
+                 "mtvsrdd is unavailable on pre-P9 targets.");
+
+          NumPESpillVSR += 2;
+          BuildMI(MBB, MI, DL, TII.get(PPC::MTVSRDD), Dst)
+              .addReg(VSRContainingGPRs[Dst].first, getKillRegState(true))
+              .addReg(VSRContainingGPRs[Dst].second, getKillRegState(true));
+        } else if (VSRContainingGPRs[Dst].second == 0) {
+          assert(Subtarget.hasP8Vector() &&
+                 "Can't move GPR to VSR on pre-P8 targets.");
+
+          ++NumPESpillVSR;
+          BuildMI(MBB, MI, DL, TII.get(PPC::MTVSRD),
+                  TRI->getSubReg(Dst, PPC::sub_64))
+              .addReg(VSRContainingGPRs[Dst].first, getKillRegState(true));
+        } else {
+          llvm_unreachable("More than two GPRs spilled to a VSR!");
+        }
+        Spilled.set(Dst);
       } else {
         const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
         // Use !IsLiveIn for the kill flag.
         // We do not want to kill registers that are live in this function
         // before their use because they will become undefined registers.
         // Functions without NoUnwind need to preserve the order of elements in
         // saved vector registers.
         if (Subtarget.needsSwapsForVSXMemOps() &&
bool PPCFrameLowering::restoreCalleeSavedRegisters(
   MachineFunction *MF = MBB.getParent();
   const PPCInstrInfo &TII = *Subtarget.getInstrInfo();
   PPCFunctionInfo *FI = MF->getInfo<PPCFunctionInfo>();
   bool MustSaveTOC = FI->mustSaveTOC();
   bool CR2Spilled = false;
   bool CR3Spilled = false;
   bool CR4Spilled = false;
   unsigned CSIIndex = 0;
+  BitVector Restored(TRI->getNumRegs());
 
   // Initialize insertion-point logic; we will be restoring in reverse
   // order of spill.
   MachineBasicBlock::iterator I = MI, BeforeI = I;
   bool AtStart = I == MBB.begin();
 
   if (!AtStart)
     --BeforeI;
if (Reg == PPC::CR2) {
         bool is31 = needsFP(*MF);
         restoreCRs(is31, CR2Spilled, CR3Spilled, CR4Spilled, MBB, I, CSI,
                    CSIIndex);
         CR2Spilled = CR3Spilled = CR4Spilled = false;
       }
 
       if (CSI[i].isSpilledToReg()) {
         DebugLoc DL;
-        NumPEReloadVSR++;
-        BuildMI(MBB, I, DL, TII.get(PPC::MFVSRD), Reg)
-            .addReg(CSI[i].getDstReg(), getKillRegState(true));
+        unsigned Dst = CSI[i].getDstReg();
+
+        if (Restored[Dst])
+          continue;
+
+        if (VSRContainingGPRs[Dst].second != 0) {
+          assert(Subtarget.hasP9Vector());
+          NumPEReloadVSR += 2;
+          BuildMI(MBB, I, DL, TII.get(PPC::MFVSRLD),
+                  VSRContainingGPRs[Dst].second)
+              .addReg(Dst);
+          BuildMI(MBB, I, DL, TII.get(PPC::MFVSRD),
+                  VSRContainingGPRs[Dst].first)
+              .addReg(TRI->getSubReg(Dst, PPC::sub_64), getKillRegState(true));
+        } else if (VSRContainingGPRs[Dst].second == 0) {
+          assert(Subtarget.hasP8Vector());
+          ++NumPEReloadVSR;
+          BuildMI(MBB, I, DL, TII.get(PPC::MFVSRD),
+                  VSRContainingGPRs[Dst].first)
+              .addReg(TRI->getSubReg(Dst, PPC::sub_64), getKillRegState(true));
+        } else {
+          llvm_unreachable("More than two GPRs spilled to a VSR!");
+        }
+
+        Restored.set(Dst);
+
       } else {
        // Default behavior for non-CR saves.
         const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
 
         // Functions without NoUnwind need to preserve the order of elements in
         // saved vector registers.
         if (Subtarget.needsSwapsForVSXMemOps() &&
             !MF->getFunction().hasFnAttribute(Attribute::NoUnwind))

llvm/test/CodeGen/PowerPC/prolog_vec_spills.mir

 # RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 -run-pass=prologepilog -ppc-enable-pe-vector-spills %s -o - | FileCheck %s
 
 ---
 name:            test1BB
 alignment:       16
 tracksRegLiveness: true
 liveins:
 body:             |
   bb.0.entry:
     $r14 = IMPLICIT_DEF
     $r15 = IMPLICIT_DEF
     $r16 = IMPLICIT_DEF
     $f0 = IMPLICIT_DEF
     $v20 = IMPLICIT_DEF
     BLR8 implicit undef $lr8, implicit undef $rm
 
+# Use mtvsrdd to save two GPRs in a single instruction
 # CHECK-LABEL: name:            test1BB
 # CHECK: body:             |
 # CHECK:      liveins: $x14, $x15, $x16, $v20
-# CHECK: $f1 = MTVSRD killed $x14
-# CHECK-NEXT: $f2 = MTVSRD killed $x15
-# CHECK-NEXT: $f3 = MTVSRD killed $x16
-# CHECK: $x16 = MFVSRD killed $f3
-# CHECK-NEXT: $x15 = MFVSRD killed $f2
-# CHECK-NEXT: $x14 = MFVSRD killed $f1
+# CHECK: $v0 = MTVSRDD killed $x14, killed $x15
+# CHECK-NEXT: $vf1 = MTVSRD killed $x16
+# CHECK: $x16 = MFVSRD killed $vf1
+# CHECK-NEXT: $x15 = MFVSRLD $v0
+# CHECK-NEXT: $x14 = MFVSRD killed $vf0
 ...
 
 ---
 name:            test2BBs
 alignment:       16
 tracksRegLiveness: true
 liveins:
 body:             |
body:             |
 
   bb.3:
     BLR8 implicit undef $lr8, implicit undef $rm
 
 ## The spilled-to registers have to be marked as live-in so that they will not be
 ## clobbered before restored in the epilogue.
 # CHECK-LABEL: name:            test2BB
 # CHECK: body:             |
-# CHECK:        $f0 = MTVSRD killed $x14
-# CHECK-NEXT:   $f1 = MTVSRD killed $x15
-# CHECK-NEXT:   $f2 = MTVSRD killed $x16
+# CHECK:        $v0 = MTVSRDD killed $x14, killed $x15
+# CHECK-NEXT:   $vf1 = MTVSRD killed $x16
 # CHECK:      bb.2:
 # CHECK-NEXT:   successors: %bb.3
-# CHECK-NEXT:   liveins: $f0, $f1, $f2
+# CHECK-NEXT:   liveins: $v0, $v1
 # CHECK:      bb.3:
-# CHECK-NEXT:   liveins: $f0, $f1, $f2
-# CHECK:        $x16 = MFVSRD killed $f2
-# CHECK-NEXT:   $x15 = MFVSRD killed $f1
-# CHECK-NEXT:   $x14 = MFVSRD killed $f0
+# CHECK-NEXT:   liveins: $v0, $v1
+# CHECK:        $x16 = MFVSRD killed $vf1
+# CHECK-NEXT:   $x15 = MFVSRLD $v0
+# CHECK-NEXT:   $x14 = MFVSRD killed $vf0
 ...