[PATCH] D104741: [SCEV] Support single-cond range check idiom in applyLoopGuards.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13660-13661
 
+    // Check for a condition of the form (X - C1) < C2.  InstCombine will create
+    // this form when combining two checks of the form x u<= C2 and x >=u C1.
+    auto MatchRangeCheckIdiom = [this, Predicate, LHS, RHS, &RewriteMap]() {
----------------
lebedev.ri wrote:
> This comment doesn't make sense right now.
> https://godbolt.org/z/n7qqsTY9e
Thanks, I adjusted the comment, but kept the form where `C1` is negative in the original condition, as this seems more common in practice.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13668
+      auto *C1 = dyn_cast<SCEVConstant>(AddExpr->getOperand(0));
+      auto *C2 = dyn_cast<SCEVConstant>(RHS);
+      if (AddExpr->getNumOperands() != 2 || !C1 || !C2)
----------------
nikic wrote:
> ... and then fetch LHSUnknown and check it already here. That seems like a more obvious grouping of conditions.
Thanks, I adjusted the checks as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104741