[PATCH] D92760: [SelectionDAG] Implement SplitVecOp_INSERT_SUBVECTOR

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

A few stylistic things to consider (I'm only really bothered about loosing the v32 tests) but otherwise looks good.



================
Comment at: llvm/test/CodeGen/AArch64/split-vector-insert.ll:1
+; RUN: llc < %s -o - -mtriple=aarch64-- -mcpu=a64fx -debug-only=legalize-types 2>&1 | FileCheck %s --check-prefix=CHECK-LEGALIZATION
+; RUN: llc < %s -o - -mtriple=aarch64-- -mcpu=a64fx | FileCheck %s --check-prefix=CHECK-CODEGEN
----------------
It's up to you but I prefer to keep RUN lines minimal so: 

`-o -` is not needed as that is the default when redirecting files into llc,
`-mtriple=aarch64--` can be replaced with `target triple = "aarch64-unknown-linux-gnu"`
`-mcpu=a64fx` can be replaced with a function attribute `attributes #0 = { "target-features"="+sve" }` remembering to reference #0 in the function definitions.


================
Comment at: llvm/test/CodeGen/AArch64/split-vector-insert.ll:2
+; RUN: llc < %s -o - -mtriple=aarch64-- -mcpu=a64fx -debug-only=legalize-types 2>&1 | FileCheck %s --check-prefix=CHECK-LEGALIZATION
+; RUN: llc < %s -o - -mtriple=aarch64-- -mcpu=a64fx | FileCheck %s --check-prefix=CHECK-CODEGEN
+; REQUIRES: asserts
----------------
Again your choice, but you could drop this then just use CHECK for the code generation validation, which is more in keeping with the majority of the code generation tests.


================
Comment at: llvm/test/CodeGen/AArch64/split-vector-insert.ll:116
+
+define <vscale x 2 x i64> @test_nxv2i64_v32i64(<vscale x 2 x i64> %a, <32 x i64> %b) {
+; CHECK-LEGALIZATION: Legally typed node: [[T1:t[0-9]+]]: nxv2i64 = insert_subvector {{t[0-9]+}}, {{t[0-9]+}}, Constant:i64<0>
----------------
I don't believe this and the other v32 test offers any value.  The v8 tests are already testing nested (two levels) type legalisation, so there's no reason to test it again at four levels.  Add this to the fact the generated code it pretty unreadable I recommend removing them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92760