This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Improve i64 splat vector lowering in RV32.
ClosedPublic

Authored by jacquesguan on Jan 11 2022, 7:19 PM.

Details

Summary

We could use vmv.v.i/vmv.v.x whose eew is 32 to lower the i64 splat vector if the i64 constant scalar could be splitted into two same i32 scalar.

Diff Detail

Event Timeline

jacquesguan created this revision.Jan 11 2022, 7:19 PM
jacquesguan requested review of this revision.Jan 11 2022, 7:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 11 2022, 7:19 PM
craig.topper added inline comments.Jan 11 2022, 7:39 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2218

auto doesn't add anything here. It's more characters than MVT

2220

could -> can

Address comment.

jacquesguan marked 2 inline comments as done.Jan 12 2022, 12:32 AM
jacquesguan added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2218

Done

2220

Done

frasercrmck added inline comments.Jan 13 2022, 6:50 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2219

I may be missing something, but isn't VT.getVectorNumElements problematic with scalable vector types? I'm surprised this doesn't warn/crash.

Also I'm not sure whether this method should assume that VT is scalable. Should it be able to produce a fixed-vector type if passed one?

craig.topper added inline comments.Jan 13 2022, 8:46 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2219

Also I'm not sure whether this method should assume that VT is scalable. Should it be able to produce a fixed-vector type if passed one?

Assuming scalable is ok. Other parts of this code are creating _VL nodes which must use a scalable type.

2219

I may be missing something, but isn't VT.getVectorNumElements problematic with scalable vector types? I'm surprised this doesn't warn/crash.

It happens to work because of a FIXME in getVectorNumElements(). It just returns getVectorMinNumElements().

This code should use MVT::getVectorVT(MVT::i32, 2 * VT.getVectorElementCount()) I think

craig.topper added inline comments.Jan 13 2022, 9:23 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2219

I've resolved the FIXME so this should fail now.

jacquesguan marked 2 inline comments as done.

Address comment.

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

Thanks, done.

craig.topper added inline comments.Jan 13 2022, 7:04 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2218

Can this be MVT::getVectorVT(MVT::i32, 2 * VT.getVectorElementCount()) as I wrote in my previous comment. If not please tell me why not.

jacquesguan added inline comments.Jan 13 2022, 7:18 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2218

It can not, because the return type of getVectorElementCount is ElementCount, and the operator * is not overloaded for it.

craig.topper added inline comments.Jan 13 2022, 7:23 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2218

VT.getVectorElementCount() * 2 work? I think the operator overload is there but maybe not symmetric.

Address comment.

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

Yes, you are right. Thanks a lot.

This revision is now accepted and ready to land.Jan 13 2022, 8:46 PM
This revision was automatically updated to reflect the committed changes.