[GISel]: Provide standard interface to observe changes in GISel passes

https://reviews.llvm.org/D54980

This provides a standard API across GISel passes to observe and notify
passes about changes (insertions/deletions/mutations) to MachineInstrs.
This patch also removes the recordInsertion method in MachineIRBuilder
and instead provides method to setObserver.

Reviewed by: vkeles.

llvm-svn: 348406

llvm/include/llvm/CodeGen/GlobalISel/Combiner.h

@@ -24,16 +24,6 @@ class CombinerInfo;
 class TargetPassConfig;
 class MachineFunction;
 
-class CombinerChangeObserver {
-public:
-  virtual ~CombinerChangeObserver() {}
-
-  /// An instruction was erased.
-  virtual void erasedInstr(MachineInstr &MI) = 0;
-  /// An instruction was created and inseerted into the function.
-  virtual void createdInstr(MachineInstr &MI) = 0;
-};
-
 class Combiner {
 public:
   Combiner(CombinerInfo &CombinerInfo, const TargetPassConfig *TPC);

llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h

@@ -20,21 +20,21 @@
 
 namespace llvm {
 
-class CombinerChangeObserver;
+class GISelChangeObserver;
 class MachineIRBuilder;
 class MachineRegisterInfo;
 class MachineInstr;
 
 class CombinerHelper {
   MachineIRBuilder &Builder;
   MachineRegisterInfo &MRI;
-  CombinerChangeObserver &Observer;
+  GISelChangeObserver &Observer;
 
   void eraseInstr(MachineInstr &MI);
   void scheduleForVisit(MachineInstr &MI);
 
 public:
-  CombinerHelper(CombinerChangeObserver &Observer, MachineIRBuilder &B);
+  CombinerHelper(GISelChangeObserver &Observer, MachineIRBuilder &B);
 
   /// If \p MI is COPY, try to combine it.
   /// Returns true if MI changed.

llvm/include/llvm/CodeGen/GlobalISel/CombinerInfo.h

@@ -17,7 +17,7 @@
 #include <cassert>
 namespace llvm {
 
-class CombinerChangeObserver;
+class GISelChangeObserver;
 class LegalizerInfo;
 class MachineInstr;
 class MachineIRBuilder;
@@ -43,7 +43,7 @@ class CombinerInfo {
   /// illegal ops that are created.
   bool LegalizeIllegalOps; // TODO: Make use of this.
   const LegalizerInfo *LInfo;
-  virtual bool combine(CombinerChangeObserver &Observer, MachineInstr &MI,
+  virtual bool combine(GISelChangeObserver &Observer, MachineInstr &MI,
                        MachineIRBuilder &B) const = 0;
 };
 } // namespace llvm

llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h

@@ -0,0 +1,38 @@
+//== ----- llvm/CodeGen/GlobalISel/GISelChangeObserver.h ---------------------
+//== //
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+/// This contains common code to allow clients to notify changes to machine
+/// instr.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CODEGEN_GLOBALISEL_GISELCHANGEOBSERVER_H
+#define LLVM_CODEGEN_GLOBALISEL_GISELCHANGEOBSERVER_H
+
+namespace llvm {
+/// Abstract class that contains various methods for clients to notify about
+/// changes. This should be the preferred way for APIs to notify changes.
+/// Typically calling erasedInstr/createdInstr multiple times should not affect
+/// the result. The observer would likely need to check if it was already
+/// notified earlier (consider using GISelWorkList).
+class MachineInstr;
+class GISelChangeObserver {
+public:
+  virtual ~GISelChangeObserver() {}
+
+  /// An instruction was erased.
+  virtual void erasedInstr(MachineInstr &MI) = 0;
+  /// An instruction was created and inserted into the function.
+  virtual void createdInstr(MachineInstr &MI) = 0;
+  /// This instruction was mutated in some way.
+  virtual void changedInstr(MachineInstr &MI) = 0;
+};
+
+} // namespace llvm
+#endif

llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h

@@ -32,6 +32,7 @@ namespace llvm {
 class LegalizerInfo;
 class Legalizer;
 class MachineRegisterInfo;
+class GISelChangeObserver;
 
 class LegalizerHelper {
 public:
@@ -48,8 +49,9 @@ class LegalizerHelper {
     UnableToLegalize,
   };
 
-  LegalizerHelper(MachineFunction &MF);
-  LegalizerHelper(MachineFunction &MF, const LegalizerInfo &LI);
+  LegalizerHelper(MachineFunction &MF, GISelChangeObserver &Observer);
+  LegalizerHelper(MachineFunction &MF, const LegalizerInfo &LI,
+                  GISelChangeObserver &Observer);
 
   /// Replace \p MI by a sequence of legal instructions that can implement the
   /// same operation. Note that this means \p MI may be deleted, so any iterator
@@ -117,6 +119,8 @@ class LegalizerHelper {
 
   MachineRegisterInfo &MRI;
   const LegalizerInfo &LI;
+  /// To keep track of changes made by the LegalizerHelper.
+  GISelChangeObserver &Observer;
 };
 
 /// Helper function that creates the given libcall.

llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h

@@ -39,6 +39,7 @@ class MachineInstr;
 class MachineIRBuilder;
 class MachineRegisterInfo;
 class MCInstrInfo;
+class GISelChangeObserver;
 
 namespace LegalizeActions {
 enum LegalizeAction : std::uint8_t {
@@ -949,9 +950,9 @@ class LegalizerInfo {
 
   bool isLegal(const MachineInstr &MI, const MachineRegisterInfo &MRI) const;
 
-  virtual bool legalizeCustom(MachineInstr &MI,
-                              MachineRegisterInfo &MRI,
-                              MachineIRBuilder &MIRBuilder) const;
+  virtual bool legalizeCustom(MachineInstr &MI, MachineRegisterInfo &MRI,
+                              MachineIRBuilder &MIRBuilder,
+                              GISelChangeObserver &Observer) const;
 
 private:
   /// Determine what action should be taken to legalize the given generic

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

@@ -30,6 +30,7 @@ namespace llvm {
 class MachineFunction;
 class MachineInstr;
 class TargetInstrInfo;
+class GISelChangeObserver;
 
 /// Class which stores all the state required in a MachineIRBuilder.
 /// Since MachineIRBuilders will only store state in this object, it allows
@@ -50,7 +51,7 @@ struct MachineIRBuilderState {
   MachineBasicBlock::iterator II;
   /// @}
 
-  std::function<void(MachineInstr *)> InsertedInstr;
+  GISelChangeObserver *Observer;
 };
 
 /// Helper class to build MachineInstr.
@@ -61,6 +62,7 @@ class MachineIRBuilderBase {
 
   MachineIRBuilderState State;
   void validateTruncExt(unsigned Dst, unsigned Src, bool IsExtend);
+  void recordInsertion(MachineInstr *MI) const;
 
 protected:
   unsigned getDestFromArg(unsigned Reg) { return Reg; }
@@ -151,12 +153,8 @@ class MachineIRBuilderBase {
   void setInstr(MachineInstr &MI);
   /// @}
 
-  /// \name Control where instructions we create are recorded (typically for
-  /// visiting again later during legalization).
-  /// @{
-  void recordInsertion(MachineInstr *InsertedInstr) const;
-  void recordInsertions(std::function<void(MachineInstr *)> InsertedInstr);
-  void stopRecordingInsertions();
+  void setChangeObserver(GISelChangeObserver &Observer);
+  void stopObservingChanges();
   /// @}
 
   /// Set the debug location to \p DL for all the next build instructions.

llvm/lib/CodeGen/GlobalISel/Combiner.cpp

@@ -12,12 +12,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GlobalISel/Combiner.h"
+#include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/CodeGen/GlobalISel/CombinerInfo.h"
-#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
-#include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/GISelWorkList.h"
-#include "llvm/ADT/PostOrderIterator.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
+#include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 
@@ -34,7 +35,7 @@ namespace {
 /// instruction creation will schedule that instruction for a future visit.
 /// Other Combiner implementations may require more complex behaviour from
 /// their CombinerChangeObserver subclass.
-class WorkListMaintainer : public CombinerChangeObserver {
+class WorkListMaintainer : public GISelChangeObserver {
   using WorkListTy = GISelWorkList<512>;
   WorkListTy &WorkList;
 
@@ -50,6 +51,11 @@ class WorkListMaintainer : public CombinerChangeObserver {
     LLVM_DEBUG(dbgs() << "Created: "; MI.print(dbgs()); dbgs() << "\n");
     WorkList.insert(&MI);
   }
+  // Currently changed conservatively assumes erased.
+  void changedInstr(MachineInstr &MI) override {
+    LLVM_DEBUG(dbgs() << "Changed: "; MI.print(dbgs()); dbgs() << "\n");
+    WorkList.remove(&MI);
+  }
 };
 }
 

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

@@ -6,8 +6,9 @@
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
-#include "llvm/CodeGen/GlobalISel/Combiner.h"
 #include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
+#include "llvm/CodeGen/GlobalISel/Combiner.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineInstr.h"
@@ -18,7 +19,7 @@
 
 using namespace llvm;
 
-CombinerHelper::CombinerHelper(CombinerChangeObserver &Observer,
+CombinerHelper::CombinerHelper(GISelChangeObserver &Observer,
                                MachineIRBuilder &B)
     : Builder(B), MRI(Builder.getMF().getRegInfo()), Observer(Observer) {}
 

llvm/lib/CodeGen/GlobalISel/Legalizer.cpp

@@ -16,6 +16,7 @@
 #include "llvm/CodeGen/GlobalISel/Legalizer.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/GISelWorkList.h"
 #include "llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
@@ -67,6 +68,42 @@ static bool isArtifact(const MachineInstr &MI) {
     return true;
   }
 }
+using InstListTy = GISelWorkList<256>;
+using ArtifactListTy = GISelWorkList<128>;
+
+class LegalizerWorkListManager : public GISelChangeObserver {
+  InstListTy &InstList;
+  ArtifactListTy &ArtifactList;
+
+public:
+  LegalizerWorkListManager(InstListTy &Insts, ArtifactListTy &Arts)
+      : InstList(Insts), ArtifactList(Arts) {}
+
+  void createdInstr(MachineInstr &MI) override {
+    // Only legalize pre-isel generic instructions.
+    // Legalization process could generate Target specific pseudo
+    // instructions with generic types. Don't record them
+    if (isPreISelGenericOpcode(MI.getOpcode())) {
+      if (isArtifact(MI))
+        ArtifactList.insert(&MI);
+      else
+        InstList.insert(&MI);
+    }
+    LLVM_DEBUG(dbgs() << ".. .. New MI: " << MI;);
+  }
+
+  void erasedInstr(MachineInstr &MI) override {
+    InstList.remove(&MI);
+    ArtifactList.remove(&MI);
+  }
+
+  void changedInstr(MachineInstr &MI) override {
+    // When insts change, we want to revisit them to legalize them again.
+    // We'll consider them the same as created.
+    LLVM_DEBUG(dbgs() << ".. .. Changed MI: " << MI;);
+    createdInstr(MI);
+  }
+};
 
 bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
   // If the ISel pipeline failed, do not bother running that pass.
@@ -77,14 +114,13 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
   init(MF);
   const TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
   MachineOptimizationRemarkEmitter MORE(MF, /*MBFI=*/nullptr);
-  LegalizerHelper Helper(MF);
 
   const size_t NumBlocks = MF.size();
   MachineRegisterInfo &MRI = MF.getRegInfo();
 
   // Populate Insts
-  GISelWorkList<256> InstList;
-  GISelWorkList<128> ArtifactList;
+  InstListTy InstList;
+  ArtifactListTy ArtifactList;
   ReversePostOrderTraversal<MachineFunction *> RPOT(&MF);
   // Perform legalization bottom up so we can DCE as we legalize.
   // Traverse BB in RPOT and within each basic block, add insts top down,
@@ -103,24 +139,12 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
         InstList.insert(&MI);
     }
   }
-  Helper.MIRBuilder.recordInsertions([&](MachineInstr *MI) {
-    // Only legalize pre-isel generic instructions.
-    // Legalization process could generate Target specific pseudo
-    // instructions with generic types. Don't record them
-    if (isPreISelGenericOpcode(MI->getOpcode())) {
-      if (isArtifact(*MI))
-        ArtifactList.insert(MI);
-      else
-        InstList.insert(MI);
-    }
-    LLVM_DEBUG(dbgs() << ".. .. New MI: " << *MI;);
-  });
+  LegalizerWorkListManager WorkListObserver(InstList, ArtifactList);
+  LegalizerHelper Helper(MF, WorkListObserver);
   const LegalizerInfo &LInfo(Helper.getLegalizerInfo());
   LegalizationArtifactCombiner ArtCombiner(Helper.MIRBuilder, MF.getRegInfo(), LInfo);
-  auto RemoveDeadInstFromLists = [&InstList,
-                                  &ArtifactList](MachineInstr *DeadMI) {
-    InstList.remove(DeadMI);
-    ArtifactList.remove(DeadMI);
+  auto RemoveDeadInstFromLists = [&WorkListObserver](MachineInstr *DeadMI) {
+    WorkListObserver.erasedInstr(*DeadMI);
   };
   bool Changed = false;
   do {
@@ -138,7 +162,7 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
       // Error out if we couldn't legalize this instruction. We may want to
       // fall back to DAG ISel instead in the future.
       if (Res == LegalizerHelper::UnableToLegalize) {
-        Helper.MIRBuilder.stopRecordingInsertions();
+        Helper.MIRBuilder.stopObservingChanges();
         reportGISelFailure(MF, TPC, MORE, "gisel-legalize",
                            "unable to legalize instruction", MI);
         return false;