[PATCH] D44776: [IRCE] Enable decreasing loops of variable bounds

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:697
 
-static bool CanBeMax(ScalarEvolution &SE, const SCEV *S, bool Signed) {
-  APInt Max = Signed ?
-      APInt::getSignedMaxValue(cast<IntegerType>(S->getType())->getBitWidth()) :
-      APInt::getMaxValue(cast<IntegerType>(S->getType())->getBitWidth());
-  return SE.getSignedRange(S).contains(Max) &&
-         SE.getUnsignedRange(S).contains(Max);
+static bool CannotBeMaxInLoop(const SCEV *BoundSCEV, Loop *L,
+                              ScalarEvolution &SE, bool Signed) {
----------------
BTW, this (and same for min) look general enough to be moved to Scalar Evolution analysis. Please consider making this in future, maybe someone else will need them. For now, I'm OK with what we have here.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:716
+  if (Pred != ICmpInst::ICMP_SLT && Pred != ICmpInst::ICMP_SGT &&
+      Pred != ICmpInst::ICMP_ULT && Pred != ICmpInst::ICMP_UGT)
+    return false;
----------------
Just a general observation (this problem seems to exist in old code as well): if the predicate is unsigned, the loop cannot be decreasing. In unsigned, all values are seen as non-negative. So this logic makes no sense. Could you please check that we only come here with signed predicates? If so, please add an assert on that.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:718
+    return false;
+
+  if (!SE.isAvailableAtLoopEntry(BoundSCEV, L))
----------------
Assert that step is known negative?


https://reviews.llvm.org/D44776