[PATCH] D136640: [RISCV][DAG] vslideup/vslidedown with zero offset and undef pass through is a nop

Wed Oct 26 09:32:00 PDT 2022

craig.topper added inline comments.

================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9888
+  case RISCVISD::VSLIDEUP_VL:
+    // vslidedown.vi undef, src, 0 -> src
+    // vslideup.vi   undef, src, 0 -> src
----------------
reames wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > Is this something we should squash in lowering of insert_vector_elt instead?
> > These all seem to be coming from RV32 insert_vector_elt of i64. If the index is 0 we can use two tail undisturbed slide1ups with VL=2 if the vector to insert into isn't undef. If it is undef we just the slide1ups.
> So, I agree we could do this via lowering.  Your D136738 does exactly that.  What I'm not clear on is why you prefer special casing this in lowering, as opposed to having a generic combine which could catch other - currently unknown - cases as well.  This code actually looks simpler to me than the version you had, and more general.  What's your concern with this approach?  
D136738 is mostly about improving the non-undef case. I had to special case undef between the two slide1downs to keep the second vslide1down from being TU in the undef case. I could check for 0 index and non-undef instead and let the old code run for the 0 index undef case. Then use this DAGCombine to fix the vslideup.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136640