[PATCH] D47775: [AArch64][SVE] Add SPLAT_VECTOR ISD Node

[Graham Hunter via Phabricator via llvm-commits]

huntergr marked an inline comment as done.
huntergr added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3544
 
+  if (MaskV->isNullValue() && VT.isScalableVector()) {
+    // Canonical splat form of first element of first input vector.
----------------
jmolloy wrote:
> It feels to me like this is a canonicalization that should be a DAGCombine rather than in the builder. Is this here specifically because the ISD::vector_shuffle is not representable with scalable vectors?
> 
> If not, I'd prefer this be moved to a canonical dag combine. In general, I feel logic here should be at a minimum because it runs before targets can override or pattern match themselves.
It's because `BUILD_VECTOR` cannot be used for scalable vectors, and if we tried to use `VECTOR_SHUFFLE`, we'd still need to generate a mask with an unknown number of elements which we can't do at the moment.

I've limited it to scalable vectors only for now precisely because this does break some unit tests if I enable it for all targets -- they do get a splat, but not the expected splat. It may be better to move this code to `SelectionDAG::getVectorShuffle` near the code that checks `TLI->hasVectorBlend()`, which also seems to match and transform splats early on.

We do have a `VECTOR_SHUFFLE_VAR` node downstream which takes a third argument as a mask, and with nodes for vscale and stepvector we're able to generate the masks we need -- but that's for a separate discussion and patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47775/new/

https://reviews.llvm.org/D47775