[PATCH] D15922: [Cloning] rename cloneLoopWithPreheader() and add assert to ensure no sub-loops

[Ashutosh Nema via llvm-commits]

ashutosh.nema added inline comments.

================
Comment at: include/llvm/Transforms/Utils/Cloning.h:224
@@ -223,2 +223,3 @@
 /// \p LoopDomBB.  Insert the new blocks before block specified in \p Before.
-Loop *cloneLoopWithPreheader(BasicBlock *Before, BasicBlock *LoopDomBB,
+/// \p OrigLoop may not have sub-loops.
+Loop *cloneInnerLoopWithPreheader(BasicBlock *Before, BasicBlock *LoopDomBB,
----------------
may not => should not

================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:705
@@ -704,2 +704,3 @@
 /// \p LoopDomBB.  Insert the new blocks before block specified in \p Before.
-Loop *llvm::cloneLoopWithPreheader(BasicBlock *Before, BasicBlock *LoopDomBB,
+/// \p OrigLoop may not have sub-loops.
+Loop *llvm::cloneInnerLoopWithPreheader(BasicBlock *Before, BasicBlock *LoopDomBB,
----------------
may not => should not

================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:706
@@ -706,1 +705,3 @@
+/// \p OrigLoop may not have sub-loops.
+Loop *llvm::cloneInnerLoopWithPreheader(BasicBlock *Before, BasicBlock *LoopDomBB,
                                    Loop *OrigLoop, ValueToValueMapTy &VMap,
----------------
cloneInnerLoopWithPreheader looks little confusing, consider L1 { L2{ L3{} } }, L2 is also a inner loop, But here we clone innermost loop only. How about cloneInnermostLoopWithPreheader ?

================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:711
@@ -709,2 +710,3 @@
                                    SmallVectorImpl<BasicBlock *> &Blocks) {
+  assert(OrigLoop->getSubLoops().empty());
   Function *F = OrigLoop->getHeader()->getParent();
----------------
Please assert with some message, i.e. Loop is not innermost


http://reviews.llvm.org/D15922