[PowerPC] Fix the implicit operands in PPCInstrInfo::PredicateInstruction()

[ZhangKang]

Summary

In the file PPCInstrInfo.cpp:

1449 bool PPCInstrInfo::PredicateInstruction(MachineInstr &MI,
1450                                         ArrayRef<MachineOperand> Pred) const {
1451   unsigned OpC = MI.getOpcode();
1452   if (OpC == PPC::BLR || OpC == PPC::BLR8) {
1453     if (Pred[1].getReg() == PPC::CTR8 || Pred[1].getReg() == PPC::CTR) {
1454       bool isPPC64 = Subtarget.isPPC64();
1455       MI.setDesc(get(Pred[0].getImm() ? (isPPC64 ? PPC::BDNZLR8 : PPC::BDNZLR)
1456                                       : (isPPC64 ? PPC::BDZLR8 : PPC::BDZLR)));
1457     } else if (Pred[0].getImm() == PPC::PRED_BIT_SET) {
              ...
          }

Above code only modify the opcode, but doesn't consider the implicit register for the new opcode.
For example, if MI is BLR implicit $lr, implicit $rm, in the line 1455. We use MI.setDesc() to convert the BLR to BDZLR.
We will get:

BDZLR implicit $lr, implicit $rm

But we don't realize BLR and BDZLR have different implicit register. In fact, the right instruction should be below:

BDZLR implicit-def $ctr, implicit $ctr, implicit $lr, implicit $rm

[efriedma]

I don't think it makes sense to try to fix implicit operands generically. If we're changing the opcode in a way that affects the operands, the code making that change should be aware of what the change needs to be. That code can add or remove operands as necessary.

"Automatically" fixing the operands later might produce the same result, but it's harder to understand what's happening.

[jsji]

Agree with Eli in general. Although this is the code making that change , current condition code `if (MI.getOpcode() != OpC) ` to fix the reg is too general .
We should only fix it when there is real needs.

So maybe set a flag where the implicit operands needs update, and only call the routine when the flag is on.

[ZhangKang]

Add IsNeedFixImplicitOps to call the fixupImplicitDefUseOperands when needed.

[jsji]

LGTM. Please wait a few days in case @efriedma has other comments.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

1679

IsNeedFixImplicitOps -> needImplicitOpsFix

[efriedma]

I was thinking more along the lines of just writing something like MIB.addReg(Pred[1].getReg(), RegState::ImplicitDefine).addReg(Pred[1].getReg(), RegState::Implicit);.

[ZhangKang]

Don't call fixupImplicitDefUseOperands ().

[efriedma]

LGTM

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

 bool PPCInstrInfo::PredicateInstruction(MachineInstr &MI,
                                         ArrayRef<MachineOperand> Pred) const {
   unsigned OpC = MI.getOpcode();
   if (OpC == PPC::BLR || OpC == PPC::BLR8) {
     if (Pred[1].getReg() == PPC::CTR8 || Pred[1].getReg() == PPC::CTR) {
       bool isPPC64 = Subtarget.isPPC64();
       MI.setDesc(get(Pred[0].getImm() ? (isPPC64 ? PPC::BDNZLR8 : PPC::BDNZLR)
                                       : (isPPC64 ? PPC::BDZLR8 : PPC::BDZLR)));
+      // Need add Def and Use for CTR implicit operand.
+      MachineInstrBuilder(*MI.getParent()->getParent(), MI)
+          .addReg(Pred[1].getReg(), RegState::Implicit)
+          .addReg(Pred[1].getReg(), RegState::ImplicitDefine);
     } else if (Pred[0].getImm() == PPC::PRED_BIT_SET) {
       MI.setDesc(get(PPC::BCLR));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI).add(Pred[1]);
     } else if (Pred[0].getImm() == PPC::PRED_BIT_UNSET) {
       MI.setDesc(get(PPC::BCLRn));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI).add(Pred[1]);
     } else {
       MI.setDesc(get(PPC::BCCLR));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI)
           .addImm(Pred[0].getImm())
           .add(Pred[1]);
     }
 
     return true;
   } else if (OpC == PPC::B) {
     if (Pred[1].getReg() == PPC::CTR8 || Pred[1].getReg() == PPC::CTR) {
       bool isPPC64 = Subtarget.isPPC64();
       MI.setDesc(get(Pred[0].getImm() ? (isPPC64 ? PPC::BDNZ8 : PPC::BDNZ)
                                       : (isPPC64 ? PPC::BDZ8 : PPC::BDZ)));
+      // Need add Def and Use for CTR implicit operand.
+      MachineInstrBuilder(*MI.getParent()->getParent(), MI)
+          .addReg(Pred[1].getReg(), RegState::Implicit)
+          .addReg(Pred[1].getReg(), RegState::ImplicitDefine);
     } else if (Pred[0].getImm() == PPC::PRED_BIT_SET) {
       MachineBasicBlock *MBB = MI.getOperand(0).getMBB();
       MI.RemoveOperand(0);
 
       MI.setDesc(get(PPC::BC));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI)
           .add(Pred[1])
           .addMBB(MBB);
if (OpC == PPC::BLR || OpC == PPC::BLR8) {
 
     bool setLR = OpC == PPC::BCTRL || OpC == PPC::BCTRL8;
     bool isPPC64 = Subtarget.isPPC64();
 
     if (Pred[0].getImm() == PPC::PRED_BIT_SET) {
       MI.setDesc(get(isPPC64 ? (setLR ? PPC::BCCTRL8 : PPC::BCCTR8)
                              : (setLR ? PPC::BCCTRL : PPC::BCCTR)));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI).add(Pred[1]);
-      return true;
     } else if (Pred[0].getImm() == PPC::PRED_BIT_UNSET) {
       MI.setDesc(get(isPPC64 ? (setLR ? PPC::BCCTRL8n : PPC::BCCTR8n)
                              : (setLR ? PPC::BCCTRLn : PPC::BCCTRn)));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI).add(Pred[1]);
-      return true;
-    }
-
+    } else {
       MI.setDesc(get(isPPC64 ? (setLR ? PPC::BCCCTRL8 : PPC::BCCCTR8)
                              : (setLR ? PPC::BCCCTRL : PPC::BCCCTR)));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI)
           .addImm(Pred[0].getImm())
           .add(Pred[1]);
+    }
+
+    // Need add Def and Use for LR implicit operand.
+    if (setLR)
+      MachineInstrBuilder(*MI.getParent()->getParent(), MI)
+          .addReg(isPPC64 ? PPC::LR8 : PPC::LR, RegState::Implicit)
+          .addReg(isPPC64 ? PPC::LR8 : PPC::LR, RegState::ImplicitDefine);
+
     return true;
   }
 
   return false;
 }
 
 bool PPCInstrInfo::SubsumesPredicate(ArrayRef<MachineOperand> Pred1,
                                      ArrayRef<MachineOperand> Pred2) const {

llvm/test/CodeGen/PowerPC/ifcvt.mir

bb.4:
   
     B %bb.3
   
   bb.5:
     BLR implicit $lr, implicit $rm
 
 
   ; CHECK-LABEL: name: testBDZLR
-  ; CHECK: BDZLR implicit $lr, implicit $rm
+  ; CHECK: BDZLR implicit $lr, implicit $rm, implicit $ctr, implicit-def $ctr
 ...