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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13677
+      if (ExactRegion.isWrappedSet() || ExactRegion.isFullSet() ||
+          ExactRegion.getLower().ugt(ExactRegion.getUpper()))
+        return false;
----------------
Checking getLower/getUpper here shouldn't be necessary, as you already check for a wrapped set.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13688
+          getConstant(ExactRegion.getLower()),
+          getUMinExpr(RewrittenLHS, getConstant(ExactRegion.getUpper())));
+      return true;
----------------
Ranges are half-open, so I don't think getUpper() is correct here. I recommend using getUnsignedMin() and getUnsignedMax() instead. If you do that, then the wrapped set check is also no longer relevant for correctness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104741