This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Start trying to handle AGPR bank
ClosedPublic

Authored by arsenm on Jul 16 2020, 2:51 PM.

Details

Summary

Try to use AGPR banks for the various merge/unmerge type
operations. Previously these would introduce copies to VGPR.

Diff Detail

Event Timeline

arsenm created this revision.Jul 16 2020, 2:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 16 2020, 2:51 PM
kerbowa added inline comments.Jul 18 2020, 8:46 PM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
3140

Fix comment.

3154

Did you mean this to be 'if (RegBank == AMDGPU::VGPRRegBankID)', or else you can move this before the call to regBankUnion?

madhur13490 added inline comments.Jul 20 2020, 1:23 AM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
3140

I think these comments can be a bit more verbose.

3144

This function is returning 'int' but you're receiving it as 'unsigned'. As of now, this is fine but no need to have this discrepancy. I think it should be unsigned uniformly.

arsenm marked an inline comment as done.Jul 20 2020, 6:15 AM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
3144

Really it should be an enum, but TableGen emits these as an anonymous enum right now

madhur13490 added inline comments.Jul 20 2020, 8:09 AM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
3144

Sure, but the type should be same either unsigned or int but not such discrepancy. This is bug prone. If tablegen's enum is unsigned then let this function return unsigned,

arsenm marked an inline comment as done.Jul 20 2020, 4:26 PM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
3144

The enum is signed

arsenm updated this revision to Diff 279386.Jul 20 2020, 5:51 PM

Made enum unsigned

kerbowa accepted this revision.Jul 25 2020, 2:24 PM

LGTM with parent

This revision is now accepted and ready to land.Jul 25 2020, 2:24 PM