This is an archive of the discontinued LLVM Phabricator instance.

[NFC][MC] TargetRegisterInfo::getSubReg is a MCRegister.
ClosedPublic

Authored by mtrofin on Nov 30 2020, 12:21 PM.

Details

Diff Detail

Event Timeline

mtrofin created this revision.Nov 30 2020, 12:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 30 2020, 12:21 PM
mtrofin requested review of this revision.Nov 30 2020, 12:21 PM
gjain added inline comments.Dec 2 2020, 10:19 AM
llvm/lib/CodeGen/CalcSpillWeights.cpp
67

CopiedPReg should be a MCRegister instead. Looking at the documentation of contains it seems there is a desire that it only takes MCRegisters

llvm/lib/Target/Hexagon/BitTracker.cpp
344

Seems like PhyR should be a MCRegister and getPhysRegBitWidth should only take MCRegisters.

llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp
596

getMinimalPhysRegClass takes an MCRegister so I think we should not be converting it to a Register

llvm/lib/Target/Mips/MipsExpandPseudo.cpp
739

Seems like this should be a MCRegister

dsanders accepted this revision.Dec 2 2020, 10:32 AM

LGTM with @gjain's suggestions. I think SIRegisterInfo.cpp could also propagate MCRegister a bit further if you wanted but I didn't check all the users of SubReg

This revision is now accepted and ready to land.Dec 2 2020, 10:32 AM
mtrofin marked 4 inline comments as done.Dec 2 2020, 3:36 PM

LGTM with @gjain's suggestions. I think SIRegisterInfo.cpp could also propagate MCRegister a bit further if you wanted but I didn't check all the users of SubReg

Ack - that probably needs its own patch, it gets a bit involved. I'll do it next.

mtrofin updated this revision to Diff 309080.Dec 2 2020, 3:36 PM

feedback

This revision was landed with ongoing or failed builds.Dec 2 2020, 3:47 PM
This revision was automatically updated to reflect the committed changes.