This is an archive of the discontinued LLVM Phabricator instance.

[NFC][MC] MCRegister API typing.
ClosedPublic

Authored by mtrofin on Oct 7 2020, 4:13 PM.

Details

Summary

Mostly LiveIntervals, with their effects (users).

Diff Detail

Event Timeline

mtrofin created this revision.Oct 7 2020, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2020, 4:13 PM
mtrofin requested review of this revision.Oct 7 2020, 4:13 PM
gjain added inline comments.Oct 7 2020, 9:04 PM
llvm/lib/CodeGen/RegisterCoalescer.cpp
2396–2397

Should this be a reference?

llvm/lib/CodeGen/RegisterCoalescer.h
33

Should we omit the assignment since we are already using the default constructor?

36

ditto

llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
112–114

Should this be instead?

Register PhysReg(Register::isPhysicalRegister(MO->getReg()) ? MO->getReg() : VRM->getPhys(Reg))
mtrofin updated this revision to Diff 297022.Oct 8 2020, 11:20 AM
mtrofin marked 4 inline comments as done.

feedback

llvm/lib/CodeGen/RegisterCoalescer.cpp
2396–2397

No, storage-wise it's a 32 bit integer.

llvm/lib/CodeGen/RegisterCoalescer.h
33

done.

llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
112–114

It'll be lowered to the same thing. Plus, we still need to cast VRM->getPhys so both sides of the ternary operator have the same type.

gjain accepted this revision.Oct 8 2020, 11:22 AM
This revision is now accepted and ready to land.Oct 8 2020, 11:22 AM
MaskRay accepted this revision.Oct 8 2020, 1:12 PM
MaskRay added a subscriber: MaskRay.

Thanks! (Make sure that you have tested with LLVM_TARGETS_TO_BUILD=all for API changes like this. )

This revision was landed with ongoing or failed builds.Oct 8 2020, 3:08 PM
This revision was automatically updated to reflect the committed changes.