[PATCH] D140851: [Patch 3/4]: Add cases for assume (X & Y != {0|Y})

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/AssumptionCache.cpp:117
     }
 
+    else if (Pred == ICmpInst::ICMP_NE) {
----------------
nit: Drop newline


================
Comment at: llvm/lib/Analysis/AssumptionCache.cpp:122
+      // information.
+      if (match(A, m_And(m_Value(X), m_Value(Y)))) {
+        AddAffected(X);
----------------
Could add `match(B, m_Zero())` here and drop the case below, no longer relevant.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:944
+    case ICmpInst::ICMP_NE:
+      // assume (v & b != a)
+      if (match(Cmp, m_c_ICmp(Pred, m_c_And(m_V, m_Value(B)), m_Value(A))) &&
----------------
and drop comments below, merge conditions, etc. We're only handling the one case now.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:950
+            KnownBits AKnown = computeKnownBits(A, Depth + 1, QueryNoAC)
+                                   .anyextOrTrunc(BitWidth);
+            if (AKnown.isZero()) {
----------------
goldstein.w.n wrote:
> nikic wrote:
> > goldstein.w.n wrote:
> > > nikic wrote:
> > > > This should just check whether A is zero, no need to compute known bits.
> > > > This should just check whether A is zero, no need to compute known bits.
> > > 
> > > How can I check if a value is known zero w.o `computeKnownBits`? I see `isKnownNonZero` but not the inverse.
> > `match(A, m_Zero())` would be the usual way.
> > `match(A, m_Zero())` would be the usual way.
> 
> Won't that only match `Constant` types? Could miss a case no?
If all bits are known zero, uses should be replaced by a zero value. There is no need to handle non-constant case here.

In fact, you should also drop the isKnnownToBeAPowerOfTwo and computeKnownBits calls on B, because we don't just need a power of two, we need to know which power of two it is: It has to be constant. We can just use `match(B, m_Power2(Pow2))` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140851