[PATCH] D33001: [RuntimeUnrolling] Add logic for loops with multiple exit blocks

[Anna Thomas via Phabricator via llvm-commits]

anna marked 3 inline comments as done.
anna added a comment.

Thanks Evgeny for the review. Updated diff.



================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:496
+  if (!Exit && UnrollRuntimeMultiExit) {
+    assert(UseEpilogRemainder && "Multi exit unrolling is currently supported "
+                                 "unrolling with epilog remainder only!");
----------------
evstupac wrote:
> How about passing "UnrollRuntimeMultiExit" as a parameter to UnrollRuntimeLoopRemainder" and introducing dependency like:
> UseEpilogRemainder |= UnrollRuntimeMultiExit for loops with multi exits and
> UnrollRuntimeMultiExit &= UseEpilogRemainder if UseEpilogRemainder is set by user?
> 
> 
> 
I think the `UnrollRuntimeMultiExit` can be just a cl::opt for now? The end goal is to support this for prolog and epilog remainder loop creation and under some heuristic that is profitable to create such loop. 


================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:691
 
+  // At this stage, the loop blocks are cloned for the prolog/epilog. Clone the
+  // other exit blocks as well for the epilog, and update the Vmap and dominator
----------------
evstupac wrote:
> Maybe it's ok, but according to the comment (line 685) we clone extra blocks right after inserting previous into function. The order needs an additional comment.
> 
> I'd recommend adding this part to "CloneLoopBlocks".
Yes, it's cleaner - added as part of CloneLoopBlocks.


https://reviews.llvm.org/D33001