This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Use vslide1down idiom for generic build_vector
ClosedPublic

Authored by reames on Apr 26 2023, 9:21 AM.

Details

Summary

We had previously been going through the stack.

A couple small notes:

  • We have the vslide1down idiom in a few other places. As a post patch, I plan to try to common the code a bit.
  • VF=2 case is still going through the splat + insert path. Picking the optimal sequence for this seems to be a bit fiddly (due to constant mat costs), so I restricted this to cases which would have previously hit the stack.
  • I'm only handling integer vectors for the moment. Mostly because I don't see the existing vfslide1down ISD nodes being in place. This will be an obvious followup.
  • One of the test diffs does expose a missing combine - a build_vector with a prefix coming from a vector extract sequence. The code after this is arguably worse (due to domain crossing vs stack store), but I think this is a narrow enough case to be non-blocking for now. Let me know if you disagree.

Diff Detail

Event Timeline

reames created this revision.Apr 26 2023, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2023, 9:21 AM
reames requested review of this revision.Apr 26 2023, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2023, 9:21 AM
luke added inline comments.Apr 27 2023, 3:38 AM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-load.ll
84–87

This is interesting. The legalised DAG must be something like build_vector <a4, a3, a2, a1, a0, undef, undef, undef>.
Is there a potential combine we could do here to convert n nested (vslide1down.vx v, undef) into (vslidedown.vi v, n)?

vfslide1down ISD notes being in place

Was that supposed to be "nodes" instead of "notes"?

craig.topper added inline comments.Apr 28 2023, 11:00 AM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp2i-sat.ll
625

I might go investigate if we can avoid scalarizing this.

vfslide1down ISD notes being in place

Was that supposed to be "nodes" instead of "notes"?

Yep. Will update.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-load.ll
84–87

This needs a bit of care. The advantage of using the vslide1down idiom is that we can use the same VL throughout and thus avoid vsetvli toggles. We may be able to make something like this work, but it requires a bit of care.

Another idea in the same vein is to do a smaller build vector of only the undef prefix and then slide into result vector.

This particular case - with undef forming a suffix - could be handled by explicitly decreasing VL and only sliding in the non-undef prefix. This works out to be the same as the former idea.

reames edited the summary of this revision. (Show Details)May 1 2023, 7:30 AM

ping

This revision is now accepted and ready to land.May 1 2023, 9:47 AM
This revision was automatically updated to reflect the committed changes.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp