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

Nicolai Hähnle via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 14 01:31:52 PDT 2016


nhaehnle added inline comments.


================
Comment at: lib/CodeGen/BranchRelaxation.cpp:210-212
 /// 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.
----------------
Is this comment still accurate?


================
Comment at: lib/CodeGen/BranchRelaxation.cpp:234-244
+  MachineBasicBlock &MBB = *OrigBB;
+
+  NewBB->transferSuccessors(&MBB);
+  MBB.addSuccessor(NewBB);
+  MBB.addSuccessor(DestBB);
+
+  // Cleanup potential unconditional branch to successor block.
----------------
I think just using OrigBB instead of MBB would help readability.


================
Comment at: lib/CodeGen/BranchRelaxation.cpp:401-405
   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());
 
----------------
This doesn't seem to be affected by your change, but I'm a bit confused about not seeing anything to account for the size change in MBB due to the erased MI.


https://reviews.llvm.org/D25499





More information about the llvm-commits mailing list