This is an archive of the discontinued LLVM Phabricator instance.

[NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify getOperand(i).getReg()
ClosedPublic

Authored by aditya_nandakumar on Feb 1 2019, 11:56 AM.

Details

Summary

It's a common pattern in GISel to have a MachineInstrBuilder from which we get various regs (commonly MIB->getOperand(0).getReg()). This adds a helper method and the above can be replaced with MIB.getOperandReg(0).

Diff Detail

Repository
rL LLVM

Event Timeline

arsenm added inline comments.Feb 1 2019, 12:01 PM
lib/Target/AArch64/AArch64LegalizerInfo.cpp
501

Why does buildGEP use register inputs instead of SrcOp/DstOp like most of the other build functions?

aditya_nandakumar marked an inline comment as done.Feb 1 2019, 1:29 PM
aditya_nandakumar added inline comments.
lib/Target/AArch64/AArch64LegalizerInfo.cpp
501

I believe this was written way before the improvements to MachineIRBuilder and hasn't been updated since.

If we're going to add a helper to shave some characters off, we might as well go for something really short like getOpReg().

If we're going to add a helper to shave some characters off, we might as well go for something really short like getOpReg().

That works for me as well. While this seems really trivial, this is repeated lots of times and gets annoying very quickly - kind of like MI->getMF() vs MI->getParent()->getParent().

If we're going to add a helper to shave some characters off, we might as well go for something really short like getOpReg().

Or simply getReg(unsigned OpIdx)---this would keep the names identical. Also, should there be something like this for subregs as well?

arsenm added a comment.Feb 2 2019, 7:51 AM

If we're going to add a helper to shave some characters off, we might as well go for something really short like getOpReg().

Or simply getReg(unsigned OpIdx)---this would keep the names identical. Also, should there be something like this for subregs as well?

No, subregisters should never exist for G_* instructions

Updated to use shorter name.

aemerson accepted this revision.Feb 4 2019, 3:32 PM

LGTM.

This revision is now accepted and ready to land.Feb 4 2019, 3:32 PM