[PATCH] D24203: [LoopUnroll] Properly update loop-info when cloning prologues and epilogues.

Michael Zolotukhin via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 7 17:19:04 PDT 2016


mzolotukhin added a comment.

Thanks for the review! I'll commit it soon.

Michael


================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:307-309
@@ +306,5 @@
+    const Loop *OldLoop = LI->getLoopFor(*BB);
+    Loop *&NewLoop = NewLoops[OldLoop];
+    if (!NewLoop) {
+      if (CreateRemainderLoop || OldLoop != L) {
+        // This is a first block belonging to OldLoop encountered in our RPO
----------------
chandlerc wrote:
> Factoring this into a helper or a lambda would make it more readable IMO because you could use early returns.
That's a good idea. I'll see if it makes the code better before I commit.

================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:309
@@ +308,3 @@
+    if (!NewLoop) {
+      if (CreateRemainderLoop || OldLoop != L) {
+        // This is a first block belonging to OldLoop encountered in our RPO
----------------
evstupac wrote:
> Should be simplified to just:
> if (CreateRemainderLoop) {
I assume that this comment can also be ignored now, but I'll expand on this just in case.

If we unroll a loop with inner loops, then even if the remainder is not a loop itself, it will contain cloned inner loops. This check catches exactly this case: if the block BB belongs not to the L (but to some of its children), then we'll need to create cloned version of this inner loop in the remainder.


https://reviews.llvm.org/D24203





More information about the llvm-commits mailing list