[PowerPC] Remove the implicit use of the register if it is replaced by Imm

[steven.zhang]

Summary

If we are changing the MI operand from Reg to Imm, we need also handle its implicit use if have.

[hfinkel]

LGTM

llvm/lib/Target/PowerPC/PPCInstrInfo.h

public:
   /// i.e. 0 to 31-th bits are all zeros
   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;
+  void replaceInstrOperandWithImm(MachineInstr &MI, unsigned OpNo,
+                                  int64_t Imm) 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

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

case PPC::sub_gt:
       return (uint64_t)Imm1 > (uint64_t)Imm2 ? TrueReg : FalseReg;
     case PPC::sub_eq:
       return Imm1 == Imm2 ? TrueReg : FalseReg;
     }
   }
   return PPC::NoRegister;
 }
 
+void PPCInstrInfo::replaceInstrOperandWithImm(MachineInstr &MI,
+                                              unsigned OpNo,
+                                              int64_t Imm) const {
+  assert(MI.getOperand(OpNo).isReg() && "Operand must be a REG");
+  // Replace the REG with the Immediate.
+  unsigned InUseReg = MI.getOperand(OpNo).getReg();
+  MI.getOperand(OpNo).ChangeToImmediate(Imm);
+
+  if (empty(MI.implicit_operands()))
+    return;
+
+  // We need to make sure that the MI didn't have any implicit use
+  // of this REG any more.
+  const TargetRegisterInfo *TRI = &getRegisterInfo();
+  int UseOpIdx = MI.findRegisterUseOperandIdx(InUseReg, false, TRI);
+  if (UseOpIdx >= 0) {
+    MachineOperand &MO = MI.getOperand(UseOpIdx);
+    if (MO.isImplicit())
+      // The operands must always be in the following order:
+      // - explicit reg defs,
+      // - other explicit operands (reg uses, immediates, etc.),
+      // - implicit reg defs
+      // - implicit reg uses
+      // Therefore, remove the implicit operand won't change the explicit
+      // operands layout.
+      MI.RemoveOperand(UseOpIdx);
+  }
+}
+
 // Replace an instruction with one that materializes a constant (and sets
 // CR0 if the original instruction was a record-form instruction).
 void PPCInstrInfo::replaceInstrWithLI(MachineInstr &MI,
                                       const LoadImmediateInfo &LII) const {
   // Remove existing operands.
   int OperandToKeep = LII.SetCR ? 1 : 0;
   for (int i = MI.getNumOperands() - 1; i > OperandToKeep; i--)
     MI.RemoveOperand(i);
for (auto &CompareUseMI : MRI->use_instructions(DefReg)) {
       unsigned FalseReg = CompareUseMI.getOperand(2).getReg();
       unsigned RegToCopy = selectReg(SExtImm, SExtComparand, Opc, TrueReg,
                                      FalseReg, CRSubReg);
       if (RegToCopy == PPC::NoRegister)
         continue;
       // Can't use PPC::COPY to copy PPC::ZERO[8]. Convert it to LI[8] 0.
       if (RegToCopy == PPC::ZERO || RegToCopy == PPC::ZERO8) {
         CompareUseMI.setDesc(get(UseOpc == PPC::ISEL8 ? PPC::LI8 : PPC::LI));
-        CompareUseMI.getOperand(1).ChangeToImmediate(0);
+        replaceInstrOperandWithImm(CompareUseMI, 1, 0);
         CompareUseMI.RemoveOperand(3);
         CompareUseMI.RemoveOperand(2);
         continue;
       }
       LLVM_DEBUG(
           dbgs() << "Found LI -> CMPI -> ISEL, replacing with a copy.\n");
       LLVM_DEBUG(DefMI->dump(); MI.dump(); CompareUseMI.dump());
       LLVM_DEBUG(dbgs() << "Is converted to:\n");
bool PPCInstrInfo::transformToImmFormFedByAdd(MachineInstr &MI,
   MI.getOperand(III.OpNoForForwarding).ChangeToRegister(RegMO->getReg(),
                                                         false, false,
                                                         RegMO->isKill());
 
   // Then, update the imm.
   if (ImmMO->isImm()) {
     // If the ImmMO is Imm, change the operand that has ZERO to that Imm
     // directly.
-    MI.getOperand(III.ZeroIsSpecialOrig).ChangeToImmediate(Imm);
+    replaceInstrOperandWithImm(MI, III.ZeroIsSpecialOrig, Imm);
   }
   else {
     // Otherwise, it is Constant Pool Index(CPI) or Global,
     // which is relocation in fact. We need to replace the special zero
     // register with ImmMO.
     // Before that, we need to fixup the target flags for imm. 
     // For some reason, we miss to set the flag for the ImmMO if it is CPI.
     if (DefMI.getOpcode() == PPC::ADDItocL)
if (SpecialShift32 || SpecialShift64) {
         MI.setDesc(get(PPC::COPY));
       } else {
         // The 32 bit and 64 bit instructions are quite different.
         if (SpecialShift32) {
           // Left shifts use (N, 0, 31-N), right shifts use (32-N, N, 31).
           uint64_t SH = RightShift ? 32 - ShAmt : ShAmt;
           uint64_t MB = RightShift ? ShAmt : 0;
           uint64_t ME = RightShift ? 31 : 31 - ShAmt;
-          MI.getOperand(III.OpNoForForwarding).ChangeToImmediate(SH);
+          replaceInstrOperandWithImm(MI, III.OpNoForForwarding, SH);
           MachineInstrBuilder(*MI.getParent()->getParent(), MI).addImm(MB)
             .addImm(ME);
         } else {
           // Left shifts use (N, 63-N), right shifts use (64-N, N).
           uint64_t SH = RightShift ? 64 - ShAmt : ShAmt;
           uint64_t ME = RightShift ? ShAmt : 63 - ShAmt;
-          MI.getOperand(III.OpNoForForwarding).ChangeToImmediate(SH);
+          replaceInstrOperandWithImm(MI, III.OpNoForForwarding, SH);
           MachineInstrBuilder(*MI.getParent()->getParent(), MI).addImm(ME);
         }
       }
     } else
-      MI.getOperand(ConstantOpNo).ChangeToImmediate(Imm);
+      replaceInstrOperandWithImm(MI, ConstantOpNo, Imm);
   }
   // Convert commutative instructions (switch the operands and convert the
   // desired one to an immediate.
   else if (III.IsCommutative) {
-    MI.getOperand(ConstantOpNo).ChangeToImmediate(Imm);
+    replaceInstrOperandWithImm(MI, ConstantOpNo, Imm);
     swapMIOperands(MI, ConstantOpNo, III.OpNoForForwarding);
   } else
     llvm_unreachable("Should have exited early!");
 
   // For instructions for which the constant register replaces a different
   // operand than where the immediate goes, we need to swap them.
   if (III.OpNoForForwarding != III.ImmOpNo)
     swapMIOperands(MI, III.OpNoForForwarding, III.ImmOpNo);

llvm/test/CodeGen/PowerPC/remove-implicit-use.mir

+# RUN: llc -mtriple=powerpc64le-unknown-unknown -start-after=ppc-mi-peepholes \ 
+# RUN: -stop-before=ppc-expand-isel -verify-machineinstrs %s -o - | FileCheck %s
+--- |
+  ; ModuleID = 'a.ll'
+  source_filename = "a.c"   
+  target datalayout = "e-m:e-i64:64-n32:64"
+  target triple = "powerpc64le-unknown-linux-gnu"
+
+  ; Function Attrs: norecurse nounwind readnone
+  define signext i32 @test(i32 signext %a, i32 signext %b, i32 signext %c) local_unnamed_addr #0 {
+  entry:
+    %cmp = icmp sgt i32 %a, %b
+    %add = add nsw i32 %c, %b
+    %cond = select i1 %cmp, i32 %a, i32 %add
+    ret i32 %cond
+  }
+
+  
+  !llvm.module.flags = !{!0, !1}
+  !llvm.ident = !{!2}
+  
+  !0 = !{i32 1, !"wchar_size", i32 4}
+  !1 = !{i32 7, !"PIC Level", i32 2}
+  !2 = !{!"clang version 8.0.0 (trunk 347251)"}
+
+...
+---
+name:            test
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:       []
+liveins:         
+  - { reg: '$x3', virtual-reg: '' }
+  - { reg: '$x4', virtual-reg: '' }
+  - { reg: '$x5', virtual-reg: '' }
+frameInfo:       
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    0
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+constants:       []
+body:             |
+  bb.0.entry:
+    liveins: $x3, $x5
+
+    renamable $r4 = LI 0
+    renamable $r5 = nsw ADD4 killed renamable $r5, renamable $r5, implicit $x5
+    renamable $cr0 = CMPW renamable $r3, killed renamable $r4, implicit $x4
+    ; CHECK: ADD4
+    ; CHECK-NOT: implicit $x4
+    renamable $r3 = ISEL killed renamable $r3, killed renamable $r5, killed renamable $cr0gt, implicit $cr0, implicit $x3
+    renamable $x3 = EXTSW_32_64 killed renamable $r3
+    BLR8 implicit $lr8, implicit $rm, implicit killed $x3
+
+...
+