[PATCH] D81382: GlobalISel: Improve MachineIRBuilder construction

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: dsanders, aemerson, aditya_nandakumar, paquette.
Herald added subscribers: hiraditya, rovka, wdng.
Herald added a project: LLVM.

The current relationship between LegalizerHelper and MachineIRBuilder
confuses me, because the LegalizerHelper modifies the MachineIRBuilder
which it does not own. Constructing a LegalizerHelper destroys the
insert point, since the constructor calls setMF, which clears all the
fields. Try to separate these functions, so it's possible to construct
a LegalizerHelper from an existing MachineIRBuilder without losing the
insert point/debug loc.


https://reviews.llvm.org/D81382

Files:
  llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
  llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
  llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp


Index: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -87,7 +87,6 @@
                                  MachineIRBuilder &Builder)
     : MIRBuilder(Builder), MRI(MF.getRegInfo()),
       LI(*MF.getSubtarget().getLegalizerInfo()), Observer(Observer) {
-  MIRBuilder.setMF(MF);
   MIRBuilder.setChangeObserver(Observer);
 }
 
@@ -95,7 +94,6 @@
                                  GISelChangeObserver &Observer,
                                  MachineIRBuilder &B)
     : MIRBuilder(B), MRI(MF.getRegInfo()), LI(LI), Observer(Observer) {
-  MIRBuilder.setMF(MF);
   MIRBuilder.setChangeObserver(Observer);
 }
 LegalizerHelper::LegalizeResult
Index: llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
+++ llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
@@ -170,6 +170,7 @@
                                    ArrayRef<GISelChangeObserver *> AuxObservers,
                                    LostDebugLocObserver &LocObserver,
                                    MachineIRBuilder &MIRBuilder) {
+  MIRBuilder.setMF(MF);
   MachineRegisterInfo &MRI = MF.getRegInfo();
 
   // Populate worklists.
Index: llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
===================================================================
--- llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
+++ llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
@@ -37,23 +37,23 @@
 /// to transfer BuilderState between different kinds of MachineIRBuilders.
 struct MachineIRBuilderState {
   /// MachineFunction under construction.
-  MachineFunction *MF;
+  MachineFunction *MF = nullptr;
   /// Information used to access the description of the opcodes.
-  const TargetInstrInfo *TII;
+  const TargetInstrInfo *TII = nullptr;
   /// Information used to verify types are consistent and to create virtual registers.
-  MachineRegisterInfo *MRI;
+  MachineRegisterInfo *MRI = nullptr;
   /// Debug location to be set to any instruction we create.
   DebugLoc DL;
 
   /// \name Fields describing the insertion point.
   /// @{
-  MachineBasicBlock *MBB;
+  MachineBasicBlock *MBB = nullptr;
   MachineBasicBlock::iterator II;
   /// @}
 
-  GISelChangeObserver *Observer;
+  GISelChangeObserver *Observer = nullptr;
 
-  GISelCSEInfo *CSEInfo;
+  GISelCSEInfo *CSEInfo = nullptr;
 };
 
 class DstOp {
@@ -240,8 +240,16 @@
   /// Some constructors for easy use.
   MachineIRBuilder() = default;
   MachineIRBuilder(MachineFunction &MF) { setMF(MF); }
-  MachineIRBuilder(MachineInstr &MI) : MachineIRBuilder(*MI.getMF()) {
+
+  MachineIRBuilder(MachineBasicBlock &MBB, MachineBasicBlock::iterator InsPt) {
+    setMF(*MBB.getParent());
+    setInsertPt(MBB, InsPt);
+  }
+
+  MachineIRBuilder(MachineInstr &MI) :
+    MachineIRBuilder(*MI.getParent(), MI.getIterator()) {
     setInstr(MI);
+    setDebugLoc(MI.getDebugLoc());
   }
 
   virtual ~MachineIRBuilder() = default;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81382.269182.patch
Type: text/x-patch
Size: 3129 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200608/8ed0a908/attachment.bin>