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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Anything we can do to simplify this patch would be great - there's a lot going on.



================
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.");
----------------
Do you intend to use performExtractsShuffleAction more than once in the future? Otherwise some of these function_ref seem superfluous.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6360
               // Build the mask for the vectorized insertelement instructions.
-              if (const TreeEntry *E = getTreeEntry(Base)) {
-                VU = cast<InsertElementInst>(Base);
+              if (const TreeEntry *E = getTreeEntry(IEBase)) {
+                VU = IEBase;
----------------
Some of this is NFC - pre-commit to reduce the patch?


================
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];
----------------
IEBase is invariant to the inner do-while loop - should this still use Base?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6382
           }
-          ShuffleMask[VecId][*InsertIdx] = EU.Lane;
-          DemandedElts[VecId].setBit(*InsertIdx);
+          int Idx = *InsertIdx;
+          SmallVectorImpl<int> &Mask = ShuffleMasks[VecId][ScalarTE];
----------------
Pulling out Idx is just a NFC - precommit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115750