This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Form vmv.s.f/x from single element splats via DAG combine
ClosedPublic

Authored by reames on Aug 25 2023, 12:02 PM.

Details

Summary

This re-implements the special casing we had in lowerScalarSplat as a DAG combine. As can be seen in the tests, this ends up triggering in a bunch more cases.

The semantically interesting bit of this change is the use of the implicit truncate semantics for when XLEN > SEW. We'd already been doing this for vmv.v.x, but this change extends e.g. the constant matching to make the same assumption about vmv.s.x. Per my reading of the specification, this should be fine, and if anything, is more obviously true of vmv.s.x than vmv.v.x.

Note that this basically ends up being two changes: one for FP and one for integer. If useful for reviewers, I'm happy to split.

Diff Detail

Event Timeline

reames created this revision.Aug 25 2023, 12:02 PM
reames requested review of this revision.Aug 25 2023, 12:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 25 2023, 12:02 PM
luke accepted this revision.Aug 29 2023, 9:13 AM

LGTM

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2977

Looks like this was also factored out in https://reviews.llvm.org/D158741, so rebasing this should just be a matter of handling vmv_s_x_vl in findVSplat there

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

I had a think about how vmv.s.x only writes to one register and not the register group, but couldn't see any issues that would arise with it and this combine.

This revision is now accepted and ready to land.Aug 29 2023, 9:13 AM
craig.topper added inline comments.Aug 29 2023, 2:54 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
13966

I don't think we need "only" twice in the second sentence

13970

Const->isZero()

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

Why do we check the vl for riscv_vfmv_s_f_vl, but we don't check the VL for RISCVISD::VMV_S_X_VL in selectVSplat?

reames added inline comments.Aug 30 2023, 12:35 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
592

It doesn't matter either way with the undef pass thru. I'll switch to using srcvalue here, but it causes no test delta.

This revision was landed with ongoing or failed builds.Aug 30 2023, 12:45 PM
This revision was automatically updated to reflect the committed changes.