[PATCH] D102498: [AArch64][SVE] Improve codegen for fixed length vector concat

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14568
 
+static SDValue performSpliceCombine(SDNode *N, SelectionDAG &DAG) {
+  // splice(pg, op1, undef) -> op1
----------------
Perhaps paranoia but is it worth adding `assert(N->getOpcode() == AArch64ISD::SPLICE && "Unexepected Opcode!");`?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17694
 
+SDValue AArch64TargetLowering::LowerFixedLengthConcatVectorsToSVE(
+    SDValue Op, SelectionDAG &DAG) const {
----------------
junparser wrote:
> bsmith wrote:
> > paulwalker-arm wrote:
> > > junparser wrote:
> > > > bsmith wrote:
> > > > > junparser wrote:
> > > > > > Can we still use ConcatVector here and then lower it performConcatVectorsCombine or td file?
> > > > > This is consistent with how all other fixed length operations are handled. The issue with trying to do this in tablegen, is that we'd have to add patterns for every single <= 2048-bit vector type, which is just going to cause an explosion of patterns.
> > > > Still we can keep ConcatVector here, and lowering to SPLICE in performConcatVectorsCombine at later phase. 
> > > @junparser This wouldn't follow the design we have for SVE's fixed-length code generation support.  Our intent is to exit operation legalisation with all fixed-length operations replaced with scalable vector equivalents.
> > @junparser If what you meant here is that we could lower to a scalable CONCAT_VECTOR and then later lower/match that to a splice instruction, that doesn't work well either as we won't have a sensible way to get at the fixed sizes being used which are required to produce the correct predicate for the instruction, hence making it very difficult to later lower/match to a splice.
> make sense to me. Meanwhile, is there any plan to improve such issue? @paulwalker-arm 
@junparser It's not clear to me what issue needs solving here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102498