This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by reames on Oct 24 2022, 2:33 PM.

Details

Summary

If we have a vslideup or vslidedown with a zero offset, this is equivalent to copying the active elements into the destination. If the destination is undefined, then we can ignore masking and VL predication and simply return the source operand. This may cause additional lanes to be defined, but is otherwise a nop.

Not thought to be important; noticed while glancing at a test diff for something else, and figured I'd knock it out.

Diff Detail

Event Timeline

reames created this revision.Oct 24 2022, 2:33 PM
reames requested review of this revision.Oct 24 2022, 2:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2022, 2:33 PM
craig.topper added inline comments.Oct 25 2022, 2:03 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9888

Is this something we should squash in lowering of insert_vector_elt instead?

craig.topper added inline comments.Oct 25 2022, 2:09 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9888

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.

reames added inline comments.Oct 26 2022, 8:56 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9888

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?

craig.topper added inline comments.Oct 26 2022, 9:31 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9888

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.

reames abandoned this revision.Oct 28 2022, 8:59 AM

Defer to craig on approach.