[PATCH] D47899: [SCEV] Implement (non-exact) getSmallConstantTripCountUpperBound

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

The predicated trip count computation isn't modeling the loop in your example well; it thinks the maximum trip count is 2.  But I guess that's okay.



================
Comment at: include/llvm/Analysis/ScalarEvolution.h:731
+  /// value. The upper bound may be larger than the actual trip count.
+  /// Returns 0 if the trip count is unknown or cannot be estimated.
+  unsigned getSmallConstantTripCountUpperBound(const Loop *L);
----------------
The comment is backwards: the important thing is that actual trip count may be larger than the upper bound.


================
Comment at: include/llvm/Analysis/ScalarEvolution.h:732
+  /// Returns 0 if the trip count is unknown or cannot be estimated.
+  unsigned getSmallConstantTripCountUpperBound(const Loop *L);
+
----------------
I think I'd prefer something like "getSmallConstantTripCountEstimate()"?  Not sure.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6397
+    if (unsigned C = getConstantTripCount(TakenCount))
+      return C;
+  }
----------------
This is redundant: if the trip count is known, the max trip count is also known.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6404
+  SCEVUnionPredicate Ps;
+  const SCEV *PredBEC = getPredicatedBackedgeTakenCount(L, Ps);
+  if (PredBEC != getCouldNotCompute()) {
----------------
`getPredicatedBackedgeTakenInfo(L).getMax(L, this, PS)` would compute what you want more directly.


Repository:
  rL LLVM

https://reviews.llvm.org/D47899