[PATCH] D44676: [SCEV] Make exact taken count calculation more optimistic

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6657
 /// computable result can only be returned for loops with a single exit.
 /// Returning the minimum taken count among all exits is incorrect because one
 /// of the loop's exit limit's may have been skipped. howFarToZero assumes that
----------------
fhahn wrote:
> Please also update the comment here, stating when multiple exits are OK.
Will do before commit.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6673
+  // All exits we have collected must dominate the only latch.
+  if (!Latch)
+    return SE->getCouldNotCompute();
----------------
fhahn wrote:
> Another thing to double check. Are you sure we only compute exit counts when we have a single loop latch that dominates all exiting nodes? I had a quick look but could not find the place where we ensure that (computeBackedgeTakenCount states that the loop latch may be null). In that case we might miss a case when all exit counts are equal.
> 
The latch should be dominated by all exiting blocks, yes. It follows from current implementation of computeBackedgeTakenCount . It currently has *very* strict limitations that in practice say that the exit should actually be the latch (its successors being the loop's header and its chain of single predecessors also leads to header), so this requirement exists but implicit. If you look at https://reviews.llvm.org/D44677, here I tried to explain that the current requirement set is actually noting but a small subset of all situations when all exits dominate the latch. And in this patch, this requirement replaces all odd logic and becomes explicit.


https://reviews.llvm.org/D44676