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

Matthew Simpson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 1 10:33:34 PDT 2018


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





More information about the llvm-commits mailing list