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

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13660
 
+    // Check if (LHS Predicate RHS) represent a compare for a range check with a
+    // single condition, as InstCombine may create. A check of the form
----------------
I found the wording on this confusing the first several times I read it.  Can you reword?  One suggestion below if helpful.

Check for a condition of the form (X - 1) < C.  InstCombine will create this form when combining two checks of the form x <= C and x >u 0.  

(Note the use of the <= to avoid need for C+1 in example.  Note avoid mention "range check" as X > 0 doesn't seem like an idiomatic range check to me.)


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13666
+    //   * X u> 0.
+    if (auto *AddExpr = dyn_cast<SCEVAddExpr>(LHS)) {
+      auto *C1 = dyn_cast<SCEVConstant>(AddExpr->getOperand(0));
----------------
Style wise: I'd suggest writing a lambda with the signature
matchSomeName = [&](SCEV *& X, SCEVConstant *&C).

I at least would find early return much easier to follow here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104741