[PATCH] D25499: BranchRelaxation: Fix computing indirect branch block size

[Matt Arsenault via llvm-commits]

arsenm updated this revision to Diff 74360.
arsenm added a comment.

Fix new assertion failing on AArch64 due to updateTerminator modifying block sizes


https://reviews.llvm.org/D25499

Files:
  lib/CodeGen/BranchRelaxation.cpp


Index: lib/CodeGen/BranchRelaxation.cpp
===================================================================
--- lib/CodeGen/BranchRelaxation.cpp
+++ lib/CodeGen/BranchRelaxation.cpp
@@ -78,7 +78,8 @@
 
   MachineBasicBlock *createNewBlockAfter(MachineBasicBlock &BB);
 
-  MachineBasicBlock *splitBlockBeforeInstr(MachineInstr &MI);
+  MachineBasicBlock *splitBlockBeforeInstr(MachineInstr &MI,
+                                           MachineBasicBlock *DestBB);
   void adjustBlockOffsets(MachineBasicBlock &MBB);
   bool isBlockInRange(const MachineInstr &MI, const MachineBasicBlock &BB) const;
 
@@ -116,6 +117,7 @@
     unsigned Num = MBB.getNumber();
     assert(BlockInfo[Num].Offset % (1u << Align) == 0);
     assert(!Num || BlockInfo[PrevNum].postOffset(MBB) <= BlockInfo[Num].Offset);
+    assert(BlockInfo[Num].Size == computeBlockSize(MBB));
     PrevNum = Num;
   }
 #endif
@@ -208,7 +210,8 @@
 /// NOTE: Successor list of the original BB is out of date after this function,
 /// and must be updated by the caller! Other transforms follow using this
 /// utility function, so no point updating now rather than waiting.
-MachineBasicBlock *BranchRelaxation::splitBlockBeforeInstr(MachineInstr &MI) {
+MachineBasicBlock *BranchRelaxation::splitBlockBeforeInstr(MachineInstr &MI,
+                                                           MachineBasicBlock *DestBB) {
   MachineBasicBlock *OrigBB = MI.getParent();
 
   // Create a new MBB for the code after the OrigBB.
@@ -228,6 +231,17 @@
   // Insert an entry into BlockInfo to align it properly with the block numbers.
   BlockInfo.insert(BlockInfo.begin() + NewBB->getNumber(), BasicBlockInfo());
 
+  MachineBasicBlock &MBB = *OrigBB;
+
+  NewBB->transferSuccessors(&MBB);
+  MBB.addSuccessor(NewBB);
+  MBB.addSuccessor(DestBB);
+
+  // Cleanup potential unconditional branch to successor block.
+  // Note that updateTerminator may change the size of the blocks.
+  NewBB->updateTerminator();
+  MBB.updateTerminator();
+
   // Figure out how large the OrigBB is.  As the first half of the original
   // block, it cannot contain a tablejump.  The size includes
   // the new jump we added.  (It should be possible to do this without
@@ -386,12 +400,9 @@
 
   DebugLoc DL = MI.getDebugLoc();
   MI.eraseFromParent();
-
-  // insertUnconditonalBranch may have inserted a new block.
-  BlockInfo[MBB->getNumber()].Size += TII->insertIndirectBranch(
+  BlockInfo[BranchBB->getNumber()].Size += TII->insertIndirectBranch(
     *BranchBB, *DestBB, DL, DestOffset - SrcOffset, RS.get());
 
-  computeBlockSize(*BranchBB);
   adjustBlockOffsets(*MBB);
   return true;
 }
@@ -418,14 +429,7 @@
             // analyzable block. Split later terminators into a new block so
             // each one will be analyzable.
 
-            MachineBasicBlock *NewBB = splitBlockBeforeInstr(*Next);
-            NewBB->transferSuccessors(&MBB);
-            MBB.addSuccessor(NewBB);
-            MBB.addSuccessor(DestBB);
-
-            // Cleanup potential unconditional branch to successor block.
-            NewBB->updateTerminator();
-            MBB.updateTerminator();
+            splitBlockBeforeInstr(*Next, DestBB);
           } else {
             fixupConditionalBranch(MI);
             ++NumConditionalRelaxed;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25499.74360.patch
Type: text/x-patch
Size: 3282 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161012/0c8f8887/attachment.bin>