[PATCH] D12800: [ValueTracking] Teach isKnownNonZero about monotonically increasing PHIs

[Philip Reames via llvm-commits]

reames added a subscriber: reames.
reames added a comment.

A couple of meta comments:

- This is duplicating logic in SCEV.  I think that's okay in this case, but we don't want to take that to the extreme.
- Adding the analogous case isKnownNonNegative (aka, using it to set the sign bit in computeKnownBits) would be a really good idea.  An increasing induction variable starting at zero is pretty much the ideal canonical induction variable and we should exploit that.


================
Comment at: lib/Analysis/ValueTracking.cpp:1904
@@ +1903,3 @@
+    if (PN->getNumIncomingValues() == 2) {
+      Value *Op1 = PN->getIncomingValue(0);
+      Value *Op2 = PN->getIncomingValue(1);
----------------
I find the Op1 vs Op0 naming confusing.  Given this is inherently zero based indexing, mind either using that in the Op names or picking non-index based ones?

================
Comment at: lib/Analysis/ValueTracking.cpp:1909
@@ +1908,3 @@
+      if (ConstantInt *COp1 = dyn_cast<ConstantInt>(Op1)) {
+        if (!COp1->isZero() && !COp1->isNegative()) {
+          ConstantInt *X;
----------------
Might be time to introduce a isNonNegative on ConstantInt?  Could be another patch.

================
Comment at: lib/Analysis/ValueTracking.cpp:1912
@@ +1911,3 @@
+          if (match(Op2, m_NSWAdd(m_Specific(PN), m_ConstantInt(X))) &&
+              match(Op2, m_NUWAdd(m_Specific(PN), m_ConstantInt(X))) &&
+              !X->isNegative())
----------------
Given you're starting from zero, I think you only need the NSW check.  Given INT_MAX is always less than UINT_MAX, I believe the former implies the later in this case.  (But we might not yet have proved it.)  


Repository:
  rL LLVM

http://reviews.llvm.org/D12800