[Mlir-commits] [mlir] [mlir] `remove-dead-values`: properly handle symbol users outside of a pass root. (PR #225355)
Ivan Butygin
llvmlistbot at llvm.org
Mon Sep 28 05:03:18 PDT 2026
https://github.com/Hardcode84 updated https://github.com/llvm/llvm-project/pull/225355
>From 7e8cc53f78e5a35668efd7bdc74b88299e07357e 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 01/10] [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 1d6cd918f56d9ee981253affa51664b992454c9b 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 02/10] [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 4437ea80ed49d98dd5fd3d4479d2cd4ad116a8b4 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 03/10] [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 1f848ff01e562d255402b039c32f6b099a5ce106 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 04/10] [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 74f0adaa4c39753462295ef52f69f914df30876c 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 05/10] [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 450b4bca41bc56f5caea18ca2044fb58a0314db8 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 06/10] [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 17701a821dcadf676695e47586984c0e73e620dc 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 07/10] [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
>From 792b0d0e6ad20bf751db73cc18d2684cd5c477d8 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Tue, 22 Sep 2026 13:03:49 +0200
Subject: [PATCH 08/10] [mlir] Remove redundant threading comparison from
symbol scope test
---
mlir/test/Transforms/remove-dead-values-symbol-scope.mlir | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir b/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
index 057225ef44d53..29f4cb14af845 100644
--- a/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
+++ b/mlir/test/Transforms/remove-dead-values-symbol-scope.mlir
@@ -1,8 +1,5 @@
// 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 --pass-pipeline='builtin.module(gpu.module(remove-dead-values),builtin.module(remove-dead-values))' | FileCheck %s --check-prefixes=CHECK,SCOPED
// 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
>From a5317b18adc1474471bb55940c90355fd68b8464 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Thu, 24 Sep 2026 22:16:14 +0200
Subject: [PATCH 09/10] [mlir] Check private symbol visibility at query time
Keep private symbols out of the visibility set. Check their visibility before the set lookup and use the map root to retain the scope boundary. Cover the private root itself in the visibility test.
---
mlir/include/mlir/IR/SymbolTable.h | 9 +++++----
mlir/lib/IR/SymbolTable.cpp | 12 +++++++++---
mlir/unittests/IR/SymbolTableTest.cpp | 1 +
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index 1a15d261c7229..ab2dd387af090 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -416,9 +416,7 @@ class SymbolUserMap {
}
/// Return true if all uses of the symbol within the IR are within this scope.
- bool areAllUsesVisible(Operation *symbol) const {
- return symbolsWithAllUsesVisible.contains(symbol);
- }
+ bool areAllUsesVisible(Operation *symbol) const;
/// Return true if the given symbol has no uses within this map's scope.
bool useEmpty(Operation *symbol) const {
@@ -433,10 +431,13 @@ class SymbolUserMap {
/// A reference to the symbol table used to construct this map.
SymbolTableCollection &symbolTable;
+ /// The root of this map's scope.
+ Operation *root;
+
/// A map of symbol operations to symbol users.
DenseMap<Operation *, SetVector<Operation *>> symbolToUsers;
- /// Symbols whose uses are all visible within this map's scope.
+ /// Non-private 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 a6601b0e39088..16e4eb82ed6e2 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1049,14 +1049,13 @@ LockedSymbolTableCollection::getSymbolTable(Operation *symbolTableOp) {
SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
Operation *symbolTableOp)
- : symbolTable(symbolTable) {
+ : symbolTable(symbolTable), root(symbolTableOp) {
// Collect symbol users and visibility within each symbol table.
SmallVector<Operation *> symbols;
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() || allUsesVisible)
+ if (allUsesVisible && !symbol.isPrivate())
symbolsWithAllUsesVisible.insert(&nestedOp);
}
auto symbolUses = SymbolTable::getSymbolUses(&nestedOp);
@@ -1076,6 +1075,13 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
symbolTableOp, /*allSymUsesVisible=*/!symbolTableOp->getBlock(), walkFn);
}
+bool SymbolUserMap::areAllUsesVisible(Operation *symbol) const {
+ // Private symbols can only have users within their table.
+ if (cast<SymbolOpInterface>(symbol).isPrivate())
+ return root->isProperAncestor(symbol);
+ return symbolsWithAllUsesVisible.contains(symbol);
+}
+
void SymbolUserMap::replaceAllUsesWith(Operation *symbol,
StringAttr newSymbolName) {
auto it = symbolToUsers.find(symbol);
diff --git a/mlir/unittests/IR/SymbolTableTest.cpp b/mlir/unittests/IR/SymbolTableTest.cpp
index 376ac2591e50d..a04db6a9aac86 100644
--- a/mlir/unittests/IR/SymbolTableTest.cpp
+++ b/mlir/unittests/IR/SymbolTableTest.cpp
@@ -271,6 +271,7 @@ TEST(SymbolUserMap, AllUsesVisible) {
// A private table hides its nested symbols even when it is the map root.
SymbolUserMap hiddenMap(tables, hidden);
+ EXPECT_FALSE(hiddenMap.areAllUsesVisible(hidden));
EXPECT_TRUE(hiddenMap.areAllUsesVisible(hiddenLeaf));
EXPECT_TRUE(hiddenMap.areAllUsesVisible(hiddenPublic));
}
>From 6d4ffbeab5a3894c2af454d4e9f6875e0bd5c8b0 Mon Sep 17 00:00:00 2001
From: Ivan Butygin <ivan.butygin at gmail.com>
Date: Thu, 24 Sep 2026 22:57:52 +0200
Subject: [PATCH 10/10] [mlir] Require symbol visibility queries to stay within
scope
Document the in-scope symbol precondition. Return true directly for private symbols and remove the root field and out-of-scope test queries.
---
mlir/include/mlir/IR/SymbolTable.h | 4 +---
mlir/lib/IR/SymbolTable.cpp | 4 ++--
mlir/unittests/IR/SymbolTableTest.cpp | 4 ----
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/mlir/include/mlir/IR/SymbolTable.h b/mlir/include/mlir/IR/SymbolTable.h
index ab2dd387af090..1e57e3ff82912 100644
--- a/mlir/include/mlir/IR/SymbolTable.h
+++ b/mlir/include/mlir/IR/SymbolTable.h
@@ -416,6 +416,7 @@ class SymbolUserMap {
}
/// Return true if all uses of the symbol within the IR are within this scope.
+ /// The symbol must belong to this map's scope.
bool areAllUsesVisible(Operation *symbol) const;
/// Return true if the given symbol has no uses within this map's scope.
@@ -431,9 +432,6 @@ class SymbolUserMap {
/// A reference to the symbol table used to construct this map.
SymbolTableCollection &symbolTable;
- /// The root of this map's scope.
- Operation *root;
-
/// A map of symbol operations to symbol users.
DenseMap<Operation *, SetVector<Operation *>> symbolToUsers;
diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 16e4eb82ed6e2..4b19b4b0d4113 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -1049,7 +1049,7 @@ LockedSymbolTableCollection::getSymbolTable(Operation *symbolTableOp) {
SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
Operation *symbolTableOp)
- : symbolTable(symbolTable), root(symbolTableOp) {
+ : symbolTable(symbolTable) {
// Collect symbol users and visibility within each symbol table.
SmallVector<Operation *> symbols;
auto walkFn = [&](Operation *symbolTableOp, bool allUsesVisible) {
@@ -1078,7 +1078,7 @@ SymbolUserMap::SymbolUserMap(SymbolTableCollection &symbolTable,
bool SymbolUserMap::areAllUsesVisible(Operation *symbol) const {
// Private symbols can only have users within their table.
if (cast<SymbolOpInterface>(symbol).isPrivate())
- return root->isProperAncestor(symbol);
+ return true;
return symbolsWithAllUsesVisible.contains(symbol);
}
diff --git a/mlir/unittests/IR/SymbolTableTest.cpp b/mlir/unittests/IR/SymbolTableTest.cpp
index a04db6a9aac86..b5100b00e2c6c 100644
--- a/mlir/unittests/IR/SymbolTableTest.cpp
+++ b/mlir/unittests/IR/SymbolTableTest.cpp
@@ -235,7 +235,6 @@ TEST(SymbolUserMap, AllUsesVisible) {
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");
@@ -266,12 +265,9 @@ TEST(SymbolUserMap, AllUsesVisible) {
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_FALSE(hiddenMap.areAllUsesVisible(hidden));
EXPECT_TRUE(hiddenMap.areAllUsesVisible(hiddenLeaf));
EXPECT_TRUE(hiddenMap.areAllUsesVisible(hiddenPublic));
}
More information about the Mlir-commits
mailing list