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

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

This is reminiscent of LV's interleave group optimization, in the sense that a couple of correlated inefficient vector "gathers" are replaced by a couple of efficiently formed vectors followed by transposing shuffles. The correlated gathers may come from the two operands of a binary operation, as in this patch, or more generally from arbitrary leaves of the SLP tree.



================
Comment at: lib/Analysis/VectorUtils.cpp:535
+  return ConstantVector::get(Mask);
+}
+
----------------
Could `createStrideMask` be (re)used?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1463
+    Opcode2Operands[VLIOp0->getOpcode()].push_back(VLIOp0);
+    Opcode2Operands[VLIOp0->getOpcode()].push_back(VLIOp1);
+  }
----------------
Both operands are mapped to the opcode of operand 0?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1521
+    unsigned ElemsPerDstVec = BundleVF * NumBundles / VF;
+    unsigned ElemsPerSrcVec = ElemsPerDstVec / 2;
+    Type *DstTy = VectorType::get(VecTy->getElementType(), ElemsPerDstVec);
----------------
May be easier to follow if `ElemsPerSrcVec` is initially set to `BundleVF` and doubled inside the loop, `ElemsPerDstVec` is set to its double, and `VF` renamed to something like `NumVectors`.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1973
+      // determine if we can transpose them, and if so, continue building the
+      // tree with the transposed bundles.
+      if (isa<BinaryOperator>(VL0))
----------------
Comment that transposing operands each having a common opcode is mutually exclusive with swapping commutative operands below, and should precede it?


Repository:
  rL LLVM

https://reviews.llvm.org/D46126