[PATCH] D18158: Adding ability to unroll loops using epilogue remainder.

[Evgeny Stupachenko via llvm-commits]

evstupac added inline comments.

================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:297
@@ -292,2 +296,3 @@
+  // Don't output the runtime loop remainder if Count is a multiple of
   // TripMultiple.  Such a prolog is never needed, and is unsafe if the loop
   // contains a convergent instruction.
----------------
zzheng wrote:
> 'prolog'?
> 
> I recommend changing prolog/remainder here to 'leftover'
Thanks for catching this. For sure it is not only for prolog it is for all kind of remainder loops. "leftover" is also good, but since I used "remainder" in all other places it is better to keep the same name here.

================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:146
@@ +145,3 @@
+/// - Update PHI nodes at the unrolling loop exit and epilog loop exit
+/// - Create PHI nodes at the unrolling loop exit to combine
+///   values that exit the unrolling loop code and jump around it.
----------------
zzheng wrote:
> You mean 'unrolled loop', right?
Actually the loop is in unrolling process. Here in "UnrollRuntimeLoopRemainder" we only prepare loop for unroll. Actual unroll occurs at UnrollLoop function. So I'd like to keep "unrolling" here.

================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:191
@@ +190,3 @@
+    // NewExit was spilt 1 more time to get EpilogPreHeader.
+    assert(PN->hasOneUse() && "The phi should have 1 use");
+    PHINode *EpilogPN = cast<PHINode> (PN->use_begin()->getUser());
----------------
zzheng wrote:
> PN shoudl have 2 uses, right? One in epilogue loop or its preheader and one in Exit.
> 
> Please clarify it only has 1 use for now as we'll add the use in epilogue later.
It has one use after splitting. It will have 2 uses. Line 196 add incoming value from PreHeader. Lines 214-217 explain how it will look like. The loop structure are commented at lines 162-172.




http://reviews.llvm.org/D18158