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

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko accepted this revision.
apilipenko added a comment.
This revision is now accepted and ready to land.

LGTM.

It looks like you successfully avoid the pitfall the original  original loop predication fell into by filtering implicit exits and using trip counts. It makes me think that we can revisit loop predication again and go back to SCEV based implementation.



================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2763
+  // implicit exits (see below) before we know it's truly exact.
+  const SCEV *ExactBTC = SE->getBackedgeTakenCount(L);
+  if (isa<SCEVCouldNotCompute>(ExactBTC) ||
----------------
It looks like you need to check `hasLoopInvariantBackedgeTakenCount` first. Excerpt from `getBackedgeTakenCount` description:
```
  /// Note that it is not valid to call this method on a loop without a
  /// loop-invariant backedge-taken count (see
  /// hasLoopInvariantBackedgeTakenCount).
```


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2770
+  auto Filter = [&](BasicBlock *ExitingBB) {
+    // If our exitting block exits multiple loops, we can only rewrite the
+    // innermost one.  Otherwise, we're changing how many times the innermost
----------------
Typo. exitting - exiting


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2770-2772
+    // If our exitting block exits multiple loops, we can only rewrite the
+    // innermost one.  Otherwise, we're changing how many times the innermost
+    // loop runs before it exits. 
----------------
apilipenko wrote:
> Typo. exitting - exiting
I don't think I quite follow this comment. Can you please elaborate? Is there a test demonstrating the problem?


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2856
+      Value *RHS = ExactBTCV;
+      if (ECV->getType() != RHS->getType()) {
+        Type *WiderTy = SE->getWiderType(ECV->getType(), RHS->getType());
----------------
What is the situation when they have different types?


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

https://reviews.llvm.org/D67408