This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Use vslidedown.vi vN, vN, 0 instead of vslideup.vi vN, vM, 0 for subvector insertion
AbandonedPublic

Authored by reames on Jun 6 2023, 11:59 AM.

Details

Summary

These instructions are semantically identical in the case where the offset is 0 with the exception that vslideup has a vector overlap constraint and vslidedown doesn't. As a result, we can prefer the one without register overlap constraints to improve register allocation flexibility.

This patch is specific to subvector insertion, but this pattern may show up elsewhere. We could possibly rework this as a DAG combine if reviewers thought that was worthwhile.

Diff Detail

Event Timeline

reames created this revision.Jun 6 2023, 11:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 11:59 AM
reames requested review of this revision.Jun 6 2023, 11:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 11:59 AM
luke added a comment.Jun 7 2023, 5:57 AM

Out of curiosity I tried it as a DAG combine here: https://reviews.llvm.org/D152368
It seems to catch a few more cases, but I'm not particularly opinionated about either approach.

craig.topper added inline comments.Jun 7 2023, 8:46 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7367

Should we do the same thing here? Would that catch the cases the patch from @luke gets?

craig.topper added inline comments.Jun 8 2023, 10:25 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7367

Posted that change as https://reviews.llvm.org/D152496

Should we be using vmv.v.v instead?

luke added a comment.Jun 9 2023, 8:35 AM

Should we be using vmv.v.v instead?

I noticed that these slides of zero are also equivalent to vmerge with an all ones mask and a tail undisturbed policy, so I tried combining to that to see what changed. The vmerges then got converted to vadd.v.i vd, vs, 0 in performCombineVMergeAndVOps. Should all these forms be canonicalised to vmv.v.v?