[Mlir-commits] [mlir] [mlir][SymbolTable] Handle unknown symbol users in SymbolUserMap (PR #227918)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Vinicius Silva (viniciusfdasilva)

<details>
<summary>Changes</summary>

This PR fixes #<!-- -->226052 treating unknown users record the symbol table having this specific users instead use the assertion.


Please see my comment here explaining where the bug was introduced! [here](https://github.com/llvm/llvm-project/issues/226052#issuecomment-5922633002).

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


2 Files Affected:

- (modified) mlir/include/mlir/IR/SymbolTable.h (+3) 
- (modified) mlir/lib/IR/SymbolTable.cpp (+13-2) 


``````````diff
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index 1e57e3ff82912..afb4b91c7d04f 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -437,6 +437,9 @@ class SymbolUserMap {
 
   /// Non-private symbols whose uses are all visible within this map's scope.
   DenseSet<Operation *> symbolsWithAllUsesVisible;
+
+  // Symbol tables for which the uses of some nested op could not be computed
+  DenseSet<Operation *> symbolTablesWithUnknownUsers;
 };
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 4b19b4b0d4113..b02bcc2db17d8 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1059,7 +1059,13 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
           symbolsWithAllUsesVisible.insert(&nestedOp);
       }
       auto symbolUses = SymbolTable::getSymbolUses(&nestedOp);
-      assert(symbolUses && "expected uses to be valid");
+      
+      if (!symbolUses) {
+        // An unknown op with a region may reference any symbol visible from
+        // this table, so we cannot know all users. Record it and move on.
+        symbolTablesWithUnknownUsers.insert(symbolTableOp);
+        continue;
+      }
 
       for (const SymbolTable::SymbolUse &use : *symbolUses) {
         symbols.clear();
@@ -1076,6 +1082,11 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
 }
 
 bool SymbolUserMap::areAllUsesVisible(Operation *symbol) const {
+  for (Operation *parent = symbol->getParentOp(); parent; parent = parent->getParentOp()){
+    if(symbolTablesWithUnknownUsers.contains(parent)){
+      return false;
+    }
+  }
   // Private symbols can only have users within their table.
   if (cast<SymbolOpInterface>(symbol).isPrivate())
     return true;
@@ -1132,4 +1143,4 @@ ParseResult impl::parseOptionalVisibilityKeyword(OpAsmParser &parser,
 
 /// Include the generated symbol interfaces.
 #include "mlir/IR/SymbolInterfaces.cpp.inc"
-#include "mlir/IR/SymbolInterfacesAttrInterface.cpp.inc"
+#include "mlir/IR/SymbolInterfacesAttrInterface.cpp.inc"
\ No newline at end of file

``````````

</details>


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