[PATCH] D67408: [IndVars] An implementation of loop predication without a need for speculation

[Evgeniy via Phabricator via llvm-commits]

ebrevnov added inline comments.


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2750
+  // form.  If we have a read-only loop, and we can tell that we must exit down
+  // a path which does not need any of the values computed within the loop, we
+  // can rewrite the loop to exit on the first iteration.  Note that this
----------------
reames wrote:
> ebrevnov wrote:
> > I can't find a place where this prerequisite is actually checked. Am I just missing it?
> The code is assuming lcssa, so the lack of phis in the exist block is what you're looking for.
Got it. Thanks!


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2816
+
+  // At this point, ExitingBlocks consistents of only those blocks which are
+  // predicatable.  Given that, we know we have at least one exit we can
----------------
consistents->consists or constituents?


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2824
+  // is that enough for *all* side effects?
+  for (BasicBlock *BB : L->blocks())
+    for (auto &I : *BB)
----------------
Did you consider adding support for llvm.experimental.widenable.condition in this or one of the next patches? My understanding is semantics of llvm.experimental.widenable.condition allows us to assume that exist is taken on the first hit thus we can ignore side effects and live outs coming from the code dominated by the intrinsic.


================
Comment at: test/Transforms/IndVarSimplify/loop-predication.ll:714
+
+; TODO: We're failing to compute an exit count for the bounds check.
+; From some quick analysis, it looks like we don't handle -1 step
----------------
Any plan to support that soon?


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

https://reviews.llvm.org/D67408