[PATCH] D151468: [RISCV] Use v(f)slide1up for shuffle+insert idiom

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D151468#4375309 <https://reviews.llvm.org/D151468#4375309>, @frasercrmck wrote:

> All of the tests are for two-element vectors. Is it worth adding further testing?

Actually, there were test for 4 x E vectors, I'd just gotten the shuffle mask wrong on every single one of them.  :(  Fixed, and rebased, so the tests should make a lot more sense now.

> I played around with some other examples; should all of these also be lowered to this pattern?
>
>   ; This one matches
>   %vb = insertelement <4 x i8> poison, i8 %b, i64 0
>   %v1 = shufflevector <4 x i8> %vb, <4 x i8> poison, <4 x i32> zeroinitializer
>   %v2 = shufflevector <4 x i8> %v1, <4 x i8> %v, <4 x i32> <i32 0, i32 4, i32 5, i32 6>
>
>   ; I wouldn't have thought the splat index should matter
>   %vb = insertelement <4 x i8> poison, i8 %b, i64 0
>   %v1 = shufflevector <4 x i8> %vb, <4 x i8> poison, <4 x i32> zeroinitializer
>   %v2 = shufflevector <4 x i8> %v1, <4 x i8> %v, <4 x i32> <i32 1, i32 4, i32 5, i32 6>

This is an artifact of where I put the matching code.  This isn't a sub-vector insert, and I'd placed the vslide1up match below that check.  Good catch.  If you're okay with it, I'd like to land this as is, and then handle this in a separate patch.  I need to write dedicated matching for the vslide1down case anyways, and I'd like to have the test coverage in tree already.

>   ; Should be the same as the first example, but doesn't match
>   %vb = insertelement <4 x i8> poison, i8 %b, i64 0
>   %v2 = shufflevector <4 x i8> %vb, <4 x i8> %v, <4 x i32> <i32 0, i32 4, i32 5, i32 6>

This looks identical to the new vslide1up_4xi8_swapped test.  It does match on RV64, are you maybe testing this on RV32?  There's something a bit odd going on with that one; we're widening the op type on the insert.  I have not looked into that one yet.


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

https://reviews.llvm.org/D151468