[Mlir-commits] [mlir] [mlir][SymbolTable] Handle unknown symbol users in SymbolUserMap (PR #227918)
Vinicius Silva
llvmlistbot at llvm.org
Fri Oct 2 05:50:37 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/4] [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 1e57e3ff829127..afb4b91c7d04f2 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 4b19b4b0d41134..b02bcc2db17d83 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/4] 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 b02bcc2db17d83..80ce4139fe1b1f 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/4] 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 00000000000000..33ac6467f00da8
--- /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/4] 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 80ce4139fe1b1f..c214a0f298d8e6 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;
More information about the Mlir-commits
mailing list