This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Lower VECTOR_SPLICE to RVV instructions.
ClosedPublic

Authored by craig.topper on Feb 8 2022, 4:35 PM.

Details

Summary

This lowers VECTOR_SPLICE of scalable vectors to a slidedown follow by a slideup.
Fixed vectors are encouraged to use shufflevector instruction. The equivalent patch
for fixed vectors is D119039.

I've used a tail agnostic slidedown and limited the VL to only the
elements that will not be overwritten by the slideup. The slideup
uses VLMax for its VL. It unfortunately uses tail undisturbed policy
but it isn't required as there is no tail. We just need the merge
operand to carry the bits for the lower portion of the result.

Care was taken to ensure that either the slideup or slidedown will
be able to use a .vi instruction when the immediate is small. Which
one uses the immediate depends on the sign of the immediate.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 8 2022, 4:35 PM
craig.topper requested review of this revision.Feb 8 2022, 4:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 8 2022, 4:35 PM
This revision is now accepted and ready to land.Feb 9 2022, 8:21 AM

A few nits. I think the commit message/description could be more explicit that this is only for scalable-vector splices. Fixed-length vectors are technically supported by the intrinsic, even if they're not recommended.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
5650

This comment seems not to reflect the code on the following lines.

llvm/test/CodeGen/RISCV/rvv/vector-splice.ll
5

Do they?

frasercrmck added inline comments.Feb 9 2022, 9:45 AM
llvm/test/CodeGen/RISCV/rvv/vector-splice.ll
2034

Oh I just saw this, which probably partially answers my comment above. Why do we need this? The RVV-specific lowering code doesn't seem to care. Is there something happening at a higher level?

craig.topper added inline comments.Feb 9 2022, 10:48 AM
llvm/test/CodeGen/RISCV/rvv/vector-splice.ll
5

There's a verifier check on the range of the immediate. Without a vscale_range the only allowed constants for <vscale x 1 x *> are 0 and -1. So I increased it to give more options.

craig.topper edited the summary of this revision. (Show Details)Feb 9 2022, 10:50 AM
craig.topper added inline comments.Feb 9 2022, 10:56 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
5650

Oops that was written in an earlier iteration of the Offset calculations. At one point I created (VLMax - (VLMax + Imm)) for the negative immediate case and DAGCombiner wouldn't simplify it to -Imm so I did it myself.

Move comment.

craig.topper requested review of this revision.Feb 9 2022, 12:03 PM

Moving back to Request Review to give Fraser a chance to look again

frasercrmck accepted this revision.Mar 1 2022, 7:23 AM

LGTM, sorry for the delay.

llvm/test/CodeGen/RISCV/rvv/vector-splice.ll
5

Ah I see, thanks, that wasn't obvious.

This revision is now accepted and ready to land.Mar 1 2022, 7:23 AM
This revision was landed with ongoing or failed builds.Mar 1 2022, 10:14 AM
This revision was automatically updated to reflect the committed changes.