[llvm] InstructionSelect: Use GISelChangeObserver instead of MachineFunction::Delegate (PR #105725)

[Daniel Sanders via llvm-commits]

https://github.com/dsandersllvm updated https://github.com/llvm/llvm-project/pull/105725

>From 3205d6fd5fb07eea8f3d5e8f67a1cd2e7950c32a Mon Sep 17 00:00:00 2001
From: Daniel Sanders <daniel_l_sanders at apple.com>
Date: Wed, 21 Aug 2024 17:22:26 -0700
Subject: [PATCH] [GlobalISel] InstructionSelect: Use GISelChangeObserver
 instead of MachineFunction::Delegate

The main difference is that it's possible for multiple change observers to be
installed at the same time whereas there can only be one MachineFunction
delegate installed. This allows downstream targets to continue to use
observers to recursively select. The target in question was selecting a
gMIR instruction to a machine instruction plus some gMIR around it and
relying on observers to ensure it correctly selected any gMIR it created
before returning to the main loop.
---
 .../CodeGen/GlobalISel/GISelChangeObserver.h     | 14 ++++++++++++++
 .../CodeGen/GlobalISel/InstructionSelector.h     |  4 ++++
 .../CodeGen/GlobalISel/GISelChangeObserver.cpp   | 10 ++++++++++
 .../lib/CodeGen/GlobalISel/InstructionSelect.cpp | 16 +++++++++++-----
 4 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
index 7ec5dac9a6ebaf..1167d51e88b71c 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
@@ -138,5 +138,19 @@ class RAIIMFObsDelInstaller {
   ~RAIIMFObsDelInstaller() = default;
 };
 
+/// A simple RAII based Observer installer.
+/// Use this in a scope to install the Observer to the MachineFunction and reset
+/// it at the end of the scope.
+class RAIITemporaryObserverInstaller {
+public:
+  RAIITemporaryObserverInstaller(GISelObserverWrapper &Observers,
+                                 GISelChangeObserver &TemporaryObserver);
+  ~RAIITemporaryObserverInstaller();
+
+private:
+  GISelObserverWrapper &Observers;
+  GISelChangeObserver &TemporaryObserver;
+};
+
 } // namespace llvm
 #endif
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
index aaba56ee11251c..ae75f2593aeffe 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
@@ -16,6 +16,8 @@
 #include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
 
 namespace llvm {
+class GISelObserverWrapper;
+
 class InstructionSelector : public GIMatchTableExecutor {
 public:
   virtual ~InstructionSelector();
@@ -36,6 +38,8 @@ class InstructionSelector : public GIMatchTableExecutor {
   const TargetPassConfig *TPC = nullptr;
 
   MachineOptimizationRemarkEmitter *MORE = nullptr;
+
+  GISelObserverWrapper *AllObservers = nullptr;
 };
 } // namespace llvm
 
diff --git a/llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp b/llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp
index 59f4d60a41d80d..836d54fa989d78 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp
@@ -46,3 +46,13 @@ RAIIMFObserverInstaller::RAIIMFObserverInstaller(MachineFunction &MF,
 }
 
 RAIIMFObserverInstaller::~RAIIMFObserverInstaller() { MF.setObserver(nullptr); }
+
+RAIITemporaryObserverInstaller::RAIITemporaryObserverInstaller(
+    GISelObserverWrapper &Observers, GISelChangeObserver &TemporaryObserver)
+    : Observers(Observers), TemporaryObserver(TemporaryObserver) {
+  Observers.addObserver(&TemporaryObserver);
+}
+
+RAIITemporaryObserverInstaller::~RAIITemporaryObserverInstaller() {
+  Observers.removeObserver(&TemporaryObserver);
+}
diff --git a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
index 8c0bb85fd0771c..8639c39e151dab 100644
--- a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
@@ -75,19 +75,21 @@ InstructionSelect::InstructionSelect(CodeGenOptLevel OL, char &PassID)
 /// a non-obvious limitation for selector implementers. Therefore, to allow
 /// deletion of arbitrary instructions, we detect this case and continue
 /// selection with the predecessor of the deleted instruction.
-class InstructionSelect::MIIteratorMaintainer
-    : public MachineFunction::Delegate {
+class InstructionSelect::MIIteratorMaintainer : public GISelChangeObserver {
 #ifndef NDEBUG
   SmallSetVector<const MachineInstr *, 32> CreatedInstrs;
 #endif
 public:
   MachineBasicBlock::reverse_iterator MII;
 
-  void MF_HandleInsertion(MachineInstr &MI) override {
+  void changingInstr(MachineInstr &MI) override {}
+  void changedInstr(MachineInstr &MI) override {}
+
+  void createdInstr(MachineInstr &MI) override {
     LLVM_DEBUG(dbgs() << "Creating:  " << MI; CreatedInstrs.insert(&MI));
   }
 
-  void MF_HandleRemoval(MachineInstr &MI) override {
+  void erasingInstr(MachineInstr &MI) override {
     LLVM_DEBUG(dbgs() << "Erasing:   " << MI; CreatedInstrs.remove(&MI));
     if (MII.getInstrIterator().getNodePtr() == &MI) {
       // If the iterator points to the MI that will be erased (i.e. the MI prior
@@ -190,8 +192,11 @@ bool InstructionSelect::selectMachineFunction(MachineFunction &MF) {
     // GISelChangeObserver, because we do not want notifications about changed
     // instructions. This prevents significant compile-time regressions from
     // e.g. constrainOperandRegClass().
+    GISelObserverWrapper AllObservers;
     MIIteratorMaintainer MIIMaintainer;
-    RAIIDelegateInstaller DelInstaller(MF, &MIIMaintainer);
+    AllObservers.addObserver(&MIIMaintainer);
+    RAIIDelegateInstaller DelInstaller(MF, &AllObservers);
+    ISel->AllObservers = &AllObservers;
 
     for (MachineBasicBlock *MBB : post_order(&MF)) {
       ISel->CurMBB = MBB;
@@ -215,6 +220,7 @@ bool InstructionSelect::selectMachineFunction(MachineFunction &MF) {
         LLVM_DEBUG(MIIMaintainer.reportFullyCreatedInstrs());
       }
     }
+    AllObservers.removeObserver(&MIIMaintainer);
   }
 
   for (MachineBasicBlock &MBB : MF) {