[PATCH] D57779: [SLP] Add support for throttling.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:122-123
 static cl::opt<bool>
-ShouldVectorizeHor("slp-vectorize-hor", cl::init(true), cl::Hidden,
-                   cl::desc("Attempt to vectorize horizontal reductions"));
+    ShouldVectorizeHor("slp-vectorize-hor", cl::init(true), cl::Hidden,
+                       cl::desc("Attempt to vectorize horizontal reductions"));
+
----------------
Tabs are added


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2288-2295
+    /// Contains orders of operations along with the number of bundles that have
+    /// operations in this order. It stores only those orders that require
+    /// reordering, if reordering is not required it is counted using \a
+    /// NumOpsWantToKeepOriginalOrder.
+    DenseMap<OrdersType, unsigned, OrdersTypeDenseMapInfo>
+        NumOpsWantToKeepOrder;
+    /// Number of bundles that do not require reordering.
----------------
Tabs


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3297-3300
+    // For all canceled operations we should consider the possibility of
+    // use by with non-canceled operations and for that, it requires
+    // to populate ExternalUser list with canceled elements.
+    for (int Lane = 0, LE = Entry->Scalars.size(); Lane != LE; ++Lane) {
----------------
Should this loop be executed only for `ProposedToGather` `Entry`s?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3316-3319
+    }
+  }
+  // Canceling unprofitable elements.
+  for (std::unique_ptr<TreeEntry> &TEPtr : Tree->VectorizableTree) {
----------------
I actually don't see propagation for `ProposedTogather` and these loops can be merged, no?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4167-4170
+                     [TE](const std::unique_ptr<TreeEntry> &EntryPtr) {
+                       return EntryPtr->State != TreeEntry::Vectorize &&
+                              EntryPtr->isSame(TE.Scalars);
+                     }))
----------------
Tabs


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4184
+      if (TE->State != TreeEntry::Vectorize)
+        continue;
+      int GatherCost = 0;
----------------
Tab


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5133
+  // Erase all saved trees after vectorization, except current.
+  BuiltTrees.erase(llvm::remove_if(BuiltTrees,
+                                   [this](std::unique_ptr<TreeState> &T) {
----------------
Use `llvm::erase_if`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57779/new/

https://reviews.llvm.org/D57779