[SLP][NFC] Cleanup: Outline the code that vectorizes CmpInsts into a seaparate function.

[ABataev]

llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h

148

ArrayRef<Value *>

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

14417

try to replace with dyn_cast

[vporpo]

llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h

148

Isn't it better to use ArrayRef<CmpInst *> instead of ArrayRef<Value *> ?

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

14417

What do you mean? Like this?

auto *Select = dyn_cast<SelectInst>(U);
return Select && Select->getParent() != cast<Instruction>(V)->getParent();

[ABataev]

llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h

148

Yes, I mean use ArrayRef instead of const SmallVector &

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

14417

Yes

[vporpo]

Addressed comments.

[ABataev]

LG

llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h

private:
   /// Try to vectorize trees that start at insertvalue instructions.
   bool vectorizeInsertValueInst(InsertValueInst *IVI, BasicBlock *BB,
                                 slpvectorizer::BoUpSLP &R);
 
   /// Try to vectorize trees that start at insertelement instructions.
   bool vectorizeInsertElementInst(InsertElementInst *IEI, BasicBlock *BB,
                                   slpvectorizer::BoUpSLP &R);
 
+  /// Tries to vectorize \p CmpInts. \Returns true on success.
+  bool vectorizeCmpInsts(const SmallVector<CmpInst *, 4> &CmpInsts,
+                         BasicBlock *BB, slpvectorizer::BoUpSLP &R);
+
   /// Tries to vectorize constructs started from CmpInst, InsertValueInst or
   /// InsertElementInst instructions.
   bool vectorizeSimpleInstructions(InstSetVector &Instructions, BasicBlock *BB,
                                    slpvectorizer::BoUpSLP &R,
                                    bool AtTerminator);
 
   /// Scan the basic block and look for patterns that are likely to start
   /// a vectorization chain.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

if (auto *I1 = dyn_cast<Instruction>(Op1))
         if (S.getOpcode())
           continue;
         return false;
       }
   }
   return IsCompatibility;
 }
 
-bool SLPVectorizerPass::vectorizeSimpleInstructions(InstSetVector &Instructions,
-                                                    BasicBlock *BB, BoUpSLP &R,
-                                                    bool AtTerminator) {
-  bool OpsChanged = false;
-  SmallVector<Instruction *, 4> PostponedCmps;
-  SmallVector<WeakTrackingVH> PostponedInsts;
-  // pass1 - try to vectorize reductions only
-  for (auto *I : reverse(Instructions)) {
-    if (R.isDeleted(I))
-      continue;
-    if (isa<CmpInst>(I)) {
-      PostponedCmps.push_back(I);
-      continue;
-    }
-    OpsChanged |= vectorizeHorReduction(nullptr, I, BB, R, TTI, PostponedInsts);
-  }
-  // pass2 - try to match and vectorize a buildvector sequence.
-  for (auto *I : reverse(Instructions)) {
-    if (R.isDeleted(I) || isa<CmpInst>(I))
-      continue;
-    if (auto *LastInsertValue = dyn_cast<InsertValueInst>(I)) {
-      OpsChanged |= vectorizeInsertValueInst(LastInsertValue, BB, R);
-    } else if (auto *LastInsertElem = dyn_cast<InsertElementInst>(I)) {
-      OpsChanged |= vectorizeInsertElementInst(LastInsertElem, BB, R);
-    }
-  }
-  // Now try to vectorize postponed instructions.
-  OpsChanged |= tryToVectorize(PostponedInsts, R);
-
-  if (AtTerminator) {
-    // Try to find reductions first.
-    for (Instruction *I : PostponedCmps) {
-      if (R.isDeleted(I))
-        continue;
-      for (Value *Op : I->operands())
-        if (auto *RootOp = dyn_cast<Instruction>(Op))
-          OpsChanged |= vectorizeRootInstruction(nullptr, RootOp, BB, R, TTI);
-    }
-    // Try to vectorize operands as vector bundles.
-    for (Instruction *I : PostponedCmps) {
-      if (R.isDeleted(I))
-        continue;
-      OpsChanged |= tryToVectorize(I, R);
-    }
-    // Try to vectorize list of compares.
-    // Sort by type, compare predicate, etc.
-    auto CompareSorter = [&](Value *V, Value *V2) {
-      return compareCmp<false>(V, V2, *TLI,
-                               [&R](Instruction *I) { return R.isDeleted(I); });
-    };
+bool SLPVectorizerPass::vectorizeCmpInsts(
+    const SmallVector<CmpInst *, 4> &CmpInsts, BasicBlock *BB, BoUpSLP &R) {
+  bool Changed = false;
+  for (Instruction *I : CmpInsts) {
+        Changed |= vectorizeRootInstruction(nullptr, RootOp, BB, R, TTI);
+  for (Instruction *I : CmpInsts) {
+    Changed |= tryToVectorize(I, R);
 
-    auto AreCompatibleCompares = [&](Value *V1, Value *V2) {
-      if (V1 == V2)
-        return true;
-      return compareCmp<true>(V1, V2, *TLI,
-                              [&R](Instruction *I) { return R.isDeleted(I); });
-    };
 
-    SmallVector<Value *> Vals(PostponedCmps.begin(), PostponedCmps.end());
-    OpsChanged |= tryToVectorizeSequence<Value>(
-        Vals, CompareSorter, AreCompatibleCompares,
-        [this, &R](ArrayRef<Value *> Candidates, bool LimitForRegisterSize) {
-          // Exclude possible reductions from other blocks.
-          bool ArePossiblyReducedInOtherBlock =
-              any_of(Candidates, [](Value *V) {
-                return any_of(V->users(), [V](User *U) {
-                  return isa<SelectInst>(U) &&
-                         cast<SelectInst>(U)->getParent() !=
+  SmallVector<Value *> Vals(CmpInsts.begin(), CmpInsts.end());
+  Changed |= tryToVectorizeSequence<Value>(
+        bool ArePossiblyReducedInOtherBlock = any_of(Candidates, [](Value *V) {
+            return isa<SelectInst>(U) && cast<SelectInst>(U)->getParent() !=
                                              cast<Instruction>(V)->getParent();
-                });
-              });
-          if (ArePossiblyReducedInOtherBlock)
-            return false;
-          return tryToVectorizeList(Candidates, R, LimitForRegisterSize);
-        },
-        /*LimitForRegisterSize=*/true, R);
+  return Changed;
+}
+
+bool SLPVectorizerPass::vectorizeSimpleInstructions(InstSetVector &Instructions,
+                                                    BasicBlock *BB, BoUpSLP &R,
+                                                    bool AtTerminator) {
+  assert(all_of(Instructions,
+                [](auto *I) {
+                  return isa<CmpInst, InsertElementInst, InsertValueInst>(I);
+                }) &&
+         "This function only accepts Cmp and Insert instructions");
+  bool OpsChanged = false;
+  SmallVector<CmpInst *, 4> PostponedCmps;
+  SmallVector<WeakTrackingVH> PostponedInsts;
+  // pass1 - try to vectorize reductions only
+  for (auto *I : reverse(Instructions)) {
+    if (R.isDeleted(I))
+      continue;
+    if (isa<CmpInst>(I)) {
+      PostponedCmps.push_back(cast<CmpInst>(I));
+      continue;
+    }
+    OpsChanged |= vectorizeHorReduction(nullptr, I, BB, R, TTI, PostponedInsts);
+  }
+  // pass2 - try to match and vectorize a buildvector sequence.
+  for (auto *I : reverse(Instructions)) {
+    if (R.isDeleted(I) || isa<CmpInst>(I))
+      continue;
+    if (auto *LastInsertValue = dyn_cast<InsertValueInst>(I)) {
+      OpsChanged |= vectorizeInsertValueInst(LastInsertValue, BB, R);
+    } else if (auto *LastInsertElem = dyn_cast<InsertElementInst>(I)) {
+      OpsChanged |= vectorizeInsertElementInst(LastInsertElem, BB, R);
+    }
+  }
+  // Now try to vectorize postponed instructions.
+  OpsChanged |= tryToVectorize(PostponedInsts, R);
+
+  if (AtTerminator) {
+    OpsChanged |= vectorizeCmpInsts(PostponedCmps, BB, R);
     Instructions.clear();
   } else {
     Instructions.clear();
     // Insert in reverse order since the PostponedCmps vector was filled in
     // reverse order.
     Instructions.insert(PostponedCmps.rbegin(), PostponedCmps.rend());
   }
   return OpsChanged;