[PATCH] D92760: [SelectionDAG] Implement SplitVecOp_INSERT_SUBVECTOR

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2287-2291
+  if (ResVT.isFixedLengthVector() &&
+      N->getOperand(0).getValueType().isFixedLengthVector() &&
+      N->getOperand(1).getValueType().isScalableVector())
+    report_fatal_error("Inserting a scalable vector into a fixed-length vector "
+                       "is not yet supported. ");
----------------
Is this protection necessary?  From what I can see the code below should work for all valid forms of INSERT_SUBVECTOR. That's to say you don't need to worry about the case of inserting a scalable vector into a fixed length vector because that is not a valid use of INSERT_SUBVECTOR and thus should be caught before getting here.


================
Comment at: llvm/test/CodeGen/AArch64/split-vector-insert.ll:54
+  %castScalableSve = call <vscale x 2 x i64> @llvm.experimental.vector.insert.nxv2i64.v8i64(<vscale x 2 x i64> undef, <8 x i64> zeroinitializer, i64 0)
+  call void @_Z5svtblu12__SVUint64_tu12__SVUint64_t(<vscale x 2 x i64> undef, <vscale x 2 x i64> %castScalableSve)
+  ret void
----------------
Is this required? I'm wondering if simply passing the subvector as a function parameter (or loading it from memory) and returning the scalable result directly, leads to a simpler test. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92760