[PATCH] D12073: Make ScalarEvolution::isKnownPredicate a little smarter

[hfinkel@anl.gov via llvm-commits]

hfinkel marked 2 inline comments as done.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:7357
@@ +7356,3 @@
+    return false;
+  if (LAR->getLoop() != RAR->getLoop())
+    return false;
----------------
sanjoy wrote:
> Optional: I'd have just folded the null checks into this condition.
I did not do that so that I could easily avoid the second dyn_cast should the first one fail.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:7367
@@ +7366,3 @@
+  auto CheckWrap = [Pred](const SCEVAddRecExpr *AR) -> bool {
+    if (ICmpInst::getSignedPredicate(Pred) == Pred) {
+      if (!AR->getNoWrapFlags(SCEV::FlagNSW))
----------------
sanjoy wrote:
> I'd use `CmpInst::isSigned(Pred)` here.
Good suggestion.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:7368
@@ +7367,3 @@
+    if (ICmpInst::getSignedPredicate(Pred) == Pred) {
+      if (!AR->getNoWrapFlags(SCEV::FlagNSW))
+        return false;
----------------
sanjoy wrote:
> Why not `return AR->getNoWrapFlags(SCEV::FlagNSW);` here, and `return AR->getNoWrapFlags(SCEV::FlagNUW);` in the `else` block?
Also a good suggestion.



http://reviews.llvm.org/D12073