[PATCH] D16109: [ValueTracking] Improve known bits detection for PHI recurrences

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Analysis/ValueTracking.cpp:1461
@@ -1459,3 +1460,3 @@
           else if (LR == I)
-            L = LL;
-          else
+            NewL = LL;
+          if (NewL) {
----------------
Minor and subject to your discretion, but summarizing here what you've matched so far may be helpful for reading:

```
P = phi(NewL `op` P, R)
```

I'm also not sold on the name `NewL` -- what does "New" signify?

================
Comment at: lib/Analysis/ValueTracking.cpp:1472
@@ +1471,3 @@
+            KnownZero = APInt::getLowBitsSet(
+                BitWidth, std::min(KnownZero2.countTrailingOnes(),
+                                   KnownZero3.countTrailingOnes()));
----------------
Minor, but for multiplications I think we can actually use `KnownZero2.countTrailingOnes()` here (sometimes better and never worse than the min) -- do you mind adding a FIXME / TODO?

Edit: I noticed that this change doesn't really change this bit of code in a semantic way, so adding the TODO / FIXME isn't necessary.

================
Comment at: lib/Analysis/ValueTracking.cpp:1479
@@ +1478,3 @@
+        // prove that this value is always non-negative.
+        if (isa<ConstantInt>(R) && !cast<ConstantInt>(R)->isNegative()) {
+          Value *V = L;
----------------
Since you're using the pattern few times here, I think it'll be cleaner if you add an `m_NonNegative` to `PatterMatch.h` and use that here and later (there already exist matchers like `m_Power2` etc.).

================
Comment at: lib/Analysis/ValueTracking.cpp:1496
@@ +1495,3 @@
+            }
+            if ((match(V, m_Select(m_Value(Dummy), m_ConstantInt(CI),
+                                   m_Value(NewV))) ||
----------------
Use `m_Value()` (without the `Dummy`) since you want to ignore whatever you matched.


Repository:
  rL LLVM

http://reviews.llvm.org/D16109