[MBB] Let MachineBasicBlock::updateTerminator() return if it updates the terminator or not. NFC.

[haicheng]

Summary

Change all the return statements to return true/false.

[mcrosier]

AFAICT, the return value is never used. What is the purpose of the change?

[haicheng]

This comment was removed by haicheng.

[haicheng]

In D19955#422233, @mcrosier wrote:

AFAICT, the return value is never used. What is the purpose of the change?

This is part of my effort of enabling tail merging in mbp. I am going to use the return value to decide if I need to call the tail merging.

[qcolombet]

Hi,

If you start adding returns in a bunch of places, you should get rid of the useless else.

Cheers,
-Quentin

[haicheng]

Thank you, Quentin. I removed several elses.

[mcrosier]

I committed rL270773 to reduce the indentation of this function, per the coding guidelines. You need to either rebase this patch on top of that commit or I might even suggest just integrating this change into the patch that enables tail merging in MBP.

[haicheng]

Now D20276 always call tail merging, this patch is not needed. If D20276 needs to check whether the fall through is changed in the future, I will just integrate this change into D20276.

include/llvm/CodeGen/MachineBasicBlock.h

public:
   /// the block, it does not modify the CFG or adjust potential fall-throughs at
   /// the end of the block.
   void moveBefore(MachineBasicBlock *NewAfter);
   void moveAfter(MachineBasicBlock *NewBefore);
 
   /// Update the terminator instructions in block to account for changes to the
   /// layout. If the block previously used a fallthrough, it may now need a
   /// branch, and if it previously used branching it may now be able to use a
-  /// fallthrough.
-  void updateTerminator();
+  /// fallthrough. Return true if the terminator is updated, otherwise false.
+  bool updateTerminator();
 
   // Machine-CFG mutators
 
   /// Add Succ as a successor of this MachineBasicBlock.  The Predecessors list
   /// of Succ is automatically updated. PROB parameter is stored in
   /// Probabilities list. The default probability is set as unknown. Mixing
   /// known and unknown probabilities in successor list is not allowed. When all
   /// successors have unknown probabilities, 1 / N is returned as the

lib/CodeGen/MachineBasicBlock.cpp

 void MachineBasicBlock::moveBefore(MachineBasicBlock *NewAfter) {
   getParent()->splice(NewAfter->getIterator(), getIterator());
 }
 
 void MachineBasicBlock::moveAfter(MachineBasicBlock *NewBefore) {
   getParent()->splice(++NewBefore->getIterator(), getIterator());
 }
 
-void MachineBasicBlock::updateTerminator() {
+bool MachineBasicBlock::updateTerminator() {
   const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();
   // A block with no successors has no concerns with fall-through edges.
-  if (this->succ_empty()) return;
+  if (this->succ_empty()) return false;
 
   MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
   SmallVector<MachineOperand, 4> Cond;
   DebugLoc DL;  // FIXME: this is nowhere
   bool B = TII->AnalyzeBranch(*this, TBB, FBB, Cond);
   (void) B;
   assert(!B && "UpdateTerminators requires analyzable predecessors!");
   if (Cond.empty()) {
     if (TBB) {
       // The block has an unconditional branch. If its successor is now
       // its layout successor, delete the branch.
-      if (isLayoutSuccessor(TBB))
+      if (isLayoutSuccessor(TBB)) {
         TII->RemoveBranch(*this);
-    } else {
-      // The block has an unconditional fallthrough. If its successor is not
-      // its layout successor, insert a branch. First we have to locate the
-      // only non-landing-pad successor, as that is the fallthrough block.
-      for (succ_iterator SI = succ_begin(), SE = succ_end(); SI != SE; ++SI) {
-        if ((*SI)->isEHPad())
-          continue;
-        assert(!TBB && "Found more than one non-landing-pad successor!");
-        TBB = *SI;
-      }
+        return true;
+      }
+      return false;
+    }
+
 
-      // If there is no non-landing-pad successor, the block has no
-      // fall-through edges to be concerned with.
-      if (!TBB)
-        return;
+      return false;
 
-      // Finally update the unconditional successor to be reached via a branch
-      // if it would not be reached by fallthrough.
-      if (!isLayoutSuccessor(TBB))
-        TII->InsertBranch(*this, TBB, nullptr, Cond, DL);
+    if (!isLayoutSuccessor(TBB)) {
+      return true;
     }
-  } else {
-    if (FBB) {
-      // The block has a non-fallthrough conditional branch. If one of its
-      // successors is its layout successor, rewrite it to a fallthrough
-      // conditional branch.
-      if (isLayoutSuccessor(TBB)) {
-        if (TII->ReverseBranchCondition(Cond))
-          return;
-        TII->RemoveBranch(*this);
-        TII->InsertBranch(*this, FBB, nullptr, Cond, DL);
-      } else if (isLayoutSuccessor(FBB)) {
-        TII->RemoveBranch(*this);
-        TII->InsertBranch(*this, TBB, nullptr, Cond, DL);
-      }
-    } else {
-      // Walk through the successors and find the successor which is not
-      // a landing pad and is not the conditional branch destination (in TBB)
-      // as the fallthrough successor.
-      MachineBasicBlock *FallthroughBB = nullptr;
-      for (succ_iterator SI = succ_begin(), SE = succ_end(); SI != SE; ++SI) {
-        if ((*SI)->isEHPad() || *SI == TBB)
-          continue;
-        assert(!FallthroughBB && "Found more than one fallthrough successor.");
-        FallthroughBB = *SI;
-      }
-      if (!FallthroughBB && canFallThrough()) {
-        // We fallthrough to the same basic block as the conditional jump
-        // targets. Remove the conditional jump, leaving unconditional
-        // fallthrough.
-        // FIXME: This does not seem like a reasonable pattern to support, but
-        // it has been seen in the wild coming out of degenerate ARM test cases.
-        TII->RemoveBranch(*this);
+    return false;
+  } // end of if (Cond.empty())
+
+        return false;
+      return true;
+      return true;
+    return false;
+  }
+
 
-        // Finally update the unconditional successor to be reached via a branch
-        // if it would not be reached by fallthrough.
-        if (!isLayoutSuccessor(TBB))
-          TII->InsertBranch(*this, TBB, nullptr, Cond, DL);
-        return;
-      }
+    return true;
 
-      // The block has a fallthrough conditional branch.
-      if (isLayoutSuccessor(TBB)) {
-        if (TII->ReverseBranchCondition(Cond)) {
-          // We can't reverse the condition, add an unconditional branch.
-          Cond.clear();
-          TII->InsertBranch(*this, FallthroughBB, nullptr, Cond, DL);
-          return;
-        }
-        TII->RemoveBranch(*this);
-        TII->InsertBranch(*this, FallthroughBB, nullptr, Cond, DL);
-      } else if (!isLayoutSuccessor(FallthroughBB)) {
-        TII->RemoveBranch(*this);
-        TII->InsertBranch(*this, TBB, FallthroughBB, Cond, DL);
-      }
-    }
-  }
+      return true;
+    return true;
+    return true;
+  return false;
 }
 
 void MachineBasicBlock::validateSuccProbs() const {
 #ifndef NDEBUG
   int64_t Sum = 0;
   for (auto Prob : Probs)
     Sum += Prob.getNumerator();
   // Due to precision issue, we assume that the sum of probabilities is one if