[PATCH] D38918: Update successor after branch relaxation in ARM

[Sameer AbuAsal via Phabricator via llvm-commits]

sabuasal updated this revision to Diff 119363.
sabuasal added a comment.

You are right. I updated the change, if we did indeed split the block the patch now deletes the DestBB from successor list of the new block.  
I only cared about not having the Dest BB as a successor because it might cause scheduling issues and\or labels not being emitted, having extra successor wont.


Repository:
  rL LLVM

https://reviews.llvm.org/D38918

Files:
  lib/Target/ARM/ARMConstantIslandPass.cpp


Index: lib/Target/ARM/ARMConstantIslandPass.cpp
===================================================================
--- lib/Target/ARM/ARMConstantIslandPass.cpp
+++ lib/Target/ARM/ARMConstantIslandPass.cpp
@@ -1659,6 +1659,9 @@
   MachineInstr *BMI = &MBB->back();
   bool NeedSplit = (BMI != MI) || !BBHasFallthrough(MBB);
 
+  // Pointer to the block split from MBB if needed.
+  MachineBasicBlock *SplitBlock = nullptr;
+
   ++NumCBrFixed;
   if (BMI != MI) {
     if (std::next(MachineBasicBlock::iterator(MI)) == std::prev(MBB->end()) &&
@@ -1683,7 +1686,7 @@
   }
 
   if (NeedSplit) {
-    splitBlockBeforeInstr(MI);
+    SplitBlock = splitBlockBeforeInstr(MI);
     // No need for the branch to the next block. We're adding an unconditional
     // branch to the destination.
     int delta = TII->getInstSizeInBytes(MBB->back());
@@ -1713,9 +1716,21 @@
   unsigned MaxDisp = getUnconditionalBrDisp(Br.UncondBr);
   ImmBranches.push_back(ImmBranch(&MBB->back(), MaxDisp, false, Br.UncondBr));
 
+  if (NeedSplit) {
+    // When we split the MBB we transferred all its successors to the newly
+    // created block so the DestBB is no longer marked as a successor. Add
+    // it back here.
+    MBB->addSuccessor(DestBB);
+  }
+
   // Remove the old conditional branch.  It may or may not still be in MBB.
   BBInfo[MI->getParent()->getNumber()].Size -= TII->getInstSizeInBytes(*MI);
   MI->eraseFromParent();
+  // If we split the Block MI belongs to the new one. After deleting MI
+  // DestBB is no longer a successor.
+  if (SplitBlock)
+    SplitBlock->removeSuccessor(DestBB);
+
   adjustBBOffsetsAfter(MBB);
   return true;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38918.119363.patch
Type: text/x-patch
Size: 1642 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171017/469ce3bc/attachment.bin>