[PATCH] D148744: [InstCombine] Refactor foldSelectICmpAndOr to use `decomposeBitTestICmp` instead of bespoke logic

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 19 13:44:24 PDT 2023


goldstein.w.n created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
goldstein.w.n requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is essentially NFC as the cases `decomposeBitTestICmp` covers
that weren't already covered explicitly, will be canonicalized into
the cases explicitly covered. As well the unsigned cases don't apply
as the `Mask` is not a power of 2.

That being said, using a well established helper is less bug prone and
if some canonicalization changes, will prevent regressions here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148744

Files:
  llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp


Index: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -649,8 +649,8 @@
   Value *CmpRHS = IC->getOperand(1);
 
   unsigned C1Log;
-  bool IsEqualZero;
   bool NeedAnd = false;
+  CmpInst::Predicate Pred = IC->getPredicate();
   if (IC->isEquality()) {
     if (!match(CmpRHS, m_Zero()))
       return nullptr;
@@ -660,18 +660,15 @@
       return nullptr;
 
     C1Log = C1->logBase2();
-    IsEqualZero = IC->getPredicate() == ICmpInst::ICMP_EQ;
   } else {
-    // We also need to recognize (icmp slt X, 0) and (icmp sgt X, -1).
-    if (IC->getPredicate() == ICmpInst::ICMP_SGT && match(CmpRHS, m_AllOnes()))
-      IsEqualZero = true;
-    if (IC->getPredicate() == ICmpInst::ICMP_SLT && match(CmpRHS, m_Zero()))
-      IsEqualZero = false;
-    else
+    Value *Unused;
+    APInt C1;
+    if (!decomposeBitTestICmp(CmpLHS, CmpRHS, Pred, Unused, C1) ||
+        !C1.isPowerOf2())
       return nullptr;
 
-    C1Log = CmpLHS->getType()->getScalarSizeInBits() - 1;
-    NeedAnd = true;
+    C1Log = C1.logBase2();
+    NeedAnd = !match(CmpRHS, m_SpecificInt(C1));
   }
 
   Value *Or, *Y, *V = CmpLHS;
@@ -680,11 +677,11 @@
   if (match(FalseVal, m_Or(m_Specific(TrueVal), m_Power2(C2)))) {
     Y = TrueVal;
     Or = FalseVal;
-    NeedXor = !IsEqualZero;
+    NeedXor = Pred == ICmpInst::ICMP_NE;
   } else if (match(TrueVal, m_Or(m_Specific(FalseVal), m_Power2(C2)))) {
     Y = FalseVal;
     Or = TrueVal;
-    NeedXor = IsEqualZero;
+    NeedXor = Pred == ICmpInst::ICMP_EQ;
   } else {
     return nullptr;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148744.515079.patch
Type: text/x-patch
Size: 1717 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230419/58caff6c/attachment-0001.bin>


More information about the llvm-commits mailing list