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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2567
 
+  // (-1 + zext(A)) & Op1 --> A ? 0 : Op1
+  // Op0 & (-1 + zext(A)) --> A ? 0 : Op0
----------------
This comment and the title are missing an important detail that the value but me 0/1 (either via load range as is your alive2 link or from i1)


================
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) ||
----------------
The add doesn't need to be nsw


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2575
+        Constant::getNullValue(Ty), Op1);
+  if (match(Op1, m_NSWAdd(m_ZExt(m_Value(A)), m_AllOnes())) &&
+      (A->getType()->isIntOrIntVectorTy(1) ||
----------------
Instead of duplicating the logic for op1/op2 use 'match(&I, m_c_And(...))`


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2577
+      (A->getType()->isIntOrIntVectorTy(1) ||
+       (~computeKnownBits(A, 0, dyn_cast<Instruction>(Op1)).Zero) == 1))
+    return SelectInst::Create(
----------------
Comment that 0 refers to depth.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2577
+      (A->getType()->isIntOrIntVectorTy(1) ||
+       (~computeKnownBits(A, 0, dyn_cast<Instruction>(Op1)).Zero) == 1))
+    return SelectInst::Create(
----------------
goldstein.w.n wrote:
> Comment that 0 refers to depth.
Dont cast op1 cxti, make a simplifyquery at `I` (sorry not computer at the moment, so can't lookup exact syntax)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153148