[ARM] Simplification to ARMBlockPlacement Pass.

It simplifies the logic by moving the predecessor  (preHeader or it's predecessor) above the target (or loopExit),
instead of moving the target to after the predecessor.

Since the loopExit is no longer being moved, directions of any branches within/to it are unaffected.

While the predecessor is being moved, the backwards movement simplifies some considerations,
and the only consideration now required is that a forward WLS to the predecessor should not become backwards.

Reviewed By: dmgreen

Differential Revision: https://reviews.llvm.org/D100094

Author: malJaj

llvm/lib/Target/ARM/ARMBlockPlacement.cpp

@@ -36,7 +36,7 @@ class ARMBlockPlacement : public MachineFunctionPass {
   ARMBlockPlacement() : MachineFunctionPass(ID) {}
 
   bool runOnMachineFunction(MachineFunction &MF) override;
-  void moveBasicBlock(MachineBasicBlock *BB, MachineBasicBlock *After);
+  void moveBasicBlock(MachineBasicBlock *BB, MachineBasicBlock *Before);
   bool blockIsBefore(MachineBasicBlock *BB, MachineBasicBlock *Other);
   bool fixBackwardsWLS(MachineLoop *ML);
   bool processPostOrderLoops(MachineLoop *ML);
@@ -82,19 +82,20 @@ static MachineInstr *findWLS(MachineLoop *ML) {
 }
 
 /// Checks if loop has a backwards branching WLS, and if possible, fixes it.
-/// This requires checking the preheader (or it's predecessor) for a WLS and if
-/// its target is before it.
-/// If moving the target block wouldn't produce another backwards WLS or a new
-/// forwards LE branch, then move the target block after the preheader (or it's
-/// predecessor).
+/// This requires checking the predecessor (ie. preheader or it's predecessor)
+/// for a WLS and if its loopExit/target is before it.
+/// If moving the predecessor won't convert a WLS (to the predecessor) from
+/// a forward to a backward branching WLS, then move the predecessor block
+/// to before the loopExit/target.
 bool ARMBlockPlacement::fixBackwardsWLS(MachineLoop *ML) {
   MachineInstr *WlsInstr = findWLS(ML);
   if (!WlsInstr)
     return false;
 
   MachineBasicBlock *Predecessor = WlsInstr->getParent();
   MachineBasicBlock *LoopExit = WlsInstr->getOperand(2).getMBB();
-  // We don't want to move the function's entry block.
+
+  // We don't want to move Preheader to before the function's entry block.
   if (!LoopExit->getPrevNode())
     return false;
   if (blockIsBefore(Predecessor, LoopExit))
@@ -103,77 +104,38 @@ bool ARMBlockPlacement::fixBackwardsWLS(MachineLoop *ML) {
                     << Predecessor->getFullName() << " to "
                     << LoopExit->getFullName() << "\n");
 
-  // Make sure that moving the target block doesn't cause any of its WLSs
-  // that were previously not backwards to become backwards
-  bool CanMove = true;
-  MachineInstr *WlsInLoopExit = findWLSInBlock(LoopExit);
-  if (WlsInLoopExit) {
-    // An example loop structure where the LoopExit can't be moved, since
-    // bb1's WLS will become backwards once it's moved after bb3
-    // bb1:          - LoopExit
-    //      WLS bb2
-    // bb2:          - LoopExit2
-    //      ...
-    // bb3:          - Predecessor
-    //      WLS bb1
-    // bb4:          - Header
-    MachineBasicBlock *LoopExit2 = WlsInLoopExit->getOperand(2).getMBB();
-    // If the WLS from LoopExit to LoopExit2 is already backwards then
-    // moving LoopExit won't affect it, so it can be moved. If LoopExit2 is
-    // after the Predecessor then moving will keep it as a forward branch, so it
-    // can be moved. If LoopExit2 is between the Predecessor and LoopExit then
-    // moving LoopExit will make it a backwards branch, so it can't be moved
-    // since we'd fix one and introduce one backwards branch.
-    // TODO: Analyse the blocks to make a decision if it would be worth
-    // moving LoopExit even if LoopExit2 is between the Predecessor and
-    // LoopExit.
-    if (!blockIsBefore(LoopExit2, LoopExit) &&
-        (LoopExit2 == Predecessor || blockIsBefore(LoopExit2, Predecessor))) {
-      LLVM_DEBUG(dbgs() << DEBUG_PREFIX
-                        << "Can't move the target block as it would "
-                           "introduce a new backwards WLS branch\n");
-      CanMove = false;
-    }
-  }
-
-  if (CanMove) {
-    // Make sure no LEs become forwards.
-    // An example loop structure where the LoopExit can't be moved, since
-    // bb2's LE will become forwards once bb1 is moved after bb3.
-    // bb1:           - LoopExit
-    // bb2:
-    //      LE  bb1  - Terminator
-    // bb3:          - Predecessor
-    //      WLS bb1
-    // bb4:          - Header
-    for (auto It = LoopExit->getIterator(); It != Predecessor->getIterator();
-         It++) {
-      MachineBasicBlock *MBB = &*It;
-      for (auto &Terminator : MBB->terminators()) {
-        if (Terminator.getOpcode() != ARM::t2LoopEnd &&
-            Terminator.getOpcode() != ARM::t2LoopEndDec)
-          continue;
-        MachineBasicBlock *LETarget = Terminator.getOperand(2).getMBB();
-        // The LE will become forwards branching if it branches to LoopExit
-        // which isn't allowed by the architecture, so we should avoid
-        // introducing these.
-        // TODO: Analyse the blocks to make a decision if it would be worth
-        // moving LoopExit even if we'd introduce a forwards LE
-        if (LETarget == LoopExit) {
-          LLVM_DEBUG(dbgs() << DEBUG_PREFIX
-                            << "Can't move the target block as it would "
-                               "introduce a new forwards LE branch\n");
-          CanMove = false;
-          break;
-        }
+  // Make sure no forward branching WLSs to the Predecessor become backwards
+  // branching. An example loop structure where the Predecessor can't be moved,
+  // since bb2's WLS will become forwards once bb3 is moved before/above bb1.
+  //
+  // bb1:           - LoopExit
+  // bb2:
+  //      WLS  bb3
+  // bb3:          - Predecessor
+  //      WLS bb1
+  // bb4:          - Header
+  for (auto It = ++LoopExit->getIterator(); It != Predecessor->getIterator();
+       ++It) {
+    MachineBasicBlock *MBB = &*It;
+    for (auto &Terminator : MBB->terminators()) {
+      if (Terminator.getOpcode() != ARM::t2WhileLoopStartLR)
+        continue;
+      MachineBasicBlock *WLSTarget = Terminator.getOperand(2).getMBB();
+      // TODO: Analyse the blocks to make a decision if it would be worth
+      // moving Preheader even if we'd introduce a backwards WLS
+      if (WLSTarget == Predecessor) {
+        LLVM_DEBUG(
+            dbgs() << DEBUG_PREFIX
+                   << "Can't move Predecessor"
+                      "block as it would convert a WLS from forward to a "
+                      "backwards branching WLS\n");
+        return false;
       }
     }
   }
 
-  if (CanMove)
-    moveBasicBlock(LoopExit, Predecessor);
-
-  return CanMove;
+  moveBasicBlock(Predecessor, LoopExit);
+  return true;
 }
 
 /// Updates ordering (of WLS BB and their loopExits) in inner loops first
@@ -212,18 +174,20 @@ bool ARMBlockPlacement::blockIsBefore(MachineBasicBlock *BB,
   return BBUtils->getOffsetOf(Other) > BBUtils->getOffsetOf(BB);
 }
 
-/// Moves a given MBB to be positioned after another MBB while maintaining
-/// existing control flow
+// Moves a BasicBlock before another, without changing the control flow
 void ARMBlockPlacement::moveBasicBlock(MachineBasicBlock *BB,
-                                       MachineBasicBlock *After) {
-  LLVM_DEBUG(dbgs() << DEBUG_PREFIX << "Moving " << BB->getName() << " after "
-                    << After->getName() << "\n");
+                                       MachineBasicBlock *Before) {
+  LLVM_DEBUG(dbgs() << DEBUG_PREFIX << "Moving " << BB->getName() << " before "
+                    << Before->getName() << "\n");
   MachineBasicBlock *BBPrevious = BB->getPrevNode();
   assert(BBPrevious && "Cannot move the function entry basic block");
-  MachineBasicBlock *AfterNext = After->getNextNode();
   MachineBasicBlock *BBNext = BB->getNextNode();
 
-  BB->moveAfter(After);
+  MachineBasicBlock *BeforePrev = Before->getPrevNode();
+  assert(BeforePrev &&
+         "Cannot move the given block to before the function entry block");
+  MachineFunction *F = BB->getParent();
+  BB->moveBefore(Before);
 
   // Since only the blocks are to be moved around (but the control flow must
   // not change), if there were any fall-throughs (to/from adjacent blocks),
@@ -251,12 +215,14 @@ void ARMBlockPlacement::moveBasicBlock(MachineBasicBlock *BB,
   // Fix fall-through to the moved BB from the one that used to be before it.
   if (BBPrevious->isSuccessor(BB))
     FixFallthrough(BBPrevious, BB);
-  // Fix fall through from the destination BB to the one that used to follow.
-  if (AfterNext && After->isSuccessor(AfterNext))
-    FixFallthrough(After, AfterNext);
+  // Fix fall through from the destination BB to the one that used to before it.
+  if (BeforePrev->isSuccessor(Before))
+    FixFallthrough(BeforePrev, Before);
   // Fix fall through from the moved BB to the one that used to follow.
   if (BBNext && BB->isSuccessor(BBNext))
     FixFallthrough(BB, BBNext);
 
-  BBUtils->adjustBBOffsetsAfter(After);
+  F->RenumberBlocks();
+  BBUtils->computeAllBlockSizes();
+  BBUtils->adjustBBOffsetsAfter(&F->front());
 }