[PowerPC][NFC] Fix bugs in r+r to r+i conversion

[nemanjai]

Summary

The D-Form VSX loads introduced in ISA 3.0 are not direct D-Form equivalent of the corresponding X-Forms since they only target the Altivec registers.
Namely LXSSPX can load into any of the 64 VSX registers whereas LXSSP can only load into the upper 32 VSX registers. Similarly with the remaining affected instructions.

There is currently no way that I can see to trigger the bug, but as we add other ways of exploiting these instructions, there may very well be instances that do.

This is an NFC patch in practical terms since the changes it introduces can not be triggered without an MIR test.

[steven.zhang]

The patch will disable some valid transformation post RA(i.e. LXSDX --> LXSD if the def reg is valid for D-form). We can continue to check the dst Reg to see if it is valid, instead of disabling it completely.

[nemanjai]

Modified the changeset to not reduce the capabilities of the pass (select the correct opcode based on the register for post-ra).

[jsji]

LGTM! Thanks for fixing!

[steven.zhang]

LGTM.

llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.h

public:
   bool isZeroExtended(const MachineInstr &MI, const unsigned depth = 0) const {
    return isSignOrZeroExtended(MI, false, depth);
   }
 
   bool convertToImmediateForm(MachineInstr &MI,
                               MachineInstr **KilledDef = nullptr) const;
   void replaceInstrWithLI(MachineInstr &MI, const LoadImmediateInfo &LII) const;
 
-  bool instrHasImmForm(const MachineInstr &MI, ImmInstrInfo &III) const;
+  bool instrHasImmForm(const MachineInstr &MI, ImmInstrInfo &III,
+                       bool PostRA) const;
 
   /// getRegNumForOperand - some operands use different numbering schemes
   /// for the same registers. For example, a VSX instruction may have any of
   /// vs0-vs63 allocated whereas an Altivec instruction could only have
   /// vs32-vs63 allocated (numbered as v0-v31). This function returns the actual
   /// register number needed for the opcode/operand number combination.
   /// The operand number argument will be useful when we need to extend this
   /// to instructions that use both Altivec and VSX numbering (for different

llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp

bool ConvertibleImmForm =
       Opc == PPC::CMPDI || Opc == PPC::CMPLDI ||
       Opc == PPC::ADDI || Opc == PPC::ADDI8 ||
       Opc == PPC::ORI || Opc == PPC::ORI8 ||
       Opc == PPC::XORI || Opc == PPC::XORI8 ||
       Opc == PPC::RLDICL || Opc == PPC::RLDICLo ||
       Opc == PPC::RLDICL_32 || Opc == PPC::RLDICL_32_64 ||
       Opc == PPC::RLWINM || Opc == PPC::RLWINMo ||
       Opc == PPC::RLWINM8 || Opc == PPC::RLWINM8o;
-    if (!instrHasImmForm(MI, III) && !ConvertibleImmForm)
+    if (!instrHasImmForm(MI, III, true) && !ConvertibleImmForm)
       return nullptr;
 
     // Don't convert or %X, %Y, %Y since that's just a register move.
     if ((Opc == PPC::OR || Opc == PPC::OR8) &&
         MI.getOperand(1).getReg() == MI.getOperand(2).getReg())
       return nullptr;
     for (int i = 1, e = MI.getNumOperands(); i < e; i++) {
       MachineOperand &MO = MI.getOperand(i);
bool PPCInstrInfo::convertToImmediateForm(MachineInstr &MI,
   assert(ForwardingOperand < MI.getNumOperands() &&
          "The forwarding operand needs to be valid at this point");
   bool KillFwdDefMI = !SeenIntermediateUse &&
     MI.getOperand(ForwardingOperand).isKill();
   if (KilledDef && KillFwdDefMI)
     *KilledDef = DefMI;
 
   ImmInstrInfo III;
-  bool HasImmForm = instrHasImmForm(MI, III);
+  bool HasImmForm = instrHasImmForm(MI, III, PostRA);
   // If this is a reg+reg instruction that has a reg+imm form,
   // and one of the operands is produced by an add-immediate,
   // try to convert it.
   if (HasImmForm && transformToImmFormFedByAdd(MI, III, ForwardingOperand,
                                                *DefMI, KillFwdDefMI))
     return true;
 
   if ((DefMI->getOpcode() != PPC::LI && DefMI->getOpcode() != PPC::LI8) ||
if (ReplaceWithLI) {
     replaceInstrWithLI(MI, LII);
     LLVM_DEBUG(dbgs() << "With:\n");
     LLVM_DEBUG(MI.dump());
     return true;
   }
   return false;
 }
 
+static bool isVFReg(unsigned Reg) {
+  return PPC::VFRCRegClass.contains(Reg);
+}
+
 bool PPCInstrInfo::instrHasImmForm(const MachineInstr &MI,
-                                   ImmInstrInfo &III) const {
+                                   ImmInstrInfo &III, bool PostRA) const {
   unsigned Opc = MI.getOpcode();
   // The vast majority of the instructions would need their operand 2 replaced
   // with an immediate when switching to the reg+imm form. A marked exception
   // are the update form loads/stores for which a constant operand 2 would need
   // to turn into a displacement and move operand 1 to the operand 2 position.
   III.ImmOpNo = 2;
   III.OpNoForForwarding = 2;
   III.ImmWidth = 16;
case PPC::STFDUX:
     case PPC::STDUX:
       III.ImmOpcode = PPC::STDU;
       III.ImmMustBeMultipleOf = 4;
       break;
     case PPC::STFSUX: III.ImmOpcode = PPC::STFSU; break;
     case PPC::STFDUX: III.ImmOpcode = PPC::STFDU; break;
     }
     break;
-  // Power9 only.
+  // Power9 and up only. For some of these, the X-Form version has access to all
+  // 64 VSR's whereas the D-Form only has access to the VR's. We replace those
+  // with pseudo-ops pre-ra and for post-ra, we check that the register loaded
+  // into or stored from is one of the VR registers.
   case PPC::LXVX:
   case PPC::LXSSPX:
   case PPC::LXSDX:
   case PPC::STXVX:
   case PPC::STXSSPX:
   case PPC::STXSDX:
+  case PPC::XFLOADf32:
+  case PPC::XFLOADf64:
+  case PPC::XFSTOREf32:
+  case PPC::XFSTOREf64:
     if (!Subtarget.hasP9Vector())
       return false;
     III.SignedImm = true;
     III.ZeroIsSpecialOrig = 1;
     III.ZeroIsSpecialNew = 2;
     III.IsCommutative = true;
     III.IsSummingOperands = true;
     III.ImmOpNo = 1;
     III.OpNoForForwarding = 2;
+    III.ImmMustBeMultipleOf = 4;
     switch(Opc) {
     default: llvm_unreachable("Unknown opcode");
     case PPC::LXVX:
       III.ImmOpcode = PPC::LXV;
       III.ImmMustBeMultipleOf = 16;
       break;
     case PPC::LXSSPX:
+      if (PostRA) {
+        if (isVFReg(MI.getOperand(0).getReg()))
           III.ImmOpcode = PPC::LXSSP;
-      III.ImmMustBeMultipleOf = 4;
+        else
+          III.ImmOpcode = PPC::LFS;
+        break;
+      }
+      LLVM_FALLTHROUGH;
+    case PPC::XFLOADf32:
+      III.ImmOpcode = PPC::DFLOADf32;
       break;
     case PPC::LXSDX:
+      if (PostRA) {
+        if (isVFReg(MI.getOperand(0).getReg()))
           III.ImmOpcode = PPC::LXSD;
-      III.ImmMustBeMultipleOf = 4;
+        else
+          III.ImmOpcode = PPC::LFD;
+        break;
+      }
+      LLVM_FALLTHROUGH;
+    case PPC::XFLOADf64:
+      III.ImmOpcode = PPC::DFLOADf64;
       break;
     case PPC::STXVX:
       III.ImmOpcode = PPC::STXV;
       III.ImmMustBeMultipleOf = 16;
       break;
     case PPC::STXSSPX:
+      if (PostRA) {
+        if (isVFReg(MI.getOperand(0).getReg()))
           III.ImmOpcode = PPC::STXSSP;
-      III.ImmMustBeMultipleOf = 4;
+        else
+          III.ImmOpcode = PPC::STFS;
+        break;
+      }
+      LLVM_FALLTHROUGH;
+    case PPC::XFSTOREf32:
+      III.ImmOpcode = PPC::DFSTOREf32;
       break;
     case PPC::STXSDX:
+      if (PostRA) {
+        if (isVFReg(MI.getOperand(0).getReg()))
           III.ImmOpcode = PPC::STXSD;
-      III.ImmMustBeMultipleOf = 4;
+        else
+          III.ImmOpcode = PPC::STFD;
+        break;
+      }
+      LLVM_FALLTHROUGH;
+    case PPC::XFSTOREf64:
+      III.ImmOpcode = PPC::DFSTOREf64;
       break;
     }
     break;
   }
   return true;
 }
 
 // Utility function for swaping two arbitrary operands of an instruction.

llvm/trunk/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.mir

bb.0.entry:
     %1 = COPY $x4
     %0 = COPY $x3
     %2 = COPY %1.sub_32
     %3 = ADDI %2, 1
     %5 = IMPLICIT_DEF
     %4 = INSERT_SUBREG %5, killed %3, 1
     %6 = LI8 100
     %7 = LXSDX %0, killed %6, implicit $rm :: (load 8 from %ir.arrayidx, !tbaa !12)
-    ; CHECK: LXSD 100, %0
-    ; CHECK-LATE: lxsd 0, 100(3)
+    ; CHECK: DFLOADf64 100, %0
+    ; CHECK-LATE: lfd 0, 100(3)
     %8 = ADDI %2, 2
     %10 = IMPLICIT_DEF
     %9 = INSERT_SUBREG %10, killed %8, 1
     %11 = LI8 -120
     %12 = LXSDX %0, killed %11, implicit $rm :: (load 8 from %ir.arrayidx3, !tbaa !12)
-    ; CHECK: LXSD -120, %0
-    ; CHECK-LATE: lxsd 1, -120(3)
+    ; CHECK: DFLOADf64 -120, %0
+    ; CHECK-LATE: lfd 1, -120(3)
     %13 = XSADDDP killed %7, killed %12, implicit $rm
     $f1 = COPY %13
     BLR8 implicit $lr8, implicit $rm, implicit $f1
 
 ...
 ---
 name:            testLXSSPX
 # CHECK-ALL: name: testLXSSPX
bb.0.entry:
     %1 = COPY $x4
     %0 = COPY $x3
     %2 = COPY %1.sub_32
     %3 = ADDI %2, 1
     %5 = IMPLICIT_DEF
     %4 = INSERT_SUBREG %5, killed %3, 1
     %6 = LI8 96
     %7 = LXSSPX %0, killed %6 :: (load 4 from %ir.arrayidx, !tbaa !14)
-    ; CHECK: LXSSP 96, %0
-    ; CHECK-LATE: lxssp 0, 96(3)
+    ; CHECK: DFLOADf32 96, %0
+    ; CHECK-LATE: lfs 0, 96(3)
     %8 = ADDI %2, 2
     %10 = IMPLICIT_DEF
     %9 = INSERT_SUBREG %10, killed %8, 1
     %11 = LI8 -92
     %12 = LXSSPX %0, killed %11 :: (load 4 from %ir.arrayidx3, !tbaa !14)
-    ; CHECK: LXSSP -92, %0
-    ; CHECK-LATE: lxssp 1, -92(3)
+    ; CHECK: DFLOADf32 -92, %0
+    ; CHECK-LATE: lfs 1, -92(3)
     %13 = XSADDSP killed %7, killed %12
     $f1 = COPY %13
     BLR8 implicit $lr8, implicit $rm, implicit $f1
 
 ...
 ---
 name:            testLXVX
 # CHECK-ALL: name: testLXVX
body:             |
   bb.0.entry:
     liveins: $x3, $f1, $x5
   
     %2 = COPY $x5
     %1 = COPY $f1
     %0 = COPY $x3
     %3 = LI8 444
     STXSSPX %1, %0, killed %3 :: (store 4 into %ir.arrayidx, !tbaa !14)
-    ; CHECK: STXSSP %1, 444, %0
-    ; CHECK-LATE: stxssp 1, 444(3)
+    ; CHECK: DFSTOREf32 %1, 444, %0
+    ; CHECK-LATE: stfs 1, 444(3)
     BLR8 implicit $lr8, implicit $rm
 
 ...
 ---
 name:            testSTXSDX
 # CHECK-ALL: name: testSTXSDX
 alignment:       4
 exposesReturnsTwice: false
body:             |
   bb.0.entry:
     liveins: $x3, $f1, $x5
   
     %2 = COPY $x5
     %1 = COPY $f1
     %0 = COPY $x3
     %3 = LI8 4
     STXSDX %1, %0, killed %3, implicit $rm :: (store 8 into %ir.arrayidx, !tbaa !12)
-    ; CHECK: STXSD %1, 4, %0
-    ; CHECK-LATE: stxsd 1, 4(3)
+    ; CHECK: DFSTOREf64 %1, 4, %0
+    ; CHECK-LATE: stfd 1, 4(3)
     BLR8 implicit $lr8, implicit $rm
 
 ...
 ---
 name:            testSTXVX
 # CHECK-ALL: name: testSTXVX
 alignment:       4
 exposesReturnsTwice: false