[PATCH] D102498: [AArch64][SVE] Improve codegen for fixed length vector concat
Paul Walker via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 18 08:12:42 PDT 2021
paulwalker-arm added inline comments.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17709
+ EVT ContainerDstVT = getContainerForFixedLengthVector(DAG, VT);
+ EVT ContainerSrcVT = getContainerForFixedLengthVector(DAG, SrcVT);
+
----------------
I'm wondering if there needs to be this distinction or if we can just have `ContainerVT`. I say this because the container is based on the element type and since the `Src` and `Dst` element types for `CONCAT_VECTORS` must be the same, then the container type will also be the same.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17720-17722
+ // Skip the splice for the trivial case of concat with undef
+ if (SrcOp2.isUndef())
+ Op = SrcOp1;
----------------
This looks like something that can be extracted into a target specific dag combine (i.e. `SPLICE pg, op1, undef -> op1`) being a common scalable vector transform, and means the lowering code will be simpler. You'll see we do something similar for `AArch64ISD::UZP1`.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17708
+ // For now don't support concat of more than 2 vectors
+ if (Op->getNumOperands() != 2)
+ return SDValue();
----------------
bsmith wrote:
> joechrisellis wrote:
> > Just a question -- can we have `CONCAT_VECTOR` with a single operand? Doesn't seem like it. 🤔
> I don't believe so, no.
It is allowed for `CONCAT_VECTORS` to have a single operand, but this get's folded away during construction and so doesn't need special handling. That said, I do think it's better for the `getNumOperands()` check to be done earlier before extracting specific operands via `Op.getOperand(#num)`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102498/new/
https://reviews.llvm.org/D102498
More information about the llvm-commits
mailing list