AArch64: More branch relaxation cleanup

[rovka]

Looks neat.

[arsenm]

Merged with D22872 in r277430

lib/Target/AArch64/AArch64BranchRelaxation.cpp

class AArch64BranchRelaxation : public MachineFunctionPass {
   const AArch64InstrInfo *TII;
 
   bool relaxBranchInstructions();
   void scanFunction();
   MachineBasicBlock *splitBlockBeforeInstr(MachineInstr &MI);
   void adjustBlockOffsets(MachineBasicBlock &MBB);
   bool isBlockInRange(const MachineInstr &MI, const MachineBasicBlock &BB) const;
   void invertConditionalBranch(MachineInstr &MI) const;
+  unsigned insertInvertedConditionalBranch(MachineBasicBlock &MBB,
+                                           const MachineInstr &OldBr,
+                                           MachineBasicBlock &NewDestBB) const;
+  unsigned insertUnconditionalBranch(MachineBasicBlock &MBB,
+                                     MachineBasicBlock &NewDestBB,
+                                     const DebugLoc &DL) const;
+
   bool fixupConditionalBranch(MachineInstr &MI);
   void computeBlockSize(const MachineBasicBlock &MBB);
   unsigned getInstrOffset(const MachineInstr &MI) const;
   void dumpBBs();
   void verify();
 
 public:
   static char ID;

 void AArch64BranchRelaxation::dumpBBs() {
   for (auto &MBB : *MF) {
     const BasicBlockInfo &BBI = BlockInfo[MBB.getNumber()];
     dbgs() << format("BB#%u\toffset=%08x\t", MBB.getNumber(), BBI.Offset)
            << format("size=%#x\n", BBI.Size);
   }
 }
 
-/// BBHasFallthrough - Return true if the specified basic block can fallthrough
+// FIXME: This is a less precise version of MachineBasicBlock::canFallThrough?
+
+/// \returns true if the specified basic block can fallthrough
 /// into the block immediately after it.
-static bool BBHasFallthrough(MachineBasicBlock *MBB) {
+static bool hasFallthrough(const MachineBasicBlock &MBB) {
   // Get the next machine basic block in the function.
-  MachineFunction::iterator MBBI(MBB);
+  MachineFunction::const_iterator MBBI(MBB);
+
   // Can't fall off end of function.
   auto NextBB = std::next(MBBI);
-  if (NextBB == MBB->getParent()->end())
+  if (NextBB == MBB.getParent()->end())
     return false;
 
-  for (MachineBasicBlock *S : MBB->successors())
-    if (S == &*NextBB)
-      return true;
-
-  return false;
+  return MBB.isSuccessor(&*NextBB);
 }
 
 /// scanFunction - Do the initial scan of the function, building up
 /// information about each block.
 void AArch64BranchRelaxation::scanFunction() {
   BlockInfo.clear();
   BlockInfo.resize(MF->getNumBlockIDs());
 

 /// Invert the branch condition of \p MI and change the destination to \p NewB
 void AArch64BranchRelaxation::invertConditionalBranch(MachineInstr &MI) const {
   MI.setDesc(TII->get(getOppositeConditionOpcode(MI.getOpcode())));
 
   if (MI.getOpcode() == AArch64::Bcc)
     invertBccCondition(MI);
 }
 
+/// Insert a conditional branch at the end of \p MBB to \p NewDestBB, using the
+/// inverse condition of branch \p OldBr.
+/// \returns The number of bytes added to the block.
+unsigned AArch64BranchRelaxation::insertInvertedConditionalBranch(
+  MachineBasicBlock &MBB,
+  const MachineInstr &OldBr,
+  MachineBasicBlock &NewDestBB) const {
+  unsigned OppositeCondOpc = getOppositeConditionOpcode(OldBr.getOpcode());
+
+  MachineInstrBuilder MIB =
+    BuildMI(&MBB, OldBr.getDebugLoc(), TII->get(OppositeCondOpc))
+    .addOperand(OldBr.getOperand(0));
+
+  unsigned Opc = OldBr.getOpcode();
+
+  if (Opc == AArch64::TBZW || Opc == AArch64::TBNZW ||
+      Opc == AArch64::TBZX || Opc == AArch64::TBNZX)
+    MIB.addOperand(OldBr.getOperand(1));
+
+  if (OldBr.getOpcode() == AArch64::Bcc)
+    invertBccCondition(*MIB);
+
+  MIB.addMBB(&NewDestBB);
+
+  return TII->GetInstSizeInBytes(*MIB);
+}
+
+/// Insert an unconditional branch at the end of \p MBB to \p DestBB.
+/// \returns the number of bytes emitted.
+unsigned AArch64BranchRelaxation::insertUnconditionalBranch(
+  MachineBasicBlock &MBB,
+  MachineBasicBlock &DestBB,
+  const DebugLoc &DL) const {
+  MachineInstr *MI = BuildMI(&MBB, DL, TII->get(AArch64::B))
+    .addMBB(&DestBB);
+
+  return TII->GetInstSizeInBytes(*MI);
+}
+
 static void changeBranchDestBlock(MachineInstr &MI,
                                   MachineBasicBlock &NewDestBB) {
   unsigned Opc = MI.getOpcode();
   unsigned OpNum = (Opc == AArch64::TBZW ||
                     Opc == AArch64::TBNZW ||
                     Opc == AArch64::TBZX ||
                     Opc == AArch64::TBNZX) ? 2 : 1;
   MI.getOperand(OpNum).setMBB(&NewDestBB);
bool AArch64BranchRelaxation::fixupConditionalBranch(MachineInstr &MI) {
   // b   L1
   // L2:
 
   // If the branch is at the end of its MBB and that has a fall-through block,
   // direct the updated conditional branch to the fall-through block. Otherwise,
   // split the MBB before the next instruction.
   MachineBasicBlock *MBB = MI.getParent();
   MachineInstr *BMI = &MBB->back();
-  bool NeedSplit = (BMI != &MI) || !BBHasFallthrough(MBB);
+  bool NeedSplit = (BMI != &MI) || !hasFallthrough(*MBB);
 
   if (BMI != &MI) {
     if (std::next(MachineBasicBlock::iterator(MI)) ==
             std::prev(MBB->getLastNonDebugInstr()) &&
         BMI->isUnconditionalBranch()) {
       // Last MI in the BB is an unconditional branch. We can simply invert the
       // condition and swap destinations:
       // beq L1
if (NeedSplit) {
     MBB->back().eraseFromParent();
     // BlockInfo[SplitBB].Offset is wrong temporarily, fixed below
 
     // Update the successor lists according to the transformation to follow.
     // Do it here since if there's no split, no update is needed.
     MBB->replaceSuccessor(FBB, NewBB);
     NewBB->addSuccessor(FBB);
   }
+
   MachineBasicBlock &NextBB = *std::next(MachineFunction::iterator(MBB));
 
   DEBUG(dbgs() << "  Insert B to BB#" << DestBB->getNumber()
                << ", invert condition and change dest. to BB#"
                << NextBB.getNumber() << '\n');
 
-  unsigned OppositeCondOpc = getOppositeConditionOpcode(MI.getOpcode());
   // Insert a new conditional branch and a new unconditional branch.
-  MachineInstrBuilder MIB = BuildMI(MBB, DebugLoc(), TII->get(OppositeCondOpc))
-    .addOperand(MI.getOperand(0));
+  BlockInfo[MBB->getNumber()].Size
+    += insertInvertedConditionalBranch(*MBB, MI, NextBB);
 
-  if (MI.getOpcode() == AArch64::TBZW || MI.getOpcode() == AArch64::TBNZW ||
-      MI.getOpcode() == AArch64::TBZX || MI.getOpcode() == AArch64::TBNZX)
-    MIB.addOperand(MI.getOperand(1));
-  if (MI.getOpcode() == AArch64::Bcc)
-    invertBccCondition(*MIB);
-  MIB.addMBB(&NextBB);
-  BlockInfo[MBB->getNumber()].Size += TII->GetInstSizeInBytes(MBB->back());
-  BuildMI(MBB, DebugLoc(), TII->get(AArch64::B)).addMBB(DestBB);
-  BlockInfo[MBB->getNumber()].Size += TII->GetInstSizeInBytes(MBB->back());
+  BlockInfo[MBB->getNumber()].Size +=
+    insertUnconditionalBranch(*MBB, *DestBB, MI.getDebugLoc());
 
   // Remove the old conditional branch.  It may or may not still be in MBB.
   BlockInfo[MBB->getNumber()].Size -= TII->GetInstSizeInBytes(MI);
   MI.eraseFromParent();
 
   // Finally, keep the block offsets up to date.
   adjustBlockOffsets(*MBB);
   return true;