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

[Joe Ellis via Phabricator via llvm-commits]

joechrisellis added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17700
+  EVT VT = Op.getValueType();
+  EVT SrcVT = SrcOp1.getValueType();
+
----------------
nit: no harm in having `SrcOp1VT` and `SrcOp2VT`, I guess -- I know `SrcOp2VT` would only be used in the assertion but it seems reasonable to have both I'd say. 🙂 


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17702
+
+  assert(SrcVT == SrcOp2.getValueType());
+
----------------
Can we add a message to the assertion?


================
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();
----------------
Just a question -- can we have `CONCAT_VECTOR` with a single operand? Doesn't seem like it. 🤔 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102498