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

[Daniel Sanders via llvm-commits]

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

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.

>From 13cdc0c49797d3140e0aff6a4e8c1e4eff0d1059 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] 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.
---
 .../llvm/CodeGen/GlobalISel/GISelChangeObserver.h | 15 +++++++++++++++
 .../llvm/CodeGen/GlobalISel/InstructionSelector.h |  4 ++++
 .../CodeGen/GlobalISel/GISelChangeObserver.cpp    | 10 ++++++++++
 llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp | 15 +++++++++++----
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
index 7ec5dac9a6ebaf..214777ca9082d8 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
@@ -138,5 +138,20 @@ 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..f89a0e84bcd420 100644
--- a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
@@ -76,18 +76,21 @@ InstructionSelect::InstructionSelect(CodeGenOptLevel OL, char &PassID)
 /// deletion of arbitrary instructions, we detect this case and continue
 /// selection with the predecessor of the deleted instruction.
 class InstructionSelect::MIIteratorMaintainer
-    : public MachineFunction::Delegate {
+    : 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 +193,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 +221,7 @@ bool InstructionSelect::selectMachineFunction(MachineFunction &MF) {
         LLVM_DEBUG(MIIMaintainer.reportFullyCreatedInstrs());
       }
     }
+    AllObservers.removeObserver(&MIIMaintainer);
   }
 
   for (MachineBasicBlock &MBB : MF) {