[NFC] [PowerPC] Use isPredicable bits in instruction definitions

[qiucf]

Summary

Currently, we are using method isPredicable to check opcode of machine instr on PowerPC. However, on other platforms, it's adapted to use isPredicable bit in instruction themselves. And by default, TargetInstrInfo::isPredicable(const MachineInstr&) checks the bit in instr.

This patch only affects opcodes mentioned in current PPCInstrInfo::isPredicable, to minimalize changes. We can add it to more opcodes in future patches.

[qiucf]

Extend patch with full contexts.

[jsji]

If this is a NFC change, please add [NFC] in summary. Or else, please add test. Thanks.

[steven.zhang]

You need to revisit the place that use the isPredicable bit of the MI, which might cause functionality change. i.e.

bool TargetInstrInfo::isUnpredicatedTerminator(const MachineInstr &MI) const {
  if (!MI.isTerminator()) return false;

  // Conditional branch is a special case.
  if (MI.isBranch() && !MI.isBarrier())
    return true;
  if (!MI.isPredicable())
    return true;
  return !isPredicated(MI);
}

bool TargetInstrInfo::PredicateInstruction(
    MachineInstr &MI, ArrayRef<MachineOperand> Pred) const {
  bool MadeChange = false;

  assert(!MI.isBundle() &&
         "TargetInstrInfo::PredicateInstruction() can't handle bundles");

  const MCInstrDesc &MCID = MI.getDesc();
  if (!MI.isPredicable())
    return false;

  for (unsigned j = 0, i = 0, e = MI.getNumOperands(); i != e; ++i) {
    if (MCID.OpInfo[i].isPredicate()) {
      MachineOperand &MO = MI.getOperand(i);
      if (MO.isReg()) {
        MO.setReg(Pred[j].getReg());
        MadeChange = true;
      } else if (MO.isImm()) {
        MO.setImm(Pred[j].getImm());
        MadeChange = true;
      } else if (MO.isMBB()) {
        MO.setMBB(Pred[j].getMBB());
        MadeChange = true;
      }
      ++j;
    }
  }
  return MadeChange;
}

[qiucf]

In D71921#1798444, @steven.zhang wrote:

You need to revisit the place that use the isPredicable bit of the MI, which might cause functionality change.

In D71921#1797240, @jsji wrote:

If this is a NFC change, please add [NFC] in summary. Or else, please add test. Thanks.

I checked all references to isPredicable in codebase:

• Instr doc generation uses the bit (this is non-functional)
• CodeGen tablegen itself uses it
• Both TargetInstrInfo::isUnpredicatedTerminator and TargetInstrInfo::PredicateInstruction references the method, but PPC overrides them and doesn't use it.
• MachineInstr::findFirstPredOperandIdx references it, but only ARM and AMDGPU invokes it
• Implicit null check and machine sink have referenced it. However, this patch won't touch instrs may load or store, so result of expression would never change
• Other references are for other platforms

[steven.zhang]

LGTM as it makes sense. But please hold on for some days in case someone else have other concern. And I suggest you commit follow up patches to set this bit for other branch instructions.

[jsji]

I checked all references to isPredicable in codebase:

• Instr doc generation uses the bit (this is non-functional)
• CodeGen tablegen itself uses it
• Both TargetInstrInfo::isUnpredicatedTerminator and TargetInstrInfo::PredicateInstruction references the method, but PPC overrides them and doesn't use it.
• MachineInstr::findFirstPredOperandIdx references it, but only ARM and AMDGPU invokes it
• Implicit null check and machine sink have referenced it. However, this patch won't touch instrs may load or store, so result of expression would never change
• Other references are for other platforms

So, I assume you are saying this is *NFC* for PowerPC, right?

[qiucf]

In D71921#1799539, @jsji wrote:

I checked all references to isPredicable in codebase:

• Instr doc generation uses the bit (this is non-functional)
• CodeGen tablegen itself uses it
• Both TargetInstrInfo::isUnpredicatedTerminator and TargetInstrInfo::PredicateInstruction references the method, but PPC overrides them and doesn't use it.
• MachineInstr::findFirstPredOperandIdx references it, but only ARM and AMDGPU invokes it
• Implicit null check and machine sink have referenced it. However, this patch won't touch instrs may load or store, so result of expression would never change
• Other references are for other platforms

So, I assume you are saying this is *NFC* for PowerPC, right?

Yes. I changed title of this revision.

llvm/lib/Target/PowerPC/PPCInstr64Bit.td

 
 
 //===----------------------------------------------------------------------===//
 // Calls.
 //
 
 let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
 let isTerminator = 1, isBarrier = 1, PPC970_Unit = 7 in {
-  let isReturn = 1, Uses = [LR8, RM] in
+  let isReturn = 1, isPredicable = 1, Uses = [LR8, RM] in
     def BLR8 : XLForm_2_ext<19, 16, 20, 0, 0, (outs), (ins), "blr", IIC_BrB,
                             [(retflag)]>, Requires<[In64BitMode]>;
   let isBranch = 1, isIndirectBranch = 1, Uses = [CTR8] in {
+    let isPredicable = 1 in
       def BCTR8 : XLForm_2_ext<19, 528, 20, 0, 0, (outs), (ins), "bctr", IIC_BrB,
                                []>,
           Requires<[In64BitMode]>;
     def BCCCTR8 : XLForm_2_br<19, 528, 0, (outs), (ins pred:$cond),
                               "b${cond:cc}ctr${cond:pm} ${cond:reg}", IIC_BrB,
                               []>,
         Requires<[In64BitMode]>;
 
     def BCCTR8  : XLForm_2_br2<19, 528, 12, 0, (outs), (ins crbitrc:$bi),
                                "bcctr 12, $bi, 0", IIC_BrB, []>,
         Requires<[In64BitMode]>;
def BL8_NOP_TLS : IForm_and_DForm_4_zero<18, 0, 1, 24,
                                   "bl $func\n\tnop", IIC_BrB, []>;
 
     def BLA8_NOP : IForm_and_DForm_4_zero<18, 1, 1, 24,
                              (outs), (ins abscalltarget:$func),
                              "bla $func\n\tnop", IIC_BrB,
                              [(PPCcall_nop (i64 imm:$func))]>;
   }
   let Uses = [CTR8, RM] in {
+    let isPredicable = 1 in
       def BCTRL8 : XLForm_2_ext<19, 528, 20, 0, 1, (outs), (ins),
                                 "bctrl", IIC_BrB, [(PPCbctrl)]>,
                    Requires<[In64BitMode]>;
 
     let isCodeGenOnly = 1 in {
       def BCCCTRL8 : XLForm_2_br<19, 528, 1, (outs), (ins pred:$cond),
                                  "b${cond:cc}ctrl${cond:pm} ${cond:reg}", IIC_BrB,
                                  []>,
           Requires<[In64BitMode]>;
 
       def BCCTRL8  : XLForm_2_br2<19, 528, 12, 1, (outs), (ins crbitrc:$bi),

llvm/lib/Target/PowerPC/PPCInstrInfo.h

bool PredicateInstruction(MachineInstr &MI,
                             ArrayRef<MachineOperand> Pred) const override;
 
   bool SubsumesPredicate(ArrayRef<MachineOperand> Pred1,
                          ArrayRef<MachineOperand> Pred2) const override;
 
   bool DefinesPredicate(MachineInstr &MI,
                         std::vector<MachineOperand> &Pred) const override;
 
-  bool isPredicable(const MachineInstr &MI) const override;
-
   // Comparison optimization.
 
   bool analyzeCompare(const MachineInstr &MI, unsigned &SrcReg,
                       unsigned &SrcReg2, int &Mask, int &Value) const override;
 
   bool optimizeCompareInstr(MachineInstr &CmpInstr, unsigned SrcReg,
                             unsigned SrcReg2, int Mask, int Value,
                             const MachineRegisterInfo *MRI) const override;

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

for (unsigned c = 0; c < array_lengthof(RCs) && !Found; ++c) {
           }
       }
     }
   }
 
   return Found;
 }
 
-bool PPCInstrInfo::isPredicable(const MachineInstr &MI) const {
-  unsigned OpC = MI.getOpcode();
-  switch (OpC) {
-  default:
-    return false;
-  case PPC::B:
-  case PPC::BLR:
-  case PPC::BLR8:
-  case PPC::BCTR:
-  case PPC::BCTR8:
-  case PPC::BCTRL:
-  case PPC::BCTRL8:
-    return true;
-  }
-}
-
 bool PPCInstrInfo::analyzeCompare(const MachineInstr &MI, unsigned &SrcReg,
                                   unsigned &SrcReg2, int &Mask,
                                   int &Value) const {
   unsigned Opc = MI.getOpcode();
 
   switch (Opc) {
   default: return false;
   case PPC::CMPWI:

llvm/lib/Target/PowerPC/PPCInstrInfo.td

 let mayLoad = 1 in {
 def RESTORE_CR : PPCEmitTimePseudo<(outs crrc:$cond), (ins memri:$F),
                      "#RESTORE_CR", []>;
 def RESTORE_CRBIT : PPCEmitTimePseudo<(outs crbitrc:$cond), (ins memri:$F),
                            "#RESTORE_CRBIT", []>;
 }
 
 let isTerminator = 1, isBarrier = 1, PPC970_Unit = 7 in {
-  let isReturn = 1, Uses = [LR, RM] in
+  let isPredicable = 1, isReturn = 1, Uses = [LR, RM] in
     def BLR : XLForm_2_ext<19, 16, 20, 0, 0, (outs), (ins), "blr", IIC_BrB,
                            [(retflag)]>, Requires<[In32BitMode]>;
   let isBranch = 1, isIndirectBranch = 1, Uses = [CTR] in {
+    let isPredicable = 1 in
       def BCTR : XLForm_2_ext<19, 528, 20, 0, 0, (outs), (ins), "bctr", IIC_BrB,
                               []>;
 
     let isCodeGenOnly = 1 in {
       def BCCCTR : XLForm_2_br<19, 528, 0, (outs), (ins pred:$cond),
                                "b${cond:cc}ctr${cond:pm} ${cond:reg}", IIC_BrB,
                                []>;
 
       def BCCTR :  XLForm_2_br2<19, 528, 12, 0, (outs), (ins crbitrc:$bi),
                                 "bcctr 12, $bi, 0", IIC_BrB, []>;
let Defs = [LR] in
   def MovePCtoLR : PPCEmitTimePseudo<(outs), (ins), "#MovePCtoLR", []>,
                    PPC970_Unit_BRU;
 let Defs = [LR] in
   def MoveGOTtoLR : PPCEmitTimePseudo<(outs), (ins), "#MoveGOTtoLR", []>,
                     PPC970_Unit_BRU;
 
 let isBranch = 1, isTerminator = 1, hasCtrlDep = 1, PPC970_Unit = 7 in {
   let isBarrier = 1 in {
-  def B   : IForm<18, 0, 0, (outs), (ins directbrtarget:$dst),
+    let isPredicable = 1 in
+      def B : IForm<18, 0, 0, (outs), (ins directbrtarget:$dst),
                     "b $dst", IIC_BrB,
                     [(br bb:$dst)]>;
   def BA  : IForm<18, 1, 0, (outs), (ins absdirectbrtarget:$dst),
                   "ba $dst", IIC_BrB, []>;
   }
 
   // BCC represents an arbitrary conditional branch on a predicate.
   // FIXME: should be able to write a pattern for PPCcondbranch, but can't use
   // a two-value operand where a dag node expects two operands. :(
   let isCodeGenOnly = 1 in {
let isCodeGenOnly = 1 in {
                          (ins crbitrc:$bi, condbrtarget:$dst),
                          "bcl 4, $bi, $dst">;
       def BL_NOP  : IForm_and_DForm_4_zero<18, 0, 1, 24,
                                            (outs), (ins calltarget:$func),
                                            "bl $func\n\tnop", IIC_BrB, []>;
     }
   }
   let Uses = [CTR, RM] in {
+    let isPredicable = 1 in
       def BCTRL : XLForm_2_ext<19, 528, 20, 0, 1, (outs), (ins),
                               "bctrl", IIC_BrB, [(PPCbctrl)]>,
                   Requires<[In32BitMode]>;
 
     let isCodeGenOnly = 1 in {
       def BCCCTRL : XLForm_2_br<19, 528, 1, (outs), (ins pred:$cond),
                                 "b${cond:cc}ctrl${cond:pm} ${cond:reg}", IIC_BrB,
                                 []>;
 
       def BCCTRL  : XLForm_2_br2<19, 528, 12, 1, (outs), (ins crbitrc:$bi),
                                  "bcctrl 12, $bi, 0", IIC_BrB, []>;