[PATCH] D24683: [DAGCombine] Generalize build_vector -> vector_shuffle combine for more than 2 inputs

[Michael Kuperstein via llvm-commits]

mkuper added a comment.

Thanks, Elena!


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12883
@@ -12879,14 +12882,3 @@
 
-// If Vec holds a reference to a non-null node, return Vec.
-// Otherwise, return either a zero or an undef node of the appropriate type.
-static SDValue getRightHandValue(SelectionDAG &DAG, SDLoc DL, SDValue Vec,
-                                 EVT VT, bool Zero) {
-  if (Vec.getNode())
-    return Vec;
-
-  if (Zero)
-    return VT.isInteger() ? DAG.getConstant(0, DL, VT)
-                          : DAG.getConstantFP(0.0, DL, VT);
-
-  return DAG.getUNDEF(VT);
-}
+SDValue DAGCombiner::createBuildVecShuffle(
+    SDLoc DL, SDNode *N, const SmallVectorImpl<int> &VectorMask, SDValue VecIn1,
----------------
delena wrote:
> Please add a comment here.
Sure.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13030
@@ +13029,3 @@
+    // All inputs must have the same element type as the output.
+    if (VT.getVectorElementType() !=
+        ExtractedFromVec.getValueType().getVectorElementType())
----------------
delena wrote:
> I suppose you can put "assert" here.
I guess the comment is ambiguous - by "must" I didn't mean that we never get here if that doesn't happen, but that it's required for the combine. I'll change the comment.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13079
@@ +13078,3 @@
+    else
+      return SDValue();
+  }
----------------
delena wrote:
> In what case it fails to create a shuffle?
If the number of elements in the vectors is too different, see line 12938.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13084
@@ +13083,3 @@
+  // to the list of shuffles.
+  if (UsesZeroVector)
+    Shuffles.push_back(VT.isInteger() ? DAG.getConstant(0, dl, VT)
----------------
delena wrote:
> If you fail in shuffle creation, to you want to try zero-vector?
I don't think so. 
The only reason it'll fail is if at least one of the input vectors has the wrong size. If this happens, it doesn't really matter what the other input is - there's no advantage to blending with zero, as compared to shuffling with a vector which matches the output size.


https://reviews.llvm.org/D24683