[PATCH] D148378: [SLP][NFC] Remove comparator argument of `tryToVectorizeSequence()`

[Vasileios Porpodas via Phabricator via llvm-commits]

vporpo created this revision.
vporpo added reviewers: ABataev, vdmitrie, RKSimon, dmgreen.
Herald added a subscriber: hiraditya.
Herald added a project: All.
vporpo requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead.
Herald added a project: LLVM.

As far as I can tell there is no good reason why we are defining the comparator
before calling `tryToVectorizeSequence()` but running sort() inside
`tryToVectorizeSequence()`.
So this patch simplifies the code by moving the call to sort() closer to its
comparator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148378

Files:
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp


Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -14133,16 +14133,12 @@
 }
 
 template <typename T>
-static bool
-tryToVectorizeSequence(SmallVectorImpl<T *> &Incoming,
-                       function_ref<unsigned(T *)> Limit,
-                       function_ref<bool(T *, T *)> Comparator,
-                       function_ref<bool(T *, T *)> AreCompatible,
-                       function_ref<bool(ArrayRef<T *>, bool)> TryToVectorizeHelper,
-                       bool LimitForRegisterSize) {
+static bool tryToVectorizeSequence(
+    SmallVectorImpl<T *> &Incoming, function_ref<unsigned(T *)> Limit,
+    function_ref<bool(T *, T *)> AreCompatible,
+    function_ref<bool(ArrayRef<T *>, bool)> TryToVectorizeHelper,
+    bool LimitForRegisterSize) {
   bool Changed = false;
-  // Sort by type, parent, operands.
-  stable_sort(Incoming, Comparator);
 
   // Try to vectorize elements base on their type.
   SmallVector<T *> Candidates;
@@ -14320,8 +14316,10 @@
     };
 
     SmallVector<Value *> Vals(PostponedCmps.begin(), PostponedCmps.end());
+    stable_sort(Vals, CompareSorter);
+
     OpsChanged |= tryToVectorizeSequence<Value>(
-        Vals, Limit, CompareSorter, AreCompatibleCompares,
+        Vals, Limit, AreCompatibleCompares,
         [this, &R](ArrayRef<Value *> Candidates, bool LimitForRegisterSize) {
           // Exclude possible reductions from other blocks.
           bool ArePossiblyReducedInOtherBlock =
@@ -14483,8 +14481,10 @@
       }
     }
 
+    stable_sort(Incoming, PHICompare);
+
     HaveVectorizedPhiNodes = tryToVectorizeSequence<Value>(
-        Incoming, Limit, PHICompare, AreCompatiblePHIs,
+        Incoming, Limit, AreCompatiblePHIs,
         [this, &R](ArrayRef<Value *> Candidates, bool LimitForRegisterSize) {
           return tryToVectorizeList(Candidates, R, LimitForRegisterSize);
         },
@@ -14780,8 +14780,10 @@
     if (!isValidElementType(Pair.second.front()->getValueOperand()->getType()))
       continue;
 
+    stable_sort(Pair.second, StoreSorter);
+
     Changed |= tryToVectorizeSequence<StoreInst>(
-        Pair.second, Limit, StoreSorter, AreCompatibleStores,
+        Pair.second, Limit, AreCompatibleStores,
         [this, &R](ArrayRef<StoreInst *> Candidates, bool) {
           return vectorizeStores(Candidates, R);
         },


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148378.513742.patch
Type: text/x-patch
Size: 2496 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230414/7db4c72e/attachment-0001.bin>