This is an archive of the discontinued LLVM Phabricator instance.

[GISel][NFC]: Add some constructors for easy creation of MIRBuilders
ClosedPublic

Authored by aditya_nandakumar on Aug 11 2017, 1:28 PM.

Details

Summary

Add some constructors for easier use of MIRBuilders in backend code/passes.
Currently to use the MIRBuilder, we need
MIRBuilder B;
B.setMF(MF);
B.setInstr(SomeMI);
It would be easier to just do
MIRBuilder B(MF, SomeMI);

Diff Detail

Event Timeline

qcolombet added inline comments.Aug 14 2017, 9:12 AM
include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
91

We could have a constructor with only MI, if that makes the usage simpler:
MachineIRBuilder(MI) : MachineIRBuild(MI.getParent()->getParent()) {
setInstr(MI);
}

aditya_nandakumar marked an inline comment as done.Aug 14 2017, 9:14 AM
aditya_nandakumar added inline comments.
include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
91

You mean - instead of the above two constructors, only have one which takes MI?

qcolombet added inline comments.Aug 14 2017, 9:17 AM
include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
91

No, I mean one with MF, one with MI instead of the pair (MF MI).
That being said if you prefer (MF, MI) instead of (MI) that works for me.

aditya_nandakumar marked 2 inline comments as done.Aug 14 2017, 9:21 AM
aditya_nandakumar added inline comments.
include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
91

I see. One with MI should suffice for now.

aditya_nandakumar marked an inline comment as done.

Updated based on Quentin's feedback.

qcolombet accepted this revision.Aug 14 2017, 9:29 AM

LGTM.
One nit below.

include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
92

I would rather call the MachineIRBuilder(MF) constructor.

This revision is now accepted and ready to land.Aug 14 2017, 9:29 AM

Committed in r310860 with the changes you requested.