[PATCH] D153148: [InstCombine] Fold `(-1 + A) & B` into `A ? 0 : B` where A is effectively a bool

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2562
+  // i1>, or inst with !range {0, 2}).
+  if (match(&I, m_c_And(m_c_Add(m_ZExtOrSelf(m_Value(A)), m_AllOnes()),
+                        m_Value(B))) &&
----------------
`m_c_Add` is unnecessary due to constant on RHS.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2562
+  // i1>, or inst with !range {0, 2}).
+  if (match(&I, m_c_And(m_c_Add(m_ZExtOrSelf(m_Value(A)), m_AllOnes()),
+                        m_Value(B))) &&
----------------
nikic wrote:
> `m_c_Add` is unnecessary due to constant on RHS.
You need a one use restriction on the add and a multiuse test.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2567
+                      KnownBits::makeConstant(APInt(
+                          A->getType()->getScalarSizeInBits(), 1))) == true))
+    return SelectInst::Create(
----------------
Use `countMaxActiveBits() <= 1` instead of `KnownBits::ule`.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2569
+    return SelectInst::Create(
+        Builder.CreateTrunc(A, A->getType()->getWithNewBitWidth(1)),
+        Constant::getNullValue(Ty), B);
----------------
This gets canonicalized to an icmp eq 0, so you should directly emit that.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2569
+  // Op0 & (-1 + zext(A)) --> A ? 0 : Op0
+  if (match(Op0, m_NSWAdd(m_ZExt(m_Value(A)), m_AllOnes())) &&
+      (A->getType()->isIntOrIntVectorTy(1) ||
----------------
dtcxzyw wrote:
> nikic wrote:
> > goldstein.w.n wrote:
> > > The add doesn't need to be nsw
> > Why does this need a zext?
> It needs a zext to avoid creating redundant trunc (zext i1).
> 
You are always creating the trunc, so I don't understand how explicitly matching the zext avoids a redundant trunc. The trunc will get folded away later in either case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153148