Page MenuHomePhabricator

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
119

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

124

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
119

It's been called from SelectionDAG too.

124

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
120–121

Can this actually fail?

122–123

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

cdevadas updated this revision to Diff 383475.Fri, Oct 29, 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.Fri, Oct 29, 2:04 PM
arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/Utils.cpp
116

Typo overriden

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