This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add basic cost model for vector casting
ClosedPublic

Authored by fakepaper56 on Mar 15 2022, 11:09 PM.

Details

Summary

To perform the cost model of vector casting, the patch consider most vector
casts as their scalar form and consider those vector form of free scalr castings
as 1.

Diff Detail

Event Timeline

fakepaper56 created this revision.Mar 15 2022, 11:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 11:09 PM
fakepaper56 requested review of this revision.Mar 15 2022, 11:09 PM

Fix the wrong commit name

fakepaper56 retitled this revision from [RISCV] Add basic cost model for vector casting.x to [RISCV] Add basic cost model for vector casting.Mar 15 2022, 11:14 PM
kito-cheng added inline comments.Mar 15 2022, 11:40 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
229

Maybe check FixedVectorType rather than VectorType? I assume you only intend to handle fixed vector size since you checking useRVVForFixedLengthVectors here?

254

vecotr - > vector

fakepaper56 added a comment.EditedMar 15 2022, 11:57 PM

I also want to handle scalable vector type. useRVVForFixedLengthVectors is used to make sure fixed vectors like v4i8 are legal. But I found that it is redundant for isTypeLegal testing.

fakepaper56 updated this revision to Diff 415713.EditedMar 16 2022, 12:41 AM

The update removed useRVVForFixedLengthVectors() check since it is coverd by isTypeLegal() check and fixed the typo.

fakepaper56 marked an inline comment as done.Mar 16 2022, 12:42 AM
craig.topper added inline comments.Mar 16 2022, 12:00 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
229

Do you intend to use getTypeLegalizationCost to do this properly in the future?

246

FP_TO_SINT/UINT is missing

248

Is there any point in getting the scalar cost? Seems like its always 1 and doesn't have a real connection to how the vector ISA works.

I'm also questioning the accuracy of some of the scalar numbers. i8->float requires a pair of shifts to clear the upper bits before doing an iXLen->float conversion for example. So if we were to fix that, it would be wrong for vectors.

llvm/test/Analysis/CostModel/RISCV/cast.ll
121

If you're going to test half vectors you should have +zfh on the command line

257

How wrong are we intending the costs to be? This i8-float requires 2 vector instructions since the elements sizes are more than a factor of 2 apart.

fakepaper56 added inline comments.Mar 16 2022, 8:29 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
248

My point to use scalar cost is just it is simple to implement and enough to make vectorizer think vectorizing casting instructions is profitable in most cases.

I had written a version by vector instruction count. The logic is a little complex and I am not sure when to consider vsetvli and even lmul. I think maybe we should refine it to a table having some magic number like X86 and Arm, so I just provide the simple design now.

fakepaper56 marked an inline comment as not done.Mar 16 2022, 8:29 PM
craig.topper added inline comments.Mar 16 2022, 8:32 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
248

Please add a FIXME to document what is missing.

fakepaper56 added inline comments.Mar 16 2022, 11:01 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
229

I think I should do it. But I am not sure how to do when the legalization costs are different between Dst and Src. I will also add FIXME here.

fakepaper56 updated this revision to Diff 416131.EditedMar 17 2022, 4:04 AM

Change the approach of cost model to caculate the vector casting counts.
I don't consider vsetvli in this patch, but it may cause not accurate cost
for multiple conversioin.

fakepaper56 marked an inline comment as done.

Update the commit summary.

craig.topper added inline comments.Mar 21 2022, 11:54 AM
llvm/test/Analysis/CostModel/RISCV/cast.ll
3

Need +experimental-zvfh

craig.topper accepted this revision.Mar 21 2022, 11:55 AM

LGTM other than fixing the test command line

This revision is now accepted and ready to land.Mar 21 2022, 11:55 AM

Fixing the test cases.

fakepaper56 marked an inline comment as done.Mar 21 2022, 11:16 PM
This revision was automatically updated to reflect the committed changes.