[PATCH] D122757: [InstSimplify] Fold (ctpop(X) == N) || (X != 0) into X != 0 where N > 0

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.





================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:1811
+  Value *X;
+  const APInt *N;
+  if (!match(Cmp0, m_ICmp(Pred0, m_Intrinsic<Intrinsic::ctpop>(m_Value(X)),
----------------
Normally in this kind of code, we name this "C" (for constant).


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:1814
+                          m_APInt(N))) ||
+      !match(Cmp1, m_ICmp(Pred1, m_Specific(X), m_SpecificInt(0))) ||
+      !N->ugt(0))
----------------
Use m_ZeroInt() instead of m_SpecificInt()?


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:1815
+      !match(Cmp1, m_ICmp(Pred1, m_Specific(X), m_SpecificInt(0))) ||
+      !N->ugt(0))
+    return nullptr;
----------------
It seems more natural if we write this as "!C->isZero()"


================
Comment at: llvm/test/Transforms/InstCombine/ispow2.ll:900
 
 define i1 @is_pow2or0_ctpop_wrong_pred2_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_wrong_pred2_logical(
----------------
We should add a new test for "wrong predicate" because this test and the later one are reduced before we reach the transform in InstCombine.


================
Comment at: llvm/test/Transforms/InstSimplify/and-or-icmp-ctpop.ll:35
+  %notzero = icmp ne <2 x i32> %x, <i32 0, i32 0>
+  %r = or <2 x i1> %notzero, %cmp
+  ret <2 x i1> %r
----------------
Should this be `or <2 x i1> %cmp, %notzero`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122757