[PATCH] D69009: [IndVars] Eliminate loop exits with equivalent exit counts

[Philip Reames via Phabricator via llvm-commits]

reames marked 2 inline comments as done.
reames added inline comments.


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2758
+  SmallSet<const SCEV*, 8> DominatingExitCounts;
+  for (BasicBlock *ExitingBB : ExitingBlocks) {
+    // If our exitting block exits multiple loops, we can only rewrite the
----------------
ebrevnov wrote:
> That would be a fourth loop filtering out "bad" exits before actual work. And the method becomes pretty long. I think we need to refactor in this or consequent patch. 
What do you think of the committed NFC reorgs?  If you have suggestions, I'm happy to further tweak. 


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2793
+    // strictly follows another which does the same and is thus dead.
+    if (!DominatingExitCounts.insert(ExitCount).second) {
+      bool ExitIfTrue = !L->contains(*succ_begin(ExitingBB));
----------------
ebrevnov wrote:
> Two notes here. 1) Any reason not to use isKnownViaNonRecursiveReasoning or isKnownPredicate? 2) We can actually eliminate an exit if its exit count not less than exit count of one of its predecessors.
> 
> None of these is blocking the current patch though.
2) I believe you're referring not to the EQ case, but to the provably greater then case just above right?

1) I could generalize, but I don't have an example which illustrates the difference.  SCEV tries to canonicalize at construction (as demonstrated by some of the test changes). If you don't mind, I'd like to wait until we have a motivating example before using a more expensive API.   


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

https://reviews.llvm.org/D69009