This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Use RegClass helper functions in getRegForInlineAsmConstraint.
ClosedPublic

Authored by foad on Apr 21 2020, 9:18 AM.

Details

Summary

This avoids more long lists of register classes that have to be updated
every time we add a new one. NFC.

Diff Detail

Event Timeline

foad created this revision.Apr 21 2020, 9:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2020, 9:18 AM
foad marked an inline comment as done.Apr 21 2020, 9:23 AM
foad added inline comments.
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
10597–10599

I don't like having exceptions like this, where we override the class that getSGPRClassForBitWidth would return. There are similar things in e.g. SIRegisterInfo::getEquivalentSGPRClass.

Perhaps getSGPRClassForBitWidth et al should be split into two different helpers, "get allocatable class" and "get *something else* class". But I really don't understand the purpose and the naming of the SReg/VReg/AReg vs SGPR/VGPR/AGPR classes, so I'm not sure how to do that.

arsenm added inline comments.Apr 21 2020, 9:54 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
10597–10599

For the VGPR and AGPR cases, I think it's a naming mistake based on how these are defined in tablegen. We ended up with the register list as VGPR_* and then the class as VReg_*s for > 32.

For SGPRs, there is a distinction. SReg_* is more inclusive. SGPR_* only include number SGPRs. The various SReg_*s include different combinations of special registers like m0 and vcc or ttmp

rampitec added inline comments.Apr 21 2020, 11:00 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
10636

This comment is obsolete. We do support v32* since then. You probably do not need special case for 1024 here.

foad updated this revision to Diff 259198.Apr 22 2020, 1:13 AM

Rebase.

foad marked 2 inline comments as done.Apr 22 2020, 1:32 AM
foad added inline comments.
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
10597–10599

Thanks. Perhaps we should have both getSGPRClassForBitWidth and getSRegClassForBitWidth?

What about the TTMP classes? Should there be one for every SGPR class? Currently we have TTMP_32/64/128/256/512, missing 96/160/1024.

10636

Thanks. I committed that separately and rebased this patch.

This revision is now accepted and ready to land.Apr 22 2020, 10:32 AM
This revision was automatically updated to reflect the committed changes.