This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel/Utils: Use incoming regbank while constraining the superclasses
ClosedPublic

Authored by cdevadas on Oct 22 2021, 9:09 AM.

Details

Summary

Register operands with superclasses can possibly have multiple regBanks
if they have different register types. The regBank ambiguity resolved
during regbankselect should be used to constrain the operand regclass
instead of obtaining one from the MCInstrDesc.

This is a prerequisite patch for D109300 that introduces allocatable AV_*
Superclasses for AMDGPU by combining both VGPRs and AGPRs and we want to
restrain the regclass to either A or V based on the incoming regbank.

Diff Detail

Event Timeline

cdevadas created this revision.Oct 22 2021, 9:09 AM
cdevadas requested review of this revision.Oct 22 2021, 9:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 22 2021, 9:09 AM

It's not accurate to say it would lead to incorrect codegen, we would just prefer to pick the classes beforehand rather than let the allocator decide until it needs to split live ranges

llvm/lib/CodeGen/GlobalISel/Utils.cpp
117

I'm not sure there's any case where this shoudl be called where MRI.getRegBankOrNull(Reg) can be null?

122

I would expect this to just directly assign getCommonSubClass's result without the comparison

It's not accurate to say it would lead to incorrect codegen, we would just prefer to pick the classes beforehand rather than let the allocator decide until it needs to split live ranges

Yes, I was referring to the case when we fail to adjust the opcode for MOV/COPY to AV_* classes if we let regalloc to choose the register without fixing the class beforehand.
I will modify the description.

llvm/lib/CodeGen/GlobalISel/Utils.cpp
117

It's been called from SelectionDAG too.

122

Will do.

cdevadas updated this revision to Diff 381619.Oct 22 2021, 11:59 AM
cdevadas edited the summary of this revision. (Show Details)

Comments addressed.

arsenm added inline comments.Oct 26 2021, 1:14 PM
llvm/lib/CodeGen/GlobalISel/Utils.cpp
118–119

Can this actually fail?

120–121

Why not just call getAllocatableClass on it, like how InstrEmitter does it?

cdevadas updated this revision to Diff 383475.Oct 29 2021, 1:05 PM

Addressed review comments.
(Used getAllocatableClass + a fix in getConstrainedRegClassForOperand to avoid a crash if regbank is not yet selected)

arsenm accepted this revision.Oct 29 2021, 2:04 PM
arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/Utils.cpp
114

Typo overriden

This revision is now accepted and ready to land.Oct 29 2021, 2:04 PM