[Mlir-commits] [mlir] [mlir] `remove-dead-values`: properly handle symbol users outside of a pass root. (PR #225355)
Ivan Butygin
llvmlistbot at llvm.org
Tue Sep 22 03:35:22 PDT 2026
https://github.com/Hardcode84 updated https://github.com/llvm/llvm-project/pull/225355
>From b179575ebae52fff240e9582610f42261ac300e0 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Mon, 21 Sep 2026 11:51:00 +0200
Subject: [PATCH 1/7] [mlir] Preserve function signatures when symbol users are
outside the pass root
Add SymbolUserMap::areAllUsesVisible using symbol-table visibility. Require a complete user map before RemoveDeadValues changes a function signature. Nested symbols can have callers outside the pass root even when the local user list is empty.
Test GPU launches outside the root, local and outside calls, nested tables, and signature cleanup in complete scopes.
---
mlir/include/mlir/IR/SymbolTable.h | 16 ++-
mlir/lib/IR/SymbolTable.cpp | 17 ++-
mlir/lib/Transforms/RemoveDeadValues.cpp | 8 +-
.../remove-dead-values-symbol-scope.mlir | 103 ++++++++++++++++++
mlir/unittests/IR/SymbolTableTest.cpp | 65 +++++++++++
5 files changed, 198 insertions(+), 11 deletions(-)
create mode 100644 mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index a6d8e8660cd22..858ee4e5f588a 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -407,19 +407,26 @@ class SymbolUserMap {
/// extend beyond that of this map.
SymbolUserMap(SymbolTableCollection &symbolTable, Operation *symbolTableOp);
- /// Return the users of the provided symbol operation.
+ /// Return the users of the provided symbol operation within this map's scope.
ArrayRef<Operation *> getUsers(Operation *symbol) const {
auto it = symbolToUsers.find(symbol);
return it != symbolToUsers.end() ? it->second.getArrayRef()
: ArrayRef<Operation *>();
}
- /// Return true if the given symbol has no uses.
+ /// Return true if all uses of the symbol are visible within this map's scope.
+ /// Public symbols can have users outside the IR, and nested symbols can have
+ /// users outside the scope. Return false if the symbol's table is not in the
+ /// map. Changes to symbol visibility or table nesting invalidate this query.
+ bool areAllUsesVisible(Operation *symbol) const;
+
+ /// Return true if the given symbol has no uses within this map's scope.
bool useEmpty(Operation *symbol) const {
return !symbolToUsers.count(symbol);
}
- /// Replace all of the uses of the given symbol with `newSymbolName`.
+ /// Replace all uses of the symbol within this map's scope with
+ /// `newSymbolName`.
void replaceAllUsesWith(Operation *symbol, StringAttr newSymbolName);
private:
@@ -428,6 +435,9 @@ class SymbolUserMap {
/// A map of symbol operations to symbol users.
DenseMap<Operation *, SetVector<Operation *>> symbolToUsers;
+
+ /// Whether all uses of nested-visibility symbols in each table are visible.
+ DenseMap<Operation *, bool> allUsesVisible;
};
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index b0288185ae0ab..764ff4b3a60e8 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1050,9 +1050,10 @@ LockedSymbolTableCollection::getSymbolTable(Operation *symbolTableOp) {
SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
Operation *symbolTableOp)
: symbolTable(symbolTable) {
- // Walk each of the symbol tables looking for discardable callgraph nodes.
+ // Collect symbol users and visibility within each symbol table.
SmallVector<Operation *> symbols;
auto walkFn = [&](Operation *symbolTableOp, bool allUsesVisible) {
+ this->allUsesVisible[symbolTableOp] = allUsesVisible;
for (Operation &nestedOp : symbolTableOp->getRegion(0).getOps()) {
auto symbolUses = SymbolTable::getSymbolUses(&nestedOp);
assert(symbolUses && "expected uses to be valid");
@@ -1066,10 +1067,16 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
}
}
};
- // We just set `allSymUsesVisible` to false here because it isn't necessary
- // for building the user map.
- SymbolTable::walkSymbolTables(symbolTableOp, /*allSymUsesVisible=*/false,
- walkFn);
+ SymbolTable::walkSymbolTables(
+ symbolTableOp, /*allSymUsesVisible=*/!symbolTableOp->getBlock(), walkFn);
+}
+
+bool SymbolUserMap::areAllUsesVisible(Operation *symbol) const {
+ auto it = allUsesVisible.find(symbol->getParentOp());
+ if (it == allUsesVisible.end())
+ return false;
+ auto symbolOp = cast<SymbolOpInterface>(symbol);
+ return symbolOp.isPrivate() || (symbolOp.isNested() && it->second);
}
void SymbolUserMap::replaceAllUsesWith(Operation *symbol,
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index fe7eed7ce54c6..4b574a518d542 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -259,7 +259,7 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la,
}
/// Process a function-like operation `funcOp` using the liveness analysis `la`
-/// and `symbolUserMap`. If it is not public or external:
+/// and `symbolUserMap`. If it has a body and all symbol users are visible:
/// (1) Adding its non-live arguments to a list for future removal.
/// (2) Marking their corresponding operands in its callers for removal.
/// (3) Identifying and enqueueing unnecessary terminator operands
@@ -275,8 +275,10 @@ static void processFuncOp(FunctionOpInterface funcOp,
LDBG() << "Processing function op: "
<< OpWithFlags(funcOp,
OpPrintingFlags().skipRegions().printGenericOpForm());
- if (funcOp.isPublic() || funcOp.isExternal()) {
- LDBG() << "Function is public or external, skipping: "
+ // A nested symbol can have callers outside the pass root. Its signature must
+ // stay unchanged even if all users in the map are calls, or the map is empty.
+ if (funcOp.isExternal() || !symbolUserMap.areAllUsesVisible(funcOp)) {
+ LDBG() << "Function is external or has unknown users, skipping: "
<< funcOp.getOperation()->getName();
return;
}
diff --git a/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir b/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
new file mode 100644
index 0000000000000..37af6b65816e5
--- /dev/null
+++ b/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
@@ -0,0 +1,103 @@
+// RUN: mlir-opt %s -split-input-file --pass-pipeline='builtin.module(gpu.module(remove-dead-values{canonicalize=false}),builtin.module(remove-dead-values{canonicalize=false}))' | FileCheck %s --check-prefixes=CHECK,SCOPED
+// RUN: mlir-opt %s -split-input-file --pass-pipeline='builtin.module(gpu.module(remove-dead-values),builtin.module(remove-dead-values))' > %t.parallel
+// RUN: FileCheck %s --check-prefixes=CHECK,SCOPED < %t.parallel
+// RUN: mlir-opt %s -split-input-file --mlir-disable-threading --pass-pipeline='builtin.module(gpu.module(remove-dead-values),builtin.module(remove-dead-values))' > %t.serial
+// RUN: diff %t.parallel %t.serial
+// RUN: mlir-opt %s -split-input-file --remove-dead-values='canonicalize=false' | FileCheck %s --check-prefixes=CHECK,FULL
+
+// The launch is outside the pass root. An empty user map does not mean that the
+// kernel has no users. Keep its signature, including the unused argument.
+module attributes {gpu.container_module} {
+ gpu.module @kernels {
+ // CHECK-LABEL: gpu.func @kernel(
+ // CHECK-SAME: %{{[^ ,)]+}}: i32, %{{[^ ,)]+}}: i32) kernel
+ gpu.func @kernel(%used: i32, %dead: i32) kernel attributes {sym_visibility = "nested"} {
+ gpu.printf "value: %d", %used : i32
+ gpu.return
+ }
+ }
+ func.func @launch(%x: i32) {
+ %c1 = arith.constant 1 : index
+ // CHECK: gpu.launch_func @kernels::@kernel
+ // CHECK-SAME: args(%{{[^ ,)]+}} : i32, %{{[^ ,)]+}} : i32)
+ gpu.launch_func @kernels::@kernel blocks in (%c1, %c1, %c1)
+ threads in (%c1, %c1, %c1) args(%x : i32, %x : i32)
+ return
+ }
+}
+
+// -----
+
+module @outer {
+ module @exposed {
+ // A local call does not make the user map complete. Keep both arguments and
+ // results when the outer call is outside the pass root.
+ // SCOPED-LABEL: func.func nested @callee(
+ // SCOPED-SAME: %{{[^ ,)]+}}: i32, %{{[^ ,)]+}}: i32) -> (i32, i32)
+ // FULL-LABEL: func.func nested @callee(
+ // FULL-SAME: %{{[^ ,)]+}}: i32) -> i32
+ func.func nested @callee(%used: i32, %dead: i32) -> (i32, i32) {
+ return %used, %used : i32, i32
+ }
+ func.func @local(%x: i32) {
+ // SCOPED: call @callee(%{{[^ ,)]+}}, %{{[^ ,)]+}}) : (i32, i32) -> (i32, i32)
+ // FULL: call @callee(%{{[^ ,)]+}}) : (i32) -> i32
+ %r:2 = func.call @callee(%x, %x) : (i32, i32) -> (i32, i32)
+ return
+ }
+
+ // Visibility must propagate through all exposed tables below the root.
+ module @child attributes {sym_visibility = "nested"} {
+ // SCOPED-LABEL: func.func nested @deep(
+ // SCOPED-SAME: %{{[^ ,)]+}}: i32)
+ // FULL-LABEL: func.func nested @deep()
+ func.func nested @deep(%dead: i32) {
+ return
+ }
+ }
+
+ // Private symbols have no callers outside their table.
+ // CHECK-LABEL: func.func private @private_callee()
+ func.func private @private_callee(%dead: i32) {
+ return
+ }
+ func.func @private_caller(%x: i32) {
+ // CHECK: call @private_callee() : () -> ()
+ func.call @private_callee(%x) : (i32) -> ()
+ return
+ }
+
+ // A private table hides the nested symbols below it.
+ module @hidden attributes {sym_visibility = "private"} {
+ // CHECK-LABEL: func.func nested @hidden_callee()
+ // CHECK-NEXT: return
+ func.func nested @hidden_callee(%dead: i32) -> i32 {
+ %c1 = arith.constant 1 : i32
+ return %c1 : i32
+ }
+ func.func @hidden_caller(%x: i32) {
+ // CHECK: call @hidden_callee() : () -> ()
+ %r = func.call @hidden_callee(%x) : (i32) -> i32
+ return
+ }
+ }
+ }
+ func.func @outside(%x: i32) -> i32 {
+ // SCOPED: "test.conversion_call_op"(%{{[^ ,)]+}}, %{{[^ ,)]+}})
+ // SCOPED-SAME: callee = @exposed::@callee
+ // SCOPED-SAME: (i32, i32) -> (i32, i32)
+ // FULL: "test.conversion_call_op"(%{{[^ ,)]+}})
+ // FULL-SAME: callee = @exposed::@callee
+ // FULL-SAME: (i32) -> i32
+ %r:2 = "test.conversion_call_op"(%x, %x) {callee = @exposed::@callee}
+ : (i32, i32) -> (i32, i32)
+ // SCOPED: "test.conversion_call_op"(%{{[^ ,)]+}})
+ // SCOPED-SAME: callee = @exposed::@child::@deep
+ // SCOPED-SAME: (i32) -> ()
+ // FULL: "test.conversion_call_op"()
+ // FULL-SAME: callee = @exposed::@child::@deep
+ // FULL-SAME: () -> ()
+ "test.conversion_call_op"(%x) {callee = @exposed::@child::@deep} : (i32) -> ()
+ return %r#0 : i32
+ }
+}
diff --git a/mlir/unittests/IR/SymbolTableTest.cpp b/mlir/unittests/IR/SymbolTableTest.cpp
index fd96467e7c76e..d4431e295d23f 100644
--- a/mlir/unittests/IR/SymbolTableTest.cpp
+++ b/mlir/unittests/IR/SymbolTableTest.cpp
@@ -203,4 +203,69 @@ TEST(SymbolOpInterface, Visibility) {
symOp->hasAttr(SymbolOpInterface::getDefaultVisibilityAttrName()));
}
+TEST(SymbolUserMap, AllUsesVisible) {
+ DialectRegistry registry;
+ ::test::registerTestDialect(registry);
+ MLIRContext context(registry);
+
+ constexpr static StringLiteral kInput = R"MLIR(
+ module @root {
+ module @exposed {
+ "test.symbol"() <{sym_name = "public"}> : () -> ()
+ "test.symbol"() <{sym_name = "private", sym_visibility = "private"}> : () -> ()
+ "test.symbol"() <{sym_name = "nested", sym_visibility = "nested"}> : () -> ()
+ "test.symbol"() <{sym_name = "local_user"}> {use = @nested} : () -> ()
+ module @child attributes {sym_visibility = "nested"} {
+ "test.symbol"() <{sym_name = "leaf", sym_visibility = "nested"}> : () -> ()
+ }
+ module @hidden attributes {sym_visibility = "private"} {
+ "test.symbol"() <{sym_name = "leaf", sym_visibility = "nested"}> : () -> ()
+ }
+ }
+ "test.symbol"() <{sym_name = "outside", sym_visibility = "private"}>
+ {use = @exposed::@nested} : () -> ()
+ }
+ )MLIR";
+ OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(kInput, &context);
+ ASSERT_TRUE(module);
+ SymbolTableCollection tables;
+ Operation *exposed = SymbolTable::lookupSymbolIn(*module, "exposed");
+ Operation *publicSymbol = SymbolTable::lookupSymbolIn(exposed, "public");
+ Operation *privateSymbol = SymbolTable::lookupSymbolIn(exposed, "private");
+ Operation *nested = SymbolTable::lookupSymbolIn(exposed, "nested");
+ Operation *localUser = SymbolTable::lookupSymbolIn(exposed, "local_user");
+ Operation *outside = SymbolTable::lookupSymbolIn(*module, "outside");
+ Operation *child = SymbolTable::lookupSymbolIn(exposed, "child");
+ Operation *childLeaf = SymbolTable::lookupSymbolIn(child, "leaf");
+ Operation *hidden = SymbolTable::lookupSymbolIn(exposed, "hidden");
+ Operation *hiddenLeaf = SymbolTable::lookupSymbolIn(hidden, "leaf");
+
+ // A detached, named root contains all IR users. Public symbols can still have
+ // users outside the IR.
+ SymbolUserMap wholeMap(tables, *module);
+ EXPECT_FALSE(wholeMap.areAllUsesVisible(publicSymbol));
+ EXPECT_TRUE(wholeMap.areAllUsesVisible(privateSymbol));
+ EXPECT_TRUE(wholeMap.areAllUsesVisible(nested));
+ EXPECT_TRUE(wholeMap.areAllUsesVisible(childLeaf));
+ EXPECT_TRUE(wholeMap.areAllUsesVisible(hiddenLeaf));
+ EXPECT_EQ(wholeMap.getUsers(nested).size(), 2u);
+
+ // A map of an exposed table omits outside users, even with local users.
+ SymbolUserMap exposedMap(tables, exposed);
+ EXPECT_FALSE(exposedMap.areAllUsesVisible(publicSymbol));
+ EXPECT_TRUE(exposedMap.areAllUsesVisible(privateSymbol));
+ EXPECT_FALSE(exposedMap.areAllUsesVisible(nested));
+ ASSERT_EQ(exposedMap.getUsers(nested).size(), 1u);
+ EXPECT_EQ(exposedMap.getUsers(nested).front(), localUser);
+ EXPECT_FALSE(exposedMap.areAllUsesVisible(childLeaf));
+ EXPECT_TRUE(exposedMap.useEmpty(childLeaf));
+ EXPECT_TRUE(exposedMap.areAllUsesVisible(hiddenLeaf));
+ EXPECT_FALSE(exposedMap.areAllUsesVisible(outside));
+ EXPECT_FALSE(exposedMap.areAllUsesVisible(exposed));
+
+ // A private table hides its nested symbols even when it is the map root.
+ SymbolUserMap hiddenMap(tables, hidden);
+ EXPECT_TRUE(hiddenMap.areAllUsesVisible(hiddenLeaf));
+}
+
} // namespace
>From a06acf714dfd6840459ab2a6b351e5a5c24d543f Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Tue, 22 Sep 2026 11:26:31 +0200
Subject: [PATCH 2/7] [mlir] Use a set for symbols with fully visible uses
Collect eligible symbols during the existing SymbolUserMap walk. The visibility query becomes a set lookup and still rejects symbols outside the map scope.
---
mlir/include/mlir/IR/SymbolTable.h | 9 ++++++---
mlir/lib/IR/SymbolTable.cpp | 13 ++++---------
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index 858ee4e5f588a..1960c2942b2eb 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -11,6 +11,7 @@
#include "mlir/IR/Attributes.h"
#include "mlir/IR/OpDefinition.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/RWMutex.h"
@@ -418,7 +419,9 @@ class SymbolUserMap {
/// Public symbols can have users outside the IR, and nested symbols can have
/// users outside the scope. Return false if the symbol's table is not in the
/// map. Changes to symbol visibility or table nesting invalidate this query.
- bool areAllUsesVisible(Operation *symbol) const;
+ bool areAllUsesVisible(Operation *symbol) const {
+ return symbolsWithAllUsesVisible.contains(symbol);
+ }
/// Return true if the given symbol has no uses within this map's scope.
bool useEmpty(Operation *symbol) const {
@@ -436,8 +439,8 @@ class SymbolUserMap {
/// A map of symbol operations to symbol users.
DenseMap<Operation *, SetVector<Operation *>> symbolToUsers;
- /// Whether all uses of nested-visibility symbols in each table are visible.
- DenseMap<Operation *, bool> allUsesVisible;
+ /// Symbols whose uses are all visible within this map's scope.
+ DenseSet<Operation *> symbolsWithAllUsesVisible;
};
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 764ff4b3a60e8..4f611f8987814 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1053,8 +1053,11 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
// Collect symbol users and visibility within each symbol table.
SmallVector<Operation *> symbols;
auto walkFn = [&](Operation *symbolTableOp, bool allUsesVisible) {
- this->allUsesVisible[symbolTableOp] = allUsesVisible;
for (Operation &nestedOp : symbolTableOp->getRegion(0).getOps()) {
+ if (auto symbol = dyn_cast<SymbolOpInterface>(nestedOp)) {
+ if (symbol.isPrivate() || (symbol.isNested() && allUsesVisible))
+ symbolsWithAllUsesVisible.insert(&nestedOp);
+ }
auto symbolUses = SymbolTable::getSymbolUses(&nestedOp);
assert(symbolUses && "expected uses to be valid");
@@ -1071,14 +1074,6 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
symbolTableOp, /*allSymUsesVisible=*/!symbolTableOp->getBlock(), walkFn);
}
-bool SymbolUserMap::areAllUsesVisible(Operation *symbol) const {
- auto it = allUsesVisible.find(symbol->getParentOp());
- if (it == allUsesVisible.end())
- return false;
- auto symbolOp = cast<SymbolOpInterface>(symbol);
- return symbolOp.isPrivate() || (symbolOp.isNested() && it->second);
-}
-
void SymbolUserMap::replaceAllUsesWith(Operation *symbol,
StringAttr newSymbolName) {
auto it = symbolToUsers.find(symbol);
>From 218b2bbfccbec1038b9875754e268236648907b1 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Tue, 22 Sep 2026 11:39:14 +0200
Subject: [PATCH 3/7] [mlir] Explain initial symbol use visibility
---
mlir/lib/IR/SymbolTable.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 4f611f8987814..f14850ca1e6b8 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1070,6 +1070,7 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
}
}
};
+ // A root with no containing block has no symbol users in enclosing IR.
SymbolTable::walkSymbolTables(
symbolTableOp, /*allSymUsesVisible=*/!symbolTableOp->getBlock(), walkFn);
}
>From 9ef803c49c6cb7f1241e45ad1eabd46124a00d95 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Tue, 22 Sep 2026 11:45:33 +0200
Subject: [PATCH 4/7] [mlir] Shorten symbol use visibility documentation
---
mlir/include/mlir/IR/SymbolTable.h | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index 1960c2942b2eb..e7c6bda994088 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -415,10 +415,7 @@ class SymbolUserMap {
: ArrayRef<Operation *>();
}
- /// Return true if all uses of the symbol are visible within this map's scope.
- /// Public symbols can have users outside the IR, and nested symbols can have
- /// users outside the scope. Return false if the symbol's table is not in the
- /// map. Changes to symbol visibility or table nesting invalidate this query.
+ /// Return true if all uses of the symbol are known to be within this scope.
bool areAllUsesVisible(Operation *symbol) const {
return symbolsWithAllUsesVisible.contains(symbol);
}
>From 5bdcaed383ec374a88d7675a80142d4e63e09fe1 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Tue, 22 Sep 2026 11:50:08 +0200
Subject: [PATCH 5/7] [mlir] Explain private symbol use visibility
---
mlir/lib/IR/SymbolTable.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index f14850ca1e6b8..09d585ef4b9b3 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1055,6 +1055,7 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
auto walkFn = [&](Operation *symbolTableOp, bool allUsesVisible) {
for (Operation &nestedOp : symbolTableOp->getRegion(0).getOps()) {
if (auto symbol = dyn_cast<SymbolOpInterface>(nestedOp)) {
+ // Private symbols can only have users within this table.
if (symbol.isPrivate() || (symbol.isNested() && allUsesVisible))
symbolsWithAllUsesVisible.insert(&nestedOp);
}
>From 786465a02d00a0cf77ea0d42e43444dcd681a1b4 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Tue, 22 Sep 2026 12:05:27 +0200
Subject: [PATCH 6/7] [mlir] Clarify the signature preservation comment
---
mlir/lib/Transforms/RemoveDeadValues.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 4b574a518d542..5d173c0f6453f 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -275,8 +275,7 @@ static void processFuncOp(FunctionOpInterface funcOp,
LDBG() << "Processing function op: "
<< OpWithFlags(funcOp,
OpPrintingFlags().skipRegions().printGenericOpForm());
- // A nested symbol can have callers outside the pass root. Its signature must
- // stay unchanged even if all users in the map are calls, or the map is empty.
+ // Preserve the signature if callers may exist outside the pass root.
if (funcOp.isExternal() || !symbolUserMap.areAllUsesVisible(funcOp)) {
LDBG() << "Function is external or has unknown users, skipping: "
<< funcOp.getOperation()->getName();
>From 246515a1182dca204d28a290579e956d7af12f73 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Tue, 22 Sep 2026 12:15:32 +0200
Subject: [PATCH 7/7] [mlir] Report completeness of symbol users within the IR
Let SymbolUserMap report complete IR use lists for public symbols when their enclosing scope is covered. Keep the public visibility restriction in RemoveDeadValues. Test public symbols with local and outside IR users and preserve public function signatures during cleanup.
---
mlir/include/mlir/IR/SymbolTable.h | 2 +-
mlir/lib/IR/SymbolTable.cpp | 2 +-
mlir/lib/Transforms/RemoveDeadValues.cpp | 8 +++++---
.../remove-dead-values-symbol-scope.mlir | 14 ++++++++++++++
mlir/unittests/IR/SymbolTableTest.cpp | 17 ++++++++++++-----
5 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index e7c6bda994088..1a15d261c7229 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -415,7 +415,7 @@ class SymbolUserMap {
: ArrayRef<Operation *>();
}
- /// Return true if all uses of the symbol are known to be within this scope.
+ /// Return true if all uses of the symbol within the IR are within this scope.
bool areAllUsesVisible(Operation *symbol) const {
return symbolsWithAllUsesVisible.contains(symbol);
}
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 09d585ef4b9b3..a6601b0e39088 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1056,7 +1056,7 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
for (Operation &nestedOp : symbolTableOp->getRegion(0).getOps()) {
if (auto symbol = dyn_cast<SymbolOpInterface>(nestedOp)) {
// Private symbols can only have users within this table.
- if (symbol.isPrivate() || (symbol.isNested() && allUsesVisible))
+ if (symbol.isPrivate() || allUsesVisible)
symbolsWithAllUsesVisible.insert(&nestedOp);
}
auto symbolUses = SymbolTable::getSymbolUses(&nestedOp);
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 5d173c0f6453f..0b9321a265221 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -259,7 +259,8 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la,
}
/// Process a function-like operation `funcOp` using the liveness analysis `la`
-/// and `symbolUserMap`. If it has a body and all symbol users are visible:
+/// and `symbolUserMap`. If it is not public or external and all users are
+/// visible:
/// (1) Adding its non-live arguments to a list for future removal.
/// (2) Marking their corresponding operands in its callers for removal.
/// (3) Identifying and enqueueing unnecessary terminator operands
@@ -276,8 +277,9 @@ static void processFuncOp(FunctionOpInterface funcOp,
<< OpWithFlags(funcOp,
OpPrintingFlags().skipRegions().printGenericOpForm());
// Preserve the signature if callers may exist outside the pass root.
- if (funcOp.isExternal() || !symbolUserMap.areAllUsesVisible(funcOp)) {
- LDBG() << "Function is external or has unknown users, skipping: "
+ if (funcOp.isPublic() || funcOp.isExternal() ||
+ !symbolUserMap.areAllUsesVisible(funcOp)) {
+ LDBG() << "Function is public, external, or has unknown users, skipping: "
<< funcOp.getOperation()->getName();
return;
}
diff --git a/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir b/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
index 37af6b65816e5..057225ef44d53 100644
--- a/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
+++ b/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
@@ -56,6 +56,20 @@ module @outer {
}
}
+ // Public functions can have callers outside the IR, even if all IR users
+ // are visible to the pass.
+ // CHECK-LABEL: func.func @public_callee(
+ // CHECK-SAME: %{{[^ ,)]+}}: i32) -> i32
+ func.func @public_callee(%dead: i32) -> i32 {
+ %c1 = arith.constant 1 : i32
+ return %c1 : i32
+ }
+ func.func @public_caller(%x: i32) {
+ // CHECK: call @public_callee(%{{[^ ,)]+}}) : (i32) -> i32
+ %r = func.call @public_callee(%x) : (i32) -> i32
+ return
+ }
+
// Private symbols have no callers outside their table.
// CHECK-LABEL: func.func private @private_callee()
func.func private @private_callee(%dead: i32) {
diff --git a/mlir/unittests/IR/SymbolTableTest.cpp b/mlir/unittests/IR/SymbolTableTest.cpp
index d4431e295d23f..376ac2591e50d 100644
--- a/mlir/unittests/IR/SymbolTableTest.cpp
+++ b/mlir/unittests/IR/SymbolTableTest.cpp
@@ -214,16 +214,17 @@ TEST(SymbolUserMap, AllUsesVisible) {
"test.symbol"() <{sym_name = "public"}> : () -> ()
"test.symbol"() <{sym_name = "private", sym_visibility = "private"}> : () -> ()
"test.symbol"() <{sym_name = "nested", sym_visibility = "nested"}> : () -> ()
- "test.symbol"() <{sym_name = "local_user"}> {use = @nested} : () -> ()
+ "test.symbol"() <{sym_name = "local_user"}> {use = [@nested, @public]} : () -> ()
module @child attributes {sym_visibility = "nested"} {
"test.symbol"() <{sym_name = "leaf", sym_visibility = "nested"}> : () -> ()
}
module @hidden attributes {sym_visibility = "private"} {
"test.symbol"() <{sym_name = "leaf", sym_visibility = "nested"}> : () -> ()
+ "test.symbol"() <{sym_name = "public"}> : () -> ()
}
}
"test.symbol"() <{sym_name = "outside", sym_visibility = "private"}>
- {use = @exposed::@nested} : () -> ()
+ {use = [@exposed::@nested, @exposed::@public]} : () -> ()
}
)MLIR";
OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(kInput, &context);
@@ -239,11 +240,13 @@ TEST(SymbolUserMap, AllUsesVisible) {
Operation *childLeaf = SymbolTable::lookupSymbolIn(child, "leaf");
Operation *hidden = SymbolTable::lookupSymbolIn(exposed, "hidden");
Operation *hiddenLeaf = SymbolTable::lookupSymbolIn(hidden, "leaf");
+ Operation *hiddenPublic = SymbolTable::lookupSymbolIn(hidden, "public");
- // A detached, named root contains all IR users. Public symbols can still have
- // users outside the IR.
+ // A detached, named root contains all IR users, including public symbol
+ // users.
SymbolUserMap wholeMap(tables, *module);
- EXPECT_FALSE(wholeMap.areAllUsesVisible(publicSymbol));
+ EXPECT_TRUE(wholeMap.areAllUsesVisible(publicSymbol));
+ EXPECT_EQ(wholeMap.getUsers(publicSymbol).size(), 2u);
EXPECT_TRUE(wholeMap.areAllUsesVisible(privateSymbol));
EXPECT_TRUE(wholeMap.areAllUsesVisible(nested));
EXPECT_TRUE(wholeMap.areAllUsesVisible(childLeaf));
@@ -253,6 +256,8 @@ TEST(SymbolUserMap, AllUsesVisible) {
// A map of an exposed table omits outside users, even with local users.
SymbolUserMap exposedMap(tables, exposed);
EXPECT_FALSE(exposedMap.areAllUsesVisible(publicSymbol));
+ ASSERT_EQ(exposedMap.getUsers(publicSymbol).size(), 1u);
+ EXPECT_EQ(exposedMap.getUsers(publicSymbol).front(), localUser);
EXPECT_TRUE(exposedMap.areAllUsesVisible(privateSymbol));
EXPECT_FALSE(exposedMap.areAllUsesVisible(nested));
ASSERT_EQ(exposedMap.getUsers(nested).size(), 1u);
@@ -260,12 +265,14 @@ TEST(SymbolUserMap, AllUsesVisible) {
EXPECT_FALSE(exposedMap.areAllUsesVisible(childLeaf));
EXPECT_TRUE(exposedMap.useEmpty(childLeaf));
EXPECT_TRUE(exposedMap.areAllUsesVisible(hiddenLeaf));
+ EXPECT_TRUE(exposedMap.areAllUsesVisible(hiddenPublic));
EXPECT_FALSE(exposedMap.areAllUsesVisible(outside));
EXPECT_FALSE(exposedMap.areAllUsesVisible(exposed));
// A private table hides its nested symbols even when it is the map root.
SymbolUserMap hiddenMap(tables, hidden);
EXPECT_TRUE(hiddenMap.areAllUsesVisible(hiddenLeaf));
+ EXPECT_TRUE(hiddenMap.areAllUsesVisible(hiddenPublic));
}
} // namespace
More information about the Mlir-commits
mailing list