This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add missing AReg classes
ClosedPublic

Authored by foad on Apr 17 2020, 1:44 AM.

Details

Summary

Add 96-bit, 160-bit and 256-bit AReg classes to match VReg and SReg.
NFC as far as I know, but it may avoid weird legalization problems.

Diff Detail

Event Timeline

foad created this revision.Apr 17 2020, 1:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2020, 1:45 AM

Missing changes to: AMDGPUAsmPrinter::analyzeResourceUsage(), AMDGPUAsmParser.cpp:getRegClass(), SITargetLowering::getRegForInlineAsmConstraint().

llvm/lib/Target/AMDGPU/SIRegisterInfo.td
826

They all missing let Weight = ... I have somehow missed it.

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
1107

Missing AGPR* classes.

foad updated this revision to Diff 258939.Apr 21 2020, 2:43 AM

Add weights for all AReg classes.
Update AMDGPUAsmPrinter, AMDGPUAsmParser, AMDGPUDisassembler,
SIMCCodeEmitter and SITargetLowering::getRegForInlineAsmConstraint.

foad marked 2 inline comments as done.Apr 21 2020, 2:45 AM
foad added inline comments.
llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
1107

True, but it is also missing most VGPR* classes and some SGPR* classes. Are they really all supposed to be included here?

rampitec accepted this revision.Apr 21 2020, 10:33 AM

LGTM

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
1107

I think they are. If these will appear in MI operand definitions eventually it will assert.

This revision is now accepted and ready to land.Apr 21 2020, 10:33 AM
rampitec requested changes to this revision.Apr 21 2020, 3:37 PM

Actually SIMCCodeEmitter.cpp changes are an error.

llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
422

Ugh. This should not be here. It never can be SrcA or SrcB.

This revision now requires changes to proceed.Apr 21 2020, 3:37 PM
rampitec accepted this revision.Apr 21 2020, 4:17 PM
rampitec added inline comments.
llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
422

Ugh again. I did some testing and it turns to be right. Please disregard.

This revision is now accepted and ready to land.Apr 21 2020, 4:17 PM
This revision was automatically updated to reflect the committed changes.