This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Define risc-v's own register class to model FP Register.
ClosedPublic

Authored by ym1813382441 on Jun 1 2022, 9:53 PM.

Details

Summary

The default RegisterClass is not enough to model RISCV Register.
We define risc-v's own register class to model FP Register.
This helps to better estimate the register pressure in the loop-vectorize.

Diff Detail

Event Timeline

ym1813382441 created this revision.Jun 1 2022, 9:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2022, 9:53 PM
ym1813382441 requested review of this revision.Jun 1 2022, 9:53 PM
kito-cheng added inline comments.Jun 2 2022, 12:35 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
232

Checking isHalfTy rather than isBFloatTy here, isBFloatTy is bfloat type and we don't support that in any extension yet.

Corrected half-precision floating point.

ym1813382441 marked an inline comment as done.Jun 2 2022, 12:49 AM
kito-cheng added inline comments.Jun 2 2022, 1:05 AM
llvm/test/Transforms/LoopVectorize/RISCV/reg-usage.ll
24–25

Curious why there is still GPRRC here? I thought that should be FPRRC after this patch?

ym1813382441 added inline comments.Jun 2 2022, 2:33 AM
llvm/test/Transforms/LoopVectorize/RISCV/reg-usage.ll
24–25

Here is the register usage after VF is selected, all float types in this loop has been changed to the vector type after vectorization.
Only Uniform and Scalar instructions after vectorization are computed into scalar registers.
The output here may not be complete enough to cause you to misunderstand.

kito-cheng added inline comments.Jun 2 2022, 2:37 AM
llvm/test/Transforms/LoopVectorize/RISCV/reg-usage.ll
24–25

Yeah, make sense, does it possible to add some testcase to show FPRRC?

ym1813382441 added inline comments.Jun 5 2022, 6:29 PM
llvm/test/Transforms/LoopVectorize/RISCV/reg-usage.ll
24–25

Yeah, it's very simple. Just set VF to 1 to show FPRRC.

Add command line to show FPRRC.

ym1813382441 marked an inline comment as done.Jun 5 2022, 6:32 PM
kito-cheng added inline comments.Jun 5 2022, 7:43 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
213

Checking F is enough here since Zfh and D both implied F.

ym1813382441 added inline comments.Jun 5 2022, 10:14 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
213

okey

Remove redundant conditions.

ym1813382441 marked an inline comment as done.Jun 5 2022, 10:14 PM
This revision is now accepted and ready to land.Jun 5 2022, 10:50 PM

Just some nits from me

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
213

Nit but with V we have a ternary but here we have an if/else (of sorts)

Maybe they should be consistent?

224

Blank line between functions

239

Unnecessary semicolon. Also blank line between functions

This revision was landed with ongoing or failed builds.Jun 5 2022, 11:44 PM
This revision was automatically updated to reflect the committed changes.

Just some nits from me

Sorry I didn't notice. I have submitted it.

Sorry I didn't notice. I have submitted it.

You could create another NFC patch to address @frasercrmck's comment :)