[GISel]: Allow the MachineIRBuilder to insert instructions after

[aditya_nandakumar]

Summary

At times, it's necessary to insert instruction after an iterator.
Allow to specify InsertAfter as a boolean in the setInstr interface.

[qcolombet]

I believe we use to do that by using std::next([Instr | Iterator]WeWantToInsertAfter), but I can see how that could make things easier, so LGTM

[qcolombet]

Just one nit.

include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h

88

We should put a default value on that.

[aditya_nandakumar]

Thanks for the feedback. I've defaulted it to false.

[qcolombet]

Note: Given the other review, I let you decide if we need that patch or not.

[aditya_nandakumar]

Good point. I think this is not needed. I'll update the other patch to use the std::next version. We can add this back at a later time if it's of value.

[qcolombet]

Good point. I think this is not needed. I'll update the other patch to use the std::next version. We can add this back at a later time if it's of value.

You can probably directly use setInsertPt with getFirstTerminator I think (i.e., no need to use std::next ;))

[aditya_nandakumar]

Yup. That would work :). Thanks

include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h

class MachineIRBuilder {
   }
 
   void addUsesFromArgs(MachineInstrBuilder &MIB) { }
   template<typename UseArgTy, typename ... UseArgsTy>
   void addUsesFromArgs(MachineInstrBuilder &MIB, UseArgTy &&Arg1, UseArgsTy &&... Args) {
     addUseFromArg(MIB, Arg1);
     addUsesFromArgs(MIB, std::forward<UseArgsTy>(Args)...);
   }
+
+  bool InsertAfter;
 public:
   /// Some constructors for easy use.
   MachineIRBuilder() = default;
   MachineIRBuilder(MachineFunction &MF) { setMF(MF); }
-  MachineIRBuilder(MachineInstr &MI)
+  MachineIRBuilder(MachineInstr &MI, bool InsertAfter = false)
       : MachineIRBuilder(*MI.getParent()->getParent()) {
-    setInstr(MI);
+    setInstr(MI, InsertAfter);
   }
 
   /// Getter for the function we currently build.
   MachineFunction &getMF() {
     assert(MF && "MachineFunction is not set");
     return *MF;
   }
 
   /// Getter for the basic block we currently build.
   MachineBasicBlock &getMBB() {
     assert(MBB && "MachineBasicBlock is not set");
     return *MBB;
   }
 
   /// Current insertion point for new instructions.
   MachineBasicBlock::iterator getInsertPt() {
     return II;
   }
 
   /// Set the insertion point before the specified position.
   /// \pre MBB must be in getMF().
   /// \pre II must be a valid iterator in MBB.
-  void setInsertPt(MachineBasicBlock &MBB, MachineBasicBlock::iterator II);
+  void setInsertPt(MachineBasicBlock &MBB, MachineBasicBlock::iterator II, bool InsertAfter = false);
   /// @}
 
   /// \name Setters for the insertion point.
   /// @{
   /// Set the MachineFunction where to build instructions.
   void setMF(MachineFunction &);
 
   /// Set the insertion point to the  end of \p MBB.
   /// \pre \p MBB must be contained by getMF().
   void setMBB(MachineBasicBlock &MBB);
 
   /// Set the insertion point to before MI.
   /// \pre MI must be in getMF().
-  void setInstr(MachineInstr &MI);
+  void setInstr(MachineInstr &MI, bool InsertAfter = false);
   /// @}
 
   /// \name Control where instructions we create are recorded (typically for
   /// visiting again later during legalization).
   /// @{
   void recordInsertions(std::function<void(MachineInstr *)> InsertedInstr);
   void stopRecordingInsertions();
   /// @}

lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

 
 void MachineIRBuilder::setMBB(MachineBasicBlock &MBB) {
   this->MBB = &MBB;
   this->II = MBB.end();
   assert(&getMF() == MBB.getParent() &&
          "Basic block is in a different function");
 }
 
-void MachineIRBuilder::setInstr(MachineInstr &MI) {
+void MachineIRBuilder::setInstr(MachineInstr &MI, bool InsertAfter) {
   assert(MI.getParent() && "Instruction is not part of a basic block");
   setMBB(*MI.getParent());
   this->II = MI.getIterator();
+  this->InsertAfter = InsertAfter;
 }
 
 void MachineIRBuilder::setInsertPt(MachineBasicBlock &MBB,
-                                   MachineBasicBlock::iterator II) {
+                                   MachineBasicBlock::iterator II,
+                                   bool InsertAfter) {
   assert(MBB.getParent() == &getMF() &&
          "Basic block is in a different function");
   this->MBB = &MBB;
   this->II = II;
+  this->InsertAfter = InsertAfter;
 }
 
 void MachineIRBuilder::recordInsertions(
     std::function<void(MachineInstr *)> Inserted) {
   InsertedInstr = std::move(Inserted);
 }
 
 void MachineIRBuilder::stopRecordingInsertions() {

 
 MachineInstrBuilder MachineIRBuilder::buildInstrNoInsert(unsigned Opcode) {
   MachineInstrBuilder MIB = BuildMI(getMF(), DL, getTII().get(Opcode));
   return MIB;
 }
 
 
 MachineInstrBuilder MachineIRBuilder::insertInstr(MachineInstrBuilder MIB) {
-  getMBB().insert(getInsertPt(), MIB);
+  auto &MBB = getMBB();
+  if (!InsertAfter)
+    MBB.insert(getInsertPt(), MIB);
+  else
+    MBB.insertAfter(getInsertPt(), MIB);
+
   if (InsertedInstr)
     InsertedInstr(MIB);
   return MIB;
 }
 
 MachineInstrBuilder
 MachineIRBuilder::buildDirectDbgValue(unsigned Reg, const MDNode *Variable,
                                       const MDNode *Expr) {