[llvm-branch-commits] [GlobalISel] Don't remove from unfinalized GISelWorkList (PR #102158)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-globalisel

Author: Tobias Stadler (tobias-stadler)

<details>
<summary>Changes</summary>

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.


---
Full diff: https://github.com/llvm/llvm-project/pull/102158.diff


3 Files Affected:

- (modified) llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h (+3) 
- (modified) llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h (+1-1) 
- (modified) llvm/lib/CodeGen/GlobalISel/Combiner.cpp (+6-5) 


``````````diff
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
index cad2216db34fe..7ec5dac9a6eba 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 3ec6a1da201e2..dba3a8a14480c 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 5da9e86b20761..49842e5fd65da 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();

``````````

</details>


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