[llvm] 65c7213 - [GlobalISel] Don't remove from unfinalized GISelWorkList

[via llvm-commits]

Author: Tobias Stadler
Date: 2024-08-11T18:30:07+02:00
New Revision: 65c72139be5314ce4d844745d3706d46dbad77d5

URL: https://github.com/llvm/llvm-project/commit/65c72139be5314ce4d844745d3706d46dbad77d5
DIFF: https://github.com/llvm/llvm-project/commit/65c72139be5314ce4d844745d3706d46dbad77d5.diff

LOG: [GlobalISel] Don't remove from unfinalized GISelWorkList

Remove a hack from GISelWorkList caused by the Combiner removing
instructions from an unfinalized GISelWorkList during the DCE phase.
This is in preparation for larger changes to the WorkListMaintainer.

Pull Request: https://github.com/llvm/llvm-project/pull/102158

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
    llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
    llvm/lib/CodeGen/GlobalISel/Combiner.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
index cad2216db34fea..7ec5dac9a6ebaf 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
@@ -79,6 +79,9 @@ class GISelObserverWrapper : public MachineFunction::Delegate,
     if (It != Observers.end())
       Observers.erase(It);
   }
+  // Removes all observers
+  void clearObservers() { Observers.clear(); }
+
   // API for Observer.
   void erasingInstr(MachineInstr &MI) override {
     for (auto &O : Observers)

diff  --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
index 3ec6a1da201e2c..dba3a8a14480c1 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
@@ -82,7 +82,7 @@ class GISelWorkList {
   /// Remove I from the worklist if it exists.
   void remove(const MachineInstr *I) {
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
-    assert((Finalized || WorklistMap.empty()) && "Neither finalized nor empty");
+    assert(Finalized && "GISelWorkList used without finalizing");
 #endif
     auto It = WorklistMap.find(I);
     if (It == WorklistMap.end())

diff  --git a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
index 5da9e86b207618..49842e5fd65da6 100644
--- a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
@@ -110,11 +110,6 @@ Combiner::Combiner(MachineFunction &MF, CombinerInfo &CInfo,
   if (CSEInfo)
     B.setCSEInfo(CSEInfo);
 
-  // Setup observer.
-  ObserverWrapper->addObserver(WLObserver.get());
-  if (CSEInfo)
-    ObserverWrapper->addObserver(CSEInfo);
-
   B.setChangeObserver(*ObserverWrapper);
 }
 
@@ -147,6 +142,9 @@ bool Combiner::combineMachineInstrs() {
     LLVM_DEBUG(dbgs() << "\n\nCombiner iteration #" << Iteration << '\n');
 
     WorkList.clear();
+    ObserverWrapper->clearObservers();
+    if (CSEInfo)
+      ObserverWrapper->addObserver(CSEInfo);
 
     // Collect all instructions. Do a post order traversal for basic blocks and
     // insert with list bottom up, so while we pop_back_val, we'll traverse top
@@ -168,6 +166,9 @@ bool Combiner::combineMachineInstrs() {
       }
     }
     WorkList.finalize();
+
+    // Only notify WLObserver during actual combines
+    ObserverWrapper->addObserver(WLObserver.get());
     // Main Loop. Process the instructions here.
     while (!WorkList.empty()) {
       MachineInstr *CurrInst = WorkList.pop_back_val();