[PATCH] D64224: Keep the order of the basic blocks in the cloned loop as the original loop

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CloneFunction.cpp:783
+  for (BasicBlock *BB : OrigLoop->getBlocks()) {
+    Loop *CurLoop = LI->getLoopFor(BB);
+    Loop *&NewLoop = LMap[CurLoop];
----------------
Whitney wrote:
> hfinkel wrote:
> > Is this logic equivalent? The original code is going a pre-order traversal, and for each loop, it excludes blocks in an inner loop (when `CurLoop != LI->getLoopFor(BB)`). Here we're doing something for all blocks in the loop including those also in inner loops?
> The resulting cloned basic blocks will be the same, which are all the blocks in OrigLoop. The different is the order of the basic block. Example:
> Given OrigLoop:
> ```
> OuterHeader:
>   br InnerHeader
> InnerHeader:
>   br InnerHeader or OuterLatch
> OuterLatch:
>   br OuterHeader or OuterExit
> ```
> Output before this change:
> ```
> ClonedOuterHeader:
>   br ClonedInnerHeader
> ClonedOuterLatch:
>   br ClonedOuterHeader or ClonedOuterExit
> ClonedInnerHeader:
>   br ClonedInnerHeader or ClonedOuterLatch
> ```
> Output after this change:
> ```
> ClonedOuterHeader:
>   br ClonedInnerHeader
> ClonedInnerHeader:
>   br ClonedInnerHeader or ClonedOuterLatch
> ClonedOuterLatch:
>   br ClonedOuterHeader or ClonedOuterExit
> ```
> 
> FYI - this function is extended by me in https://reviews.llvm.org/rG7c1deeff4a67296654823a871fea5c1a2aef3b8a to support cloning a loop nest instead of only allow the innermost loop. But I didn't think properly about the ordering of the basic block at that time.
Ah, okay. I see. This loop is not part of the pre-order traversal, it's just cloning all of the blocks in the outer loop in order. That seems fine.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64224/new/

https://reviews.llvm.org/D64224