[Mlir-commits] [mlir] [mlir][SymbolTable] Handle unknown symbol users in SymbolUserMap (PR #227918)
Vinicius Silva
llvmlistbot at llvm.org
Tue Oct 6 04:49:01 PDT 2026
https://github.com/viniciusfdasilva updated https://github.com/llvm/llvm-project/pull/227918
>From 046d0ee8c67382381658ae9540b24067ac8873e6 Mon Sep 17 00:00:00 2001
From: Vinicius Silva <viniciusilva at ieee.org>
Date: Wed, 30 Sep 2026 21:48:46 -0300
Subject: [PATCH 1/7] [mlir][SymbolTable] Handle unknown symbol users in
SymbolUserMap
---
mlir/include/mlir/IR/SymbolTable.h | 3 +++
mlir/lib/IR/SymbolTable.cpp | 15 +++++++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
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
>From 7b334b434a686351bd685a1acd00a6f6bbf4c846 Mon Sep 17 00:00:00 2001
From: Vinicius Silva <viniciusilva at ieee.org>
Date: Thu, 1 Oct 2026 15:08:07 -0300
Subject: [PATCH 2/7] Formating source code using
---
mlir/lib/IR/SymbolTable.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index b02bcc2db17d8..80ce4139fe1b1 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1059,7 +1059,7 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
symbolsWithAllUsesVisible.insert(&nestedOp);
}
auto symbolUses = SymbolTable::getSymbolUses(&nestedOp);
-
+
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.
@@ -1082,8 +1082,9 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
}
bool SymbolUserMap::areAllUsesVisible(Operation *symbol) const {
- for (Operation *parent = symbol->getParentOp(); parent; parent = parent->getParentOp()){
- if(symbolTablesWithUnknownUsers.contains(parent)){
+ for (Operation *parent = symbol->getParentOp(); parent;
+ parent = parent->getParentOp()) {
+ if (symbolTablesWithUnknownUsers.contains(parent)) {
return false;
}
}
>From 515ff3b2107f4fa78a5975552e4005563b9e9dac Mon Sep 17 00:00:00 2001
From: Vinicius Silva <viniciusilva at ieee.org>
Date: Fri, 2 Oct 2026 09:48:50 -0300
Subject: [PATCH 3/7] Adding tests into mlir/test
---
.../remove-dead-values-unknown-op.mlir | 158 ++++++++++++++++++
1 file changed, 158 insertions(+)
create mode 100644 mlir/test/Transforms/remove-dead-values-unknown-op.mlir
diff --git a/mlir/test/Transforms/remove-dead-values-unknown-op.mlir b/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
new file mode 100644
index 0000000000000..33ac6467f00da
--- /dev/null
+++ b/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
@@ -0,0 +1,158 @@
+// RUN: mlir-opt %s -split-input-file -allow-unregistered-dialect --pass-pipeline='builtin.module(gpu.module(remove-dead-values{canonicalize=false}),builtin.module(remove-dead-values{canonicalize=false}))' | FileCheck %s
+// RUN: mlir-opt %s -split-input-file -allow-unregistered-dialect --pass-pipeline='builtin.module(gpu.module(remove-dead-values),builtin.module(remove-dead-values))' | FileCheck %s
+// RUN: mlir-opt %s -split-input-file -allow-unregistered-dialect --remove-dead-values='canonicalize=false' | FileCheck %s
+
+// An unregistered op with a region may reference any symbol visible from its
+// symbol table, so SymbolUserMap cannot compute all of its uses. It must not
+// crash the pass, and symbols it may use must be treated conservatively.
+// See https://github.com/llvm/llvm-project/issues/226052.
+//
+// Every case is wrapped in an outer module so that the nested pipelines above
+// run the pass on the inner module.
+
+// Original reproducer. The value consumed by the unknown op must stay alive.
+// CHECK-LABEL: func.func @bbarg_of_unknown_op_2(
+// CHECK-SAME: %{{[^ ,)]+}}: f32)
+// CHECK: tensor.empty
+// CHECK: linalg.fill
+// CHECK: "dummy.dummy_op"
+module {
+ module {
+ func.func @bbarg_of_unknown_op_2(%arg0: f32) {
+ %0 = tensor.empty() : tensor<10xf32>
+ %1 = linalg.fill ins(%arg0 : f32) outs(%0 : tensor<10xf32>) -> tensor<10xf32>
+ "dummy.dummy_op"(%1) ({
+ }) : (tensor<10xf32>) -> ()
+ }
+ }
+}
+
+// -----
+
+// The unknown op is a direct child of the symbol table, next to the function.
+// CHECK-LABEL: func.func @unknown_op_at_module_level(
+// CHECK-SAME: %{{[^ ,)]+}}: f32) -> tensor<10xf32>
+// CHECK: tensor.empty
+// CHECK: linalg.fill
+// CHECK: "dummy.top_level_op"
+module {
+ module {
+ func.func @unknown_op_at_module_level(%arg0: f32) -> tensor<10xf32> {
+ %0 = tensor.empty() : tensor<10xf32>
+ %1 = linalg.fill ins(%arg0 : f32) outs(%0 : tensor<10xf32>) -> tensor<10xf32>
+ return %1 : tensor<10xf32>
+ }
+ "dummy.top_level_op"() ({
+ }) : () -> ()
+ }
+}
+
+// -----
+
+// The unknown op has a region with a block argument and a body.
+// CHECK-LABEL: func.func @unknown_op_with_block_arg(
+// CHECK-SAME: %{{[^ ,)]+}}: f32)
+// CHECK: "dummy.dummy_op"
+// CHECK-NEXT: ^bb0(%{{[^ ,)]+}}: tensor<10xf32>):
+// CHECK-NEXT: "dummy.yield"
+module {
+ module {
+ func.func @unknown_op_with_block_arg(%arg0: f32) {
+ %0 = tensor.empty() : tensor<10xf32>
+ %1 = linalg.fill ins(%arg0 : f32) outs(%0 : tensor<10xf32>) -> tensor<10xf32>
+ "dummy.dummy_op"(%1) ({
+ ^bb0(%arg1: tensor<10xf32>):
+ "dummy.yield"(%arg1) : (tensor<10xf32>) -> ()
+ }) : (tensor<10xf32>) -> ()
+ return
+ }
+ }
+}
+
+// -----
+
+// Dead values unrelated to the unknown op are still removed.
+// CHECK-LABEL: func.func @dead_value_next_to_unknown_op(
+// CHECK-SAME: %{{[^ ,)]+}}: f32)
+// CHECK-NOT: tensor.empty
+// CHECK-NOT: linalg.fill
+// CHECK: "dummy.dummy_op"
+module {
+ module {
+ func.func @dead_value_next_to_unknown_op(%arg0: f32) {
+ %0 = tensor.empty() : tensor<10xf32>
+ %1 = linalg.fill ins(%arg0 : f32) outs(%0 : tensor<10xf32>) -> tensor<10xf32>
+ "dummy.dummy_op"() ({
+ }) : () -> ()
+ return
+ }
+ }
+}
+
+// -----
+
+// Control, no unknown op: the dead argument of a private callee is removed.
+// CHECK-LABEL: func.func private @callee_control(
+// CHECK-SAME: %{{[^ ,)]+}}: i32) -> i32
+// CHECK-LABEL: func.func @caller_control(
+// CHECK: call @callee_control(%{{[^ ,)]+}}) : (i32) -> i32
+module {
+ module {
+ func.func private @callee_control(%used: i32, %dead: i32) -> i32 {
+ return %used : i32
+ }
+ func.func @caller_control(%x: i32) -> i32 {
+ %r = func.call @callee_control(%x, %x) : (i32, i32) -> i32
+ return %r : i32
+ }
+ }
+}
+
+// -----
+
+// Same as the control plus an unknown op with a region. Not all users of the
+// callee are known, so keep its signature, including the unused argument.
+// CHECK-LABEL: func.func private @callee_unknown_user(
+// CHECK-SAME: %{{[^ ,)]+}}: i32, %{{[^ ,)]+}}: i32) -> i32
+// CHECK-LABEL: func.func @caller_unknown_user(
+// CHECK: call @callee_unknown_user(%{{[^ ,)]+}}, %{{[^ ,)]+}}) : (i32, i32) -> i32
+// CHECK: "dummy.dummy_op"
+module {
+ module {
+ func.func private @callee_unknown_user(%used: i32, %dead: i32) -> i32 {
+ return %used : i32
+ }
+ func.func @caller_unknown_user(%x: i32) -> i32 {
+ %r = func.call @callee_unknown_user(%x, %x) : (i32, i32) -> i32
+ "dummy.dummy_op"() ({
+ }) : () -> ()
+ return %r : i32
+ }
+ }
+}
+
+// -----
+
+// The unknown op lives in a symbol table enclosing the callee, and that table
+// is inside the pass root. The enclosing table must be taken into account when
+// deciding that all uses are visible.
+// CHECK-LABEL: module @outer
+// CHECK: func.func private @callee_nested(
+// CHECK-SAME: %{{[^ ,)]+}}: i32, %{{[^ ,)]+}}: i32) -> i32
+// CHECK: call @callee_nested(%{{[^ ,)]+}}, %{{[^ ,)]+}}) : (i32, i32) -> i32
+// CHECK: "dummy.dummy_op"
+module {
+ module @outer {
+ module @inner {
+ func.func private @callee_nested(%used: i32, %dead: i32) -> i32 {
+ return %used : i32
+ }
+ func.func @caller_nested(%x: i32) -> i32 {
+ %r = func.call @callee_nested(%x, %x) : (i32, i32) -> i32
+ return %r : i32
+ }
+ }
+ "dummy.dummy_op"() ({
+ }) : () -> ()
+ }
+}
\ No newline at end of file
>From a8178eb21c47cbfedb18a4d2e103dfea8f2880fd Mon Sep 17 00:00:00 2001
From: Vinicius Silva <viniciusilva at ieee.org>
Date: Fri, 2 Oct 2026 09:50:06 -0300
Subject: [PATCH 4/7] Formatting SymbolTable.cpp using clang-format
---
mlir/lib/IR/SymbolTable.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 80ce4139fe1b1..c214a0f298d8e 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1059,7 +1059,6 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
symbolsWithAllUsesVisible.insert(&nestedOp);
}
auto symbolUses = SymbolTable::getSymbolUses(&nestedOp);
-
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.
@@ -1082,12 +1081,14 @@ 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;
>From 51e1e6a7ed2b2cc2f9e4284052e3a55a22e760d0 Mon Sep 17 00:00:00 2001
From: Vinicius Silva <viniciusilva at ieee.org>
Date: Mon, 5 Oct 2026 20:48:47 -0300
Subject: [PATCH 5/7] Adding new line into remove-dead-values-unknown-op.mlir
file
---
mlir/test/Transforms/remove-dead-values-unknown-op.mlir | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/test/Transforms/remove-dead-values-unknown-op.mlir b/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
index 33ac6467f00da..03d17137c6566 100644
--- a/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
+++ b/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
@@ -155,4 +155,4 @@ module {
"dummy.dummy_op"() ({
}) : () -> ()
}
-}
\ No newline at end of file
+}
>From 1df0367f112475dc48b99a7e9174339fadd4a501 Mon Sep 17 00:00:00 2001
From: Vinicius Silva <viniciusilva at ieee.org>
Date: Mon, 5 Oct 2026 21:15:52 -0300
Subject: [PATCH 6/7] Adding safeguard through the issue symbolUses null case
---
mlir/lib/IR/SymbolTable.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index c214a0f298d8e..3183104033441 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1082,10 +1082,12 @@ 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;
+ if (!symbolTablesWithUnknownUsers.empty()) {
+ for (Operation *parent = symbol->getParentOp(); parent;
+ parent = parent->getParentOp()) {
+ if (symbolTablesWithUnknownUsers.contains(parent)) {
+ return false;
+ }
}
}
>From 285a85e0d7e33fa602677ed34461ffa862af3cfc Mon Sep 17 00:00:00 2001
From: Vinicius Silva <viniciusilva at ieee.org>
Date: Tue, 6 Oct 2026 08:48:41 -0300
Subject: [PATCH 7/7] Refine comments on handling unregistered ops
---
.../Transforms/remove-dead-values-unknown-op.mlir | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/mlir/test/Transforms/remove-dead-values-unknown-op.mlir b/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
index 03d17137c6566..f49145eb8788b 100644
--- a/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
+++ b/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
@@ -2,15 +2,8 @@
// RUN: mlir-opt %s -split-input-file -allow-unregistered-dialect --pass-pipeline='builtin.module(gpu.module(remove-dead-values),builtin.module(remove-dead-values))' | FileCheck %s
// RUN: mlir-opt %s -split-input-file -allow-unregistered-dialect --remove-dead-values='canonicalize=false' | FileCheck %s
-// An unregistered op with a region may reference any symbol visible from its
-// symbol table, so SymbolUserMap cannot compute all of its uses. It must not
-// crash the pass, and symbols it may use must be treated conservatively.
-// See https://github.com/llvm/llvm-project/issues/226052.
-//
-// Every case is wrapped in an outer module so that the nested pipelines above
-// run the pass on the inner module.
-
-// Original reproducer. The value consumed by the unknown op must stay alive.
+// Unregistered ops may reference any visible symbol, so handle them conservatively.
+// The value consumed by the unknown op must stay alive.
// CHECK-LABEL: func.func @bbarg_of_unknown_op_2(
// CHECK-SAME: %{{[^ ,)]+}}: f32)
// CHECK: tensor.empty
More information about the Mlir-commits
mailing list