[PATCH] D97122: [SCEV] Improve handling of pointer compares involving subtractions (WIP).

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1840
+                       getZeroExtendExpr(SM->getOperand(1), Ty));
+  }
+
----------------
Couple of comments on this transform:
1) It's not clear why (or if) this is a profitable canonicalization.  We're replacing one zext with two.  I'd want an explanation of why this was the right idea, and in particular, why not to canonicalize the other direction.
2) umax can have more than two operands.
3) this should be a separate change once you're ready for actual review.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3121
+  //   * X s>= C1
+  //   * C1 s>= 0
+  //
----------------
Another way to consider approaching this:
(-C1 + X) /u C2
== -C1/C2 + (-C1 % C2 + X) /u C2

(i.e. extract out the whole integer divisions and leave only the possible fractional part)


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:10979
+  auto *Add = dyn_cast<SCEVAddExpr>(LHS);
+  if (Add && Pred == CmpInst::ICMP_ULE) {
+    auto *X = Add->getOperand(0);
----------------
You can also handle ULT without much work.  isKnownNonNegative becomes isKnownPositive in that case.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:10982
+    auto *Y = Add->getOperand(1);
+    if (Y == RHS && isKnownNonPositive(X) && isKnownNonNegative(Y) &&
+        isKnownPredicateViaConstantRanges(CmpInst::ICMP_UGE, Y,
----------------
Do we canonicalize x + (-Y) to (-Y) + X somewhere?  If not, you're missing some pattern matching cases here.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97122