[PATCH] [DAGCombiner] Combine shuffles of BUILD_VECTOR and SCALAR_TO_VECTOR

[Simon Pilgrim]

Thanks for the review Hal, I'll update the patch later.

In http://reviews.llvm.org/D8516#145467, @hfinkel wrote:

> > At the moment the inputs are only combined if they are only being used once - I'm interested in extending this so that constant inputs are always combined. It would create more constant data but would remove more shuffles (which may be introducing their own constant data for masks anyhow). Comments please.
>
>
> We need to be careful here. Constant loads can be much more expensive than shuffles on some targets.


Yes that was my concern as well - there isn't an easy way to determine how expensive the shuffle is that we're trying to remove. I'll leave it as it is for now and only combine when the inputs.are only used once.


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12029
@@ +12028,3 @@
+    }
+    if (Ops.size() == VT.getVectorNumElements()) {
+      // Create SCALAR_TO_VECTOR if the only defined input is input[0].
----------------
hfinkel wrote:
> When would this be false?
We have an early out from the loop if an active operand turns out to be something other than BUILD_VECTOR or SCALAR_TO_VECTOR. I'll move this logic to the opening if() entry into the block.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12042
@@ +12041,3 @@
+        for (SDValue &Op : Ops)
+          Op = DAG.getSExtOrTrunc(Op, SDLoc(N), SVT);
+      return DAG.getNode(ISD::BUILD_VECTOR, SDLoc(N), VT, Ops);
----------------
hfinkel wrote:
> It might be better to use ZExt, depending on the target/type. Might be better to use something like this:
>   Op = isZExtFree(Op.getValueType(), SVT) ? DAG.getZExtOrTrunc(Op, SDLoc(N), SVT) : DAG.getSExtOrTrunc(Op, SDLoc(N), SVT);
Easy enough to add.

http://reviews.llvm.org/D8516

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/