[VectorCombine] refactor matching code to reduce duplication; NFC

cmp/binop were already diverging even though they are largely
the same logic.

llvm/lib/Transforms/Vectorize/VectorCombine.cpp

@@ -33,31 +33,28 @@ using namespace llvm::PatternMatch;
 STATISTIC(NumVecCmp, "Number of vector compares formed");
 STATISTIC(NumVecBO, "Number of vector binops formed");
 
-static bool foldExtractCmp(Instruction &I, const TargetTransformInfo &TTI) {
+/// Try to reduce extract element costs by converting scalar compares to vector
-  // Match a cmp with extracted vector operands.
+/// compares followed by extract.
-  CmpInst::Predicate Pred;
+/// cmp (ext0 V0, C0), (ext1 V1, C1)
-  Instruction *Ext0, *Ext1;
+static bool foldExtExtCmp(Instruction *Ext0, Value *V0, uint64_t C0,
-  if (!match(&I, m_Cmp(Pred, m_Instruction(Ext0), m_Instruction(Ext1))))
+                          Instruction *Ext1, Value *V1, uint64_t C1,
-    return false;
+                          Instruction &I, const TargetTransformInfo &TTI) {
-
+  assert(isa<CmpInst>(&I) && "Expected a compare");
-  Value *V0, *V1;
-  ConstantInt *C;
-  if (!match(Ext0, m_ExtractElement(m_Value(V0), m_ConstantInt(C))) ||
-      !match(Ext1, m_ExtractElement(m_Value(V1), m_Specific(C))) ||
-      V0->getType() != V1->getType())
-    return false;
-
   Type *ScalarTy = Ext0->getType();
   Type *VecTy = V0->getType();
   bool IsFP = ScalarTy->isFloatingPointTy();
   unsigned CmpOpcode = IsFP ? Instruction::FCmp : Instruction::ICmp;
 
+  // TODO: Handle C0 != C1 by shuffling 1 of the operands.
+  if (C0 != C1)
+    return false;
+
   // Check if the existing scalar code or the vector alternative is cheaper.
   // Extra uses of the extracts mean that we include those costs in the
   // vector total because those instructions will not be eliminated.
   // ((2 * extract) + scalar cmp) < (vector cmp + extract) ?
-  int ExtractCost = TTI.getVectorInstrCost(Instruction::ExtractElement,
+  int ExtractCost =
-                                           VecTy, C->getZExtValue());
+      TTI.getVectorInstrCost(Instruction::ExtractElement, VecTy, C0);
   int ScalarCmpCost = TTI.getCmpSelInstrCost(CmpOpcode, ScalarTy, I.getType());
   int VecCmpCost = TTI.getCmpSelInstrCost(CmpOpcode, VecTy,
                                           CmpInst::makeCmpResultType(VecTy));
@@ -72,38 +69,26 @@ static bool foldExtractCmp(Instruction &I, const TargetTransformInfo &TTI) {
   // cmp Pred (extelt V0, C), (extelt V1, C) --> extelt (cmp Pred V0, V1), C
   ++NumVecCmp;
   IRBuilder<> Builder(&I);
+  CmpInst::Predicate Pred = cast<CmpInst>(&I)->getPredicate();
   Value *VecCmp = IsFP ? Builder.CreateFCmp(Pred, V0, V1)
                        : Builder.CreateICmp(Pred, V0, V1);
-  Value *Ext = Builder.CreateExtractElement(VecCmp, C);
+  Value *Extract = Builder.CreateExtractElement(VecCmp, Ext0->getOperand(1));
-  I.replaceAllUsesWith(Ext);
+  I.replaceAllUsesWith(Extract);
   return true;
 }
 
 /// Try to reduce extract element costs by converting scalar binops to vector
 /// binops followed by extract.
-static bool foldExtractBinop(Instruction &I, const TargetTransformInfo &TTI) {
+/// bo (ext0 V0, C0), (ext1 V1, C1)
-  // It is not safe to transform things like div, urem, etc. because we may
+static bool foldExtExtBinop(Instruction *Ext0, Value *V0, uint64_t C0,
-  // create undefined behavior when executing those on unknown vector elements.
+                            Instruction *Ext1, Value *V1, uint64_t C1,
-  if (!isSafeToSpeculativelyExecute(&I))
+                            Instruction &I, const TargetTransformInfo &TTI) {
-    return false;
+  assert(isa<BinaryOperator>(&I) && "Expected a binary operator");
-
+  Type *ScalarTy = Ext0->getType();
-  // Match a scalar binop with extracted vector operands:
+  Type *VecTy = V0->getType();
-  // bo (extelt X, C0), (extelt Y, C1)
+  Instruction::BinaryOps BOpcode = cast<BinaryOperator>(I).getOpcode();
-  Instruction *Ext0, *Ext1;
-  if (!match(&I, m_BinOp(m_Instruction(Ext0), m_Instruction(Ext1))))
-    return false;
-
-  Value *X, *Y;
-  uint64_t C0, C1;
-  if (!match(Ext0, m_ExtractElement(m_Value(X), m_ConstantInt(C0))) ||
-      !match(Ext1, m_ExtractElement(m_Value(Y), m_ConstantInt(C1))) ||
-      X->getType() != Y->getType())
-    return false;
 
   // Check if using a vector binop would be cheaper.
-  Instruction::BinaryOps BOpcode = cast<BinaryOperator>(I).getOpcode();
-  Type *ScalarTy = I.getType();
-  Type *VecTy = X->getType();
   int ScalarBOCost = TTI.getArithmeticInstrCost(BOpcode, ScalarTy);
   int VecBOCost = TTI.getArithmeticInstrCost(BOpcode, VecTy);
   int Extract0Cost = TTI.getVectorInstrCost(Instruction::ExtractElement,
@@ -119,7 +104,7 @@ static bool foldExtractBinop(Instruction &I, const TargetTransformInfo &TTI) {
                TTI.getVectorInstrCost(Instruction::ExtractElement, VecTy, C1) &&
            "Different costs for same extract?");
     int ExtractCost = Extract0Cost;
-    if (X != Y) {
+    if (V0 != V1) {
       int ScalarCost = ExtractCost + ExtractCost + ScalarBOCost;
       int VecCost = VecBOCost + ExtractCost +
                     !Ext0->hasOneUse() * ExtractCost +
@@ -129,7 +114,7 @@ static bool foldExtractBinop(Instruction &I, const TargetTransformInfo &TTI) {
     } else {
       // Handle an extra-special case. If the 2 binop operands are identical,
       // adjust the formulas to account for that:
-      // bo (extelt X, C), (extelt X, C) --> extelt (bo X, X), C
+      // bo (extelt V, C), (extelt V, C) --> extelt (bo V, V), C
       // The extra use charge allows for either the CSE'd pattern or an
       // unoptimized form with identical values.
       bool HasUseTax = Ext0 == Ext1 ? !Ext0->hasNUses(2)
@@ -143,7 +128,7 @@ static bool foldExtractBinop(Instruction &I, const TargetTransformInfo &TTI) {
     // bo (extelt X, C), (extelt Y, C) --> extelt (bo X, Y), C
     ++NumVecBO;
     IRBuilder<> Builder(&I);
-    Value *NewBO = Builder.CreateBinOp(BOpcode, X, Y);
+    Value *NewBO = Builder.CreateBinOp(BOpcode, V0, V1);
     if (auto *VecBOInst = dyn_cast<Instruction>(NewBO)) {
       // All IR flags are safe to back-propagate because any potential poison
       // created in unused vector elements is discarded by the extract.
@@ -158,6 +143,32 @@ static bool foldExtractBinop(Instruction &I, const TargetTransformInfo &TTI) {
   return false;
 }
 
+/// Match an instruction with extracted vector operands.
+static bool foldExtractExtract(Instruction &I, const TargetTransformInfo &TTI) {
+  Instruction *Ext0, *Ext1;
+  CmpInst::Predicate Pred = CmpInst::BAD_ICMP_PREDICATE;
+  if (!match(&I, m_Cmp(Pred, m_Instruction(Ext0), m_Instruction(Ext1))) &&
+      !match(&I, m_BinOp(m_Instruction(Ext0), m_Instruction(Ext1))))
+    return false;
+
+  Value *V0, *V1;
+  uint64_t C0, C1;
+  if (!match(Ext0, m_ExtractElement(m_Value(V0), m_ConstantInt(C0))) ||
+      !match(Ext1, m_ExtractElement(m_Value(V1), m_ConstantInt(C1))) ||
+      V0->getType() != V1->getType())
+    return false;
+
+  if (Pred != CmpInst::BAD_ICMP_PREDICATE)
+    return foldExtExtCmp(Ext0, V0, C0, Ext1, V1, C1, I, TTI);
+
+  // It is not safe to transform things like div, urem, etc. because we may
+  // create undefined behavior when executing those on unknown vector elements.
+  if (isSafeToSpeculativelyExecute(&I))
+    return foldExtExtBinop(Ext0, V0, C0, Ext1, V1, C1, I, TTI);
+
+  return false;
+}
+
 /// This is the entry point for all transforms. Pass manager differences are
 /// handled in the callers of this function.
 static bool runImpl(Function &F, const TargetTransformInfo &TTI,
@@ -172,10 +183,8 @@ static bool runImpl(Function &F, const TargetTransformInfo &TTI,
     // use->defs, so we're more likely to succeed by starting from the bottom.
     // TODO: It could be more efficient to remove dead instructions
     //       iteratively in this loop rather than waiting until the end.
-    for (Instruction &I : make_range(BB.rbegin(), BB.rend())) {
+    for (Instruction &I : make_range(BB.rbegin(), BB.rend()))
-      MadeChange |= foldExtractCmp(I, TTI);
+      MadeChange |= foldExtractExtract(I, TTI);
-      MadeChange |= foldExtractBinop(I, TTI);
-    }
   }
 
   // We're done with transforms, so remove dead instructions.