[PATCH] D142271: [ValueTracking] Add KnownBits patterns `xor(x, x - 1)` and `and(x, -x)` for knowing upper bits to be zero

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

I believe this is missing negative tests (e.g. `x & -y`, `x ^ (x + 2)`). We should also have tests for commuted variants and basic vector tests.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1086
 
+    // Common idiom blsi is: and(x, -x) is common idiom for clearing all but
+    // lowest set bit. If we have a single known bit in x, we can clear all bits
----------------



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1097
+        APInt Mask = APInt::getZero(Known.One.getBitWidth());
+        Mask.setBits(MinBit + 1, Mask.getBitWidth());
+        Known.Zero |= Mask;
----------------
`APInt::getBitsSetFrom(BitWidth, MinBit + 1)`


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1099
+        Known.Zero |= Mask;
+        Known.One &= (~Mask);
+      }
----------------
Redundant parentheses.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1139
+         match(I->getOperand(1),
+               m_c_Add(m_AllOnes(), m_Specific(I->getOperand(0)))))) {
+      unsigned MinBit = std::min(Known.One.countTrailingZeros(),
----------------
I'd suggest to combine these two matches as `match(I, m_c_BinOp(m_Value(X), m_Add(m_Deferred(X), m_AllOnes())))`.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1143
+      APInt Mask = APInt::getZero(Known.One.getBitWidth());
+      Mask.setBits(MinBit + 1, Mask.getBitWidth());
+      Known ^= Known2;
----------------
`APInt::getBitsSetFrom` here again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142271