[PATCH] D119039: [RISCV] Lower the shufflevector equivalent of vector.splice

[Zakk Chen via Phabricator via llvm-commits]

khchen added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2577
+  if (!Lo || !Hi)
+    return -1;
+
----------------
craig.topper wrote:
> khchen wrote:
> > I'm wondering is there any cases which have only one value for valves?
> > I don't get it because X86 did the assignment if one of values not found but we return -1 here.
> It would happen for 
> 
> <1, 2, 3, 4, 5, 6, 7, -1> or <-1, 0, 1, 2, 3, 4, 5, 6>
> 
> Those cases should be handled as a single slidedown or slideup. We already have matchShuffleAsSlideDown for the slidedown case, we don't have matchShuffleAsSlideUp yet.
> 
> X86 has a single instruction with two vector operands. I'm not sure why they wanted to use the same input twice instead of making the unused one undef.
`-1` is invalid shufflevector operands showed in my local machine, 
I tried to give `undef` (ex. <1, 2, 3, undef> and changed this piece of code same with x86,
The result seems like correct.

Why I did that because I was thinking is it possible to have an unify function for both x86 and risc-v because they are almost same except here and assert checking.

anyway, it could be another RFC patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119039