[PATCH] Teach SplitBlockPredecessors how to handle landingpad blocks.

[Philip Reames]

Minor comments inline.  Once these are addressed, I'm willing to give an LGTM.  As we discussed offline, I plan on waiting another day or two to give other interested parties time to comment.


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Transforms/Utils/BasicBlockUtils.h:205
@@ -204,6 +204,3 @@
 
-/// SplitBlockPredecessors - This method transforms BB by introducing a new
-/// basic block into the function, and moving some of the predecessors of BB to
-/// be predecessors of the new block.  The new predecessors are indicated by the
-/// Preds array, which has NumPreds elements in it.  The new block is given a
-/// suffix of 'Suffix'.  This function returns the new block.
+/// SplitBlockPredecessors - This method transforms non landingpad BB by
+/// introducing a new basic block into the function, and moving some of the
----------------
I'd suggest organizing the comment as:
This method returns <high level comment, true of each>.  The specific transform performed depends on whether BB starts with a landingpad instruction.  

If BB does not contain a landingpad ....

If BB does start with a landingpad ...

================
Comment at: include/llvm/Transforms/Utils/BasicBlockUtils.h:208
@@ +207,3 @@
+/// predecessors of BB to be predecessors of the new block.  The new
+/// predecessors are indicated by the Preds array, which has NumPreds elements
+/// in it.  The new block is given a suffix of 'Suffix'.
----------------
Given there is no longer a NumPreds argument, you can delete that part.

================
Comment at: lib/Transforms/Utils/BasicBlockUtils.cpp:457
@@ -456,6 +456,3 @@
 
-/// SplitBlockPredecessors - This method transforms BB by introducing a new
-/// basic block into the function, and moving some of the predecessors of BB to
-/// be predecessors of the new block.  The new predecessors are indicated by the
-/// Preds array, which has NumPreds elements in it.  The new block is given a
-/// suffix of 'Suffix'.
+/// SplitBlockPredecessors - This method transforms non landingpad BB by
+/// introducing a new basic block into the function, and moving some of the
----------------
Given this is just duplicated from the header, delete the comment here.

================
Comment at: lib/Transforms/Utils/BasicBlockUtils.cpp:480
@@ +479,3 @@
+    SmallVector<BasicBlock*, 2> NewBBs;
+    std::string NewName = std::string(Suffix) + ".split-lp";
+
----------------
Not sure it makes sense to add the extra suffix here.  I'd just drop this.

http://reviews.llvm.org/D7157

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/