Review comments

Change-Id: I5355b103c34cb5d49868f44e3f8c10b95764a531

Author: jrbyrnes

llvm/include/llvm/Analysis/CmpInstAnalysis.h

@@ -95,28 +95,28 @@ namespace llvm {
   /// Represents the operation icmp (X & Mask) pred C, where pred can only be
   /// eq or ne.
   struct DecomposedBitTest {
-    Value *X = nullptr;
+    Value *X;
     CmpInst::Predicate Pred;
     APInt Mask;
     APInt C;
   };
 
   /// Decompose an icmp into the form ((X & Mask) pred C) if possible.
   /// Unless \p AllowNonZeroC is true, C will always be 0. If \p
-  /// DecomposeBitMask is specified, then, for equality predicates, this will
+  /// DecomposeAnd is specified, then, for equality predicates, this will
   /// decompose bitmasking (e.g. implemented via `and`).
   std::optional<DecomposedBitTest>
   decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
                        bool LookThroughTrunc = true, bool AllowNonZeroC = false,
-                       bool DecomposeBitMask = false);
+                       bool DecomposeAnd = false);
 
   /// Decompose an icmp into the form ((X & Mask) pred C) if
   /// possible. Unless \p AllowNonZeroC is true, C will always be 0.
-  /// If \p DecomposeBitMask is specified, then, for equality predicates, this
+  /// If \p DecomposeAnd is specified, then, for equality predicates, this
   /// will decompose bitmasking (e.g. implemented via `and`).
   std::optional<DecomposedBitTest>
   decomposeBitTest(Value *Cond, bool LookThroughTrunc = true,
-                   bool AllowNonZeroC = false, bool DecomposeBitMask = false);
+                   bool AllowNonZeroC = false, bool DecomposeAnd = false);
 
 } // end namespace llvm
 

llvm/lib/Analysis/CmpInstAnalysis.cpp

@@ -76,11 +76,11 @@ Constant *llvm::getPredForFCmpCode(unsigned Code, Type *OpTy,
 std::optional<DecomposedBitTest>
 llvm::decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
                            bool LookThruTrunc, bool AllowNonZeroC,
-                           bool DecomposeBitMask) {
+                           bool DecomposeAnd) {
   using namespace PatternMatch;
 
   const APInt *OrigC;
-  if ((ICmpInst::isEquality(Pred) && !DecomposeBitMask) ||
+  if ((ICmpInst::isEquality(Pred) && !DecomposeAnd) ||
       !match(RHS, m_APIntAllowPoison(OrigC)))
     return std::nullopt;
 
@@ -102,7 +102,7 @@ llvm::decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
 
   switch (Pred) {
   default:
-    return std::nullopt;
+    llvm_unreachable("Unexpected predicate");
   case ICmpInst::ICMP_SLT: {
     // X < 0 is equivalent to (X & SignMask) != 0.
     if (C.isZero()) {
@@ -152,11 +152,14 @@ llvm::decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
   }
   case ICmpInst::ICMP_EQ:
   case ICmpInst::ICMP_NE: {
-    assert(DecomposeBitMask);
+    assert(DecomposeAnd);
     const APInt *AndC;
     Value *AndVal;
     if (match(LHS, m_And(m_Value(AndVal), m_APIntAllowPoison(AndC)))) {
-      Result = {AndVal /*X*/, Pred /*Pred*/, *AndC /*Mask*/, *OrigC /*C*/};
+      LHS = AndVal;
+      Result.Mask = *AndC;
+      Result.C = C;
+      Result.Pred = Pred;
       break;
     }
 
@@ -175,25 +178,24 @@ llvm::decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
     Result.X = X;
     Result.Mask = Result.Mask.zext(X->getType()->getScalarSizeInBits());
     Result.C = Result.C.zext(X->getType()->getScalarSizeInBits());
-  } else if (!Result.X) {
+  } else
     Result.X = LHS;
-  }
 
   return Result;
 }
 
 std::optional<DecomposedBitTest> llvm::decomposeBitTest(Value *Cond,
                                                         bool LookThruTrunc,
                                                         bool AllowNonZeroC,
-                                                        bool DecomposeBitMask) {
+                                                        bool DecomposeAnd) {
   using namespace PatternMatch;
   if (auto *ICmp = dyn_cast<ICmpInst>(Cond)) {
     // Don't allow pointers. Splat vectors are fine.
     if (!ICmp->getOperand(0)->getType()->isIntOrIntVectorTy())
       return std::nullopt;
     return decomposeBitTestICmp(ICmp->getOperand(0), ICmp->getOperand(1),
                                 ICmp->getPredicate(), LookThruTrunc,
-                                AllowNonZeroC, DecomposeBitMask);
+                                AllowNonZeroC, DecomposeAnd);
   }
   Value *X;
   if (Cond->getType()->isIntOrIntVectorTy(1) &&

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

@@ -878,7 +878,7 @@ static Value *foldSignedTruncationCheck(ICmpInst *ICmp0, ICmpInst *ICmp1,
     auto Res = llvm::decomposeBitTestICmp(
         ICmp->getOperand(0), ICmp->getOperand(1), Pred,
         /*LookThroughTrunc=*/true, /*AllowNonZeroC=*/false,
-        /*DecomposeBitMask=*/true);
+        /*DecomposeAnd=*/true);
     if (Res && Res->Pred == ICmpInst::ICMP_EQ) {
       X = Res->X;
       UnsetBitsMask = Res->Mask;

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

@@ -3730,29 +3730,31 @@ static Value *foldSelectBitTest(SelectInst &Sel, Value *CondVal, Value *TrueVal,
   Value *CmpLHS, *CmpRHS;
 
   if (match(CondVal, m_ICmp(Pred, m_Value(CmpLHS), m_Value(CmpRHS)))) {
-    auto Res = decomposeBitTestICmp(
-        CmpLHS, CmpRHS, Pred, /*LookThroughTrunc=*/true,
-        /*AllowNonZeroC=*/false, /*DecomposeBitMask=*/true);
-
-    if (!Res)
-      return nullptr;
+    if (ICmpInst::isEquality(Pred)) {
+      if (!match(CmpRHS, m_Zero()))
+        return nullptr;
 
-    V = CmpLHS;
-    AndMask = Res->Mask;
+      V = CmpLHS;
+      const APInt *AndRHS;
+      if (!match(CmpLHS, m_And(m_Value(), m_Power2(AndRHS))))
+        return nullptr;
 
-    if (!ICmpInst::isEquality(Pred)) {
+      AndMask = *AndRHS;
+    } else if (auto Res = decomposeBitTestICmp(CmpLHS, CmpRHS, Pred)) {
+      assert(ICmpInst::isEquality(Res->Pred) && "Not equality test?");
+      AndMask = Res->Mask;
       V = Res->X;
       KnownBits Known =
           computeKnownBits(V, /*Depth=*/0, SQ.getWithInstruction(&Sel));
       AndMask &= Known.getMaxValue();
-      CreateAnd = true;
-    }
-
-    Pred = Res->Pred;
+      if (!AndMask.isPowerOf2())
+        return nullptr;
 
-    if (!AndMask.isPowerOf2())
+      Pred = Res->Pred;
+      CreateAnd = true;
+    } else {
       return nullptr;
-
+    }
   } else if (auto *Trunc = dyn_cast<TruncInst>(CondVal)) {
     V = Trunc->getOperand(0);
     AndMask = APInt(V->getType()->getScalarSizeInBits(), 1);

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp

@@ -2765,7 +2765,7 @@ static bool detectShiftUntilBitTestIdiom(Loop *CurLoop, Value *&BaseX,
   auto MatchDecomposableConstantBitMask = [&]() {
     auto Res = llvm::decomposeBitTestICmp(
         CmpLHS, CmpRHS, Pred, /*LookThroughTrunc=*/true,
-        /*AllowNonZeroC=*/false, /*DecomposeBitMask=*/true);
+        /*AllowNonZeroC=*/false, /*DecomposeAnd=*/true);
     if (Res && Res->Mask.isPowerOf2()) {
       assert(ICmpInst::isEquality(Res->Pred));
       Pred = Res->Pred;