[PATCH] D35539: [IRCE] Handle loops with step different from 1/-1

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

The change generally looks good to me. I'll take a closer look tomorrow. Some minor comments inline.



================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:807
 
-  auto IsInductionVar = [&](const SCEVAddRecExpr *AR, bool &IsIncreasing) {
+  auto IsInductionVar = [&](const SCEVAddRecExpr *AR, bool &IsIncreasing,
+                            ConstantInt *&StepCI) {
----------------
Pls Add a comment here that we calculate `StepCI` and whether the step is increasing or not (apart from identifying if `AR` is an IV).


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:821
+      StepCI = StepExpr->getValue();
+      assert(!StepCI->isZero() && "Zero step?");
+      IsIncreasing = !StepCI->isNegative();
----------------
could you pls check this assert in as NFC.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1594
   ConstantInt *ConstD = D->getValue();
-  if (!(ConstD->isMinusOne() || ConstD->isOne()))
-    return None;
+  assert(!ConstD->isZero() && "Recurrence with zero step?");
 
----------------
NFC assert here can be checked in separately (with the above NFC assert).


https://reviews.llvm.org/D35539