[PATCH] D105986: [SLP]Improve vectorization of gathered loads.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

A few minors



================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3053
+                     all_of(LI->users(),
+                            [this](User *U) { return getTreeEntry(U); }));
+           }))) {
----------------
Add comment explaining this logic


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3071
+                                  Roots.size()) *
+                         (NumElts >= 4 ? 1 : 2);
+          if (Diff && *Diff < MaxLoads) {
----------------
Can you clean this up? The clang-formating has made it difficult to follow.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3740
+            auto *CI = cast<ConstantInt>(Op);
+            Operands.push_back(ConstantExpr::getIntegerCast(
+                CI, Ty, CI->getValue().isSignBitSet()));
----------------
Could we end up in a situation where we end up truncating here and causing overflow problems later on?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5715
+            Builder.CreateShuffleVector(Entries.front()->VectorizedValue,
                                         Entries.back()->VectorizedValue, Mask);
     } else {
----------------
By using .back() can't we safely use this for the Entries.size() == 1 case as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105986