This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add initial support for getRegUsageForType and getNumberOfRegisters
ClosedPublic

Authored by kito-cheng on Jan 9 2022, 6:38 AM.

Details

Summary

Those two TTI hooks are used during vectorization for calculating
register pressure, the default implementation isn't consider for LMUL,
and that's also definitly wrong value for register number (all register class
are 8 registers).

So in this patch we tried to:

  1. Calculate right register usage for vector type and scalar type.
  2. Return right number of register for general purpose register and vector register.

Diff Detail

Event Timeline

kito-cheng created this revision.Jan 9 2022, 6:38 AM
kito-cheng requested review of this revision.Jan 9 2022, 6:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 9 2022, 6:38 AM
kito-cheng updated this revision to Diff 398435.Jan 9 2022, 6:44 AM

Minor cleanup

kito-cheng updated this revision to Diff 398436.Jan 9 2022, 6:45 AM

Minor cleanup

craig.topper added inline comments.Jan 9 2022, 3:02 PM
llvm/lib/Target/RISCV/RISCVSubtarget.h
167

No else after return

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
280

Don't use auto

284

No else after return

288

Don't use auto

289

Drop curly braces

kito-cheng updated this revision to Diff 398479.Jan 9 2022, 6:38 PM

Changes:

kito-cheng marked 5 inline comments as done.Jan 9 2022, 6:38 PM
kito-cheng updated this revision to Diff 398482.Jan 9 2022, 6:40 PM

Minor cleanup

craig.topper added inline comments.Jan 10 2022, 11:10 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
286

Does this give a different answer than what the default implementation using getTypeLegalizationCost would give?

craig.topper added inline comments.Jan 10 2022, 11:13 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
286

Err nevermind. Of course it would

289

Can we fall back to the default implementation for non-vector?

kito-cheng added inline comments.Jan 10 2022, 7:43 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
289

Although I think the default one is not good, but we didn't find good way to testing those part yet, so OK to me.

craig.topper added inline comments.Jan 10 2022, 7:45 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
289

It's calling getTypeLegalizationCost which should say how many pieces a GPR splits into. I'm not sure what the right answer for FP is because FP can't be split.

kito-cheng added inline comments.Jan 10 2022, 7:52 PM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
289

Oh, I got that, I thought the default one is this, which is always return 1:
https://llvm.org/doxygen/TargetTransformInfoImpl_8h_source.html#l00305

But it should be this one:
https://llvm.org/doxygen/BasicTTIImpl_8h_source.html#l00365

Changes:

  • Use BasicTTIImplBase::getRegUsageForType for non-vector types.
craig.topper added inline comments.Jan 16 2022, 11:27 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
282

Do we need to check the V extension is enabled here?

Changes:

  • Check V ext. when calculating register usage for scalable types.
kito-cheng marked an inline comment as done.Jan 16 2022, 7:32 PM
kito-cheng added inline comments.
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
282

Good catch, thanks!

This revision is now accepted and ready to land.Jan 16 2022, 11:08 PM
This revision was landed with ongoing or failed builds.Jan 16 2022, 11:28 PM
This revision was automatically updated to reflect the committed changes.