[PATCH] D151736: [RISCV] Lower inserts into identity shuffles as v{,f}slide1ups

[Luke Lau via Phabricator via llvm-commits]

luke added a comment.

In D151736#4381994 <https://reviews.llvm.org/D151736#4381994>, @reames wrote:

> Did you look at the alternative of adding a DAG combine to reverse the shuffle+insert form into the insert+shuffle form?  That'd been what I'd been tentatively thinking about, but I hadn't explored it past idea phase.

Not yet, but I'd like to explore that option before returning to this patch. This combine feels very specific and doesn't seem to be catch any cases in the benchmarks I've been looking at.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:6189
+        ArrayRef<int> ShufMask = Shuffle->getMask();
+        if (ShuffleVectorInst::isIdentityMask(ShufMask.drop_front(1)) &&
+            VecVT.isFixedLengthVector() && VecVT.getVectorNumElements() > 1) {
----------------
frasercrmck wrote:
> This function seems to assert that the mask is not empty. Is it all possible we could end up triggering this assert with one-element vectors? Would we better be moving the `VecVT.getVectorNumElements() > 1` check before calling `isIdentityMask`?
> 
> Could we try and add testing for the above case?
Good catch, definitely meant to move the`VecVT.getVectorNumElements() > 1` first. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151736