[Mlir-commits] [mlir] 3027d41 - [mlir][SymbolTable] Handle unknown symbol users in SymbolUserMap (#227918)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Oct 21 02:56:42 PDT 2026
Author: Vinicius Silva
Date: 2026-10-21T11:56:36+02:00
New Revision: 3027d412c6e38bdd3bf7b07cc1c1ba68920d6073
URL: https://github.com/llvm/llvm-project/commit/3027d412c6e38bdd3bf7b07cc1c1ba68920d6073
DIFF: https://github.com/llvm/llvm-project/commit/3027d412c6e38bdd3bf7b07cc1c1ba68920d6073.diff
LOG: [mlir][SymbolTable] Handle unknown symbol users in SymbolUserMap (#227918)
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).
Added:
mlir/test/Transforms/remove-dead-values-unknown-op.mlir
Modified:
mlir/include/mlir/IR/SymbolTable.h
mlir/lib/IR/SymbolTable.cpp
Removed:
################################################################################
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..3183104033441c 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1059,7 +1059,12 @@ 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 +1081,16 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
}
bool SymbolUserMap::areAllUsesVisible(Operation *symbol) const {
+
+ if (!symbolTablesWithUnknownUsers.empty()) {
+ 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 +1147,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
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..f49145eb8788b5
--- /dev/null
+++ b/mlir/test/Transforms/remove-dead-values-unknown-op.mlir
@@ -0,0 +1,151 @@
+// 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
+
+// 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
+// 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"() ({
+ }) : () -> ()
+ }
+}
More information about the Mlir-commits
mailing list