[PATCH] D46126: [SLP] Vectorize transposable binary operand bundles

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1484
+  SmallVector<BoUpSLP::ValueList, 4> Bundles;
+  for (auto &Entry : Opcode2Operands) {
+    ArrayRef<Value *> Bundle(Entry.second);
----------------
ABataev wrote:
> `auto &`->`const BoUpSLP::ValueList &` or, probably, you may use `ArrayRef<Value*` here
Using `const std::pair<unsigned, BoUpSLP::ValueList> &`


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2478
+      unsigned TransposeShuffleCost = 0;
+      SmallVector<const Value *, 4> Operands;
+      if (auto TransposedBundles = transposeBinOpBundle(E->Scalars)) {
----------------
ABataev wrote:
> Pre-reserve the space for the Operands here.
I'm not sure we can do this. For the transpose case, since we don't know what the operands are (they will be shuffles), I've left `Operands` empty. If we instead pre-reserve space, we'll end up sending a vector of null pointers to `getArithmeticInstrCost`, which will probably break.


Repository:
  rL LLVM

https://reviews.llvm.org/D46126