This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Lower vector shuffles to vrgather operations
ClosedPublic

Authored by frasercrmck on Apr 15 2021, 4:31 AM.

Details

Summary

This patch extends the lowering of RVV fixed-length vector shuffles to
avoid the default stack expansion and instead lower to vrgather
instructions.

For "permute"-style shuffles where one vector is swizzled, we can lower
to one vrgather. For shuffles involving two vector operands, we lower to
one unmasked vrgather (or splat, where appropriate) followed by a masked
vrgather which blends in the second half.

On occasion, when it's not possible to create a legal BUILD_VECTOR for
the indices, we use vrgatherei16 instructions with 16-bit index types.

For 8-bit element vectors where we may have indices over 255, we have a
fairly blunt fallback to the stack expansion to avoid custom-splitting
of the vector types.

To enable the selection of masked vrgather instructions, this patch
extends the various RISCVISD::VRGATHER nodes to take a passthru operand.

Diff Detail

Event Timeline

frasercrmck created this revision.Apr 15 2021, 4:31 AM
frasercrmck requested review of this revision.Apr 15 2021, 4:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2021, 4:31 AM

I wonder if we could take advantage of this "(vs1[i] >= VLMAX) ? 0 : vs2[vs1[i]]" to use an OR instead of a merge in some cases. If the index type has SEW=32 or SEW=64 surely a "-1" would be above VLMAX. If were to do that it should be a different patch.

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

This might be clearer as "IsSelect == SwapOps"

1607

or if max LMUL is capped.

llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
146

Should we change the names of these to include _MASK or something? This is now difference than the other _VL ISD opcodes and many of the others were set up to match the VP SDNodes.

Alternatively, we pattern match VSELECT_VL+VRGATHER_VX_VL/VRGATHER_VV_VL? I think that's how VP is intended to work.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-shuffles.ll
104

Depending on how costly a vrgather is, this doesn't look like much of an improvement.

It's also only using 1 element from vector 2. Would that be cheaper to just extract+insert into place?

frasercrmck marked 3 inline comments as done.Apr 16 2021, 1:44 AM
frasercrmck added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1572

Hah yep.

1607

good point, cheers.

llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
146

I was wondering about that. My original version added a separate MASK version. I think maybe it depends on what we want to do with the VP nodes as a whole. Since we haven't really answered that question (I'm planning on looking into VP support shortly) I didn't want this patch to be the bellweather for how we lower VP as we should be able to change our minds if another strategy proves better.

Perhaps the best idea for now is the VSELECT one. It doesn't change the behaviour of the existing nodes, doesn't introduce new ones, and may "just work" with our VP lowering strategy. I don't think it'd increase the patterns any more than I've already done with the extra non-truemask pats.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-shuffles.ll
104

Yeah there's definitely a couple of heuristics we can chuck at this to optimize certain cases. Shuffles are one of those things that's hard to get right first try.

As you say, one of them is having a sense of how many elements are used by each operand, much like we do in BUILD_VECTOR lowering. Single-element manipulation will be better in certain cases.

I think another could be to detect "almost splats"; where one operand is a full splat and the other is shuffled, or when either operand is almost splatted save for a handful of elements.

I'll be coming back to shuffles over the next wee while. Do you think this is something to do now or leave as a follow-up?

frasercrmck marked 3 inline comments as done.
  • address minor feedback
  • remove passthru from VRGATHER nodes; add vselect patterns in their place
craig.topper accepted this revision.Apr 16 2021, 9:17 AM

LGTM

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-shuffles.ll
104

It can be a follow up.

This revision is now accepted and ready to land.Apr 16 2021, 9:17 AM
This revision was automatically updated to reflect the committed changes.