[PATCH] D115750: [SLP]Further improvement of the cost model for scalars used in buildvectors.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6216
+    function_ref<std::pair<T *, bool>(T *, ArrayRef<int>)> ResizeAction,
+    function_ref<T *(ArrayRef<int>, ArrayRef<T *>)> Action) {
+  assert(!ShuffleMask.empty() && "Empty list of shuffles for inserts.");
----------------
RKSimon wrote:
> Do you intend to use performExtractsShuffleAction more than once in the future? Otherwise some of these function_ref seem superfluous.
Yes, this patch is part of D107966, which unifies cost estimation and code emission.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6363
                 do {
-                  int Idx = E->findLaneForValue(Base);
-                  ShuffleMask.back()[Idx] = Idx;
+                  int Idx = E->findLaneForValue(IEBase);
+                  SmallVectorImpl<int> &Mask = ShuffleMasks.back()[ScalarTE];
----------------
RKSimon wrote:
> IEBase is invariant to the inner do-while loop - should this still use Base?
I'll check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115750