[PATCH] D44288: [WIP][LICM] Extend must execute to path taken on first iteration

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1519
 
+/// Return true if we can prove that the given ExitBlock is not reached on the
+/// first iteration of the given loop.  That is, the backedge of the loop must
----------------
This comment looks out of sync. The method does not return `true`, and it does not have any `ExitBlock` as a parameter.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1520
+/// Return true if we can prove that the given ExitBlock is not reached on the
+/// first iteration of the given loop.  That is, the backedge of the loop must
+/// be executed before the ExitBlock is executed in any dynamic execution trace.
----------------
Assert that the loop has one backedge?


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1522
+/// be executed before the ExitBlock is executed in any dynamic execution trace.
+static BasicBlock* ProveDirectionOnFirstIteration(BranchInst *BI,
+                                                  const DominatorTree *DT,
----------------
Asterisk misplaced.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1533
+  if (!ICI)
+    return nullptr;
+  // Check for cmp (phi [x, preheader] ...), y where (pred x, y is known
----------------
Make it `Optional` and return `None` as an option.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1589
+        if (Next == CurLoop->getHeader())
+          // Hit cycle - todo: handle inner cycles!
+          break;
----------------
How about `if (LI.isLoopHeader(Next))`? You have just check that you don't leave the current loop, so any header is either your header or a header of some inner loop.


https://reviews.llvm.org/D44288