[mlir] Enable remove-dead-values to delete unused private function #161471

linuxlonelyeagle · 2025-10-01T01:55:38Z

Fix #158760. Introduce the deleteDeadFunction function, which is used to delete dead functions.

llvmbot · 2025-10-01T01:56:12Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: lonely eagle (linuxlonelyeagle)

Changes

Fix #158760. If a private functionOpInterface has no users and there is no callOpInterface in its region, it is a useless function, and the remove-dead-values pass will delete this function.

Full diff: https://github.com/llvm/llvm-project/pull/161471.diff

2 Files Affected:

(modified) mlir/lib/Transforms/RemoveDeadValues.cpp (+15-2)
(modified) mlir/test/Transforms/remove-dead-values.mlir (+15)

diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index e0c65b0e09774..18c6006f04788 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -297,6 +297,17 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module,
     return;
   }
 
+  // If a private function has neither users and function calls, it is a useless
+  // function.
+  SymbolTable::UseRange uses = *funcOp.getSymbolUses(module);
+  auto callSites = funcOp.getFunctionBody().getOps<CallOpInterface>();
+  if (uses.empty() && callSites.empty()) {
+    LDBG() << "Delete function op: "
+           << OpWithFlags(funcOp, OpPrintingFlags().skipRegions());
+    funcOp.erase();
+    return;
+  }
+
   // Get the list of unnecessary (non-live) arguments in `nonLiveArgs`.
   SmallVector<Value> arguments(funcOp.getArguments());
   BitVector nonLiveArgs = markLives(arguments, nonLiveSet, la);
@@ -312,7 +323,6 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module,
   // Do (2). (Skip creating generic operand cleanup entries for call ops.
   // Call arguments will be removed in the call-site specific segment-aware
   // cleanup, avoiding generic eraseOperands bitvector mechanics.)
-  SymbolTable::UseRange uses = *funcOp.getSymbolUses(module);
   for (SymbolTable::SymbolUse use : uses) {
     Operation *callOp = use.getUser();
     assert(isa<CallOpInterface>(callOp) && "expected a call-like user");
@@ -881,9 +891,12 @@ void RemoveDeadValues::runOnOperation() {
   // end of this pass.
   RDVFinalCleanupList finalCleanupList;
 
+  module->walk([&](FunctionOpInterface op) {
+    processFuncOp(op, module, la, deadVals, finalCleanupList);
+  });
   module->walk([&](Operation *op) {
     if (auto funcOp = dyn_cast<FunctionOpInterface>(op)) {
-      processFuncOp(funcOp, module, la, deadVals, finalCleanupList);
+      // The FunctionOpInterface has been processed in advance.
     } else if (auto regionBranchOp = dyn_cast<RegionBranchOpInterface>(op)) {
       processRegionBranchOp(regionBranchOp, la, deadVals, finalCleanupList);
     } else if (auto branchOp = dyn_cast<BranchOpInterface>(op)) {
diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir
index 56449469dc29f..b8ad762c52ddf 100644
--- a/mlir/test/Transforms/remove-dead-values.mlir
+++ b/mlir/test/Transforms/remove-dead-values.mlir
@@ -649,3 +649,18 @@ func.func @callee(%arg0: index, %arg1: index, %arg2: index) -> index {
   %res = call @mutl_parameter(%arg0, %arg1, %arg2) : (index, index, index) -> (index)
   return %res : index
 }
+
+// -----
+
+// CHECK-NOT: func private @single_private_func 
+func.func private @single_private_func(%arg0: i64) -> (i64) {
+    %c0_i64 = arith.constant 0 : i64
+    %2 = arith.cmpi eq, %arg0, %c0_i64 : i64
+    cf.cond_br %2, ^bb1, ^bb2
+  ^bb1:  // pred: ^bb0
+    %c1_i64 = arith.constant 1 : i64
+    return %c1_i64 : i64
+  ^bb2:  // pred: ^bb0
+    %c3_i64 = arith.constant 3 : i64
+    return %c3_i64 : i64
+}
\ No newline at end of file

ftynse

I don't think this fixes the bug in question, just removes the function that contained the bug. We also have a dead symbol elimination pass that handles such cases generically.

linuxlonelyeagle · 2025-10-01T09:02:57Z

I don't think this fixes the bug in question, just removes the function that contained the bug. We also have a dead symbol elimination pass that handles such cases generically.

https://mlir.llvm.org/docs/Passes/ I didn't see the dead symbol elimination pass.
Beyond that, I believe such a fix is reasonable.In this bug, the key reason is that the dead code analysis pass determined the entire IR to be dead.This led to unexpected results in liveness analysis.The removal of dead values relies on liveness analysis, which is why this bug occurred.

Looking at this from another angle, the private function is actually dead, so I introduced this.

joker-eph · 2025-10-01T10:31:48Z

I believe such a fix is reasonable.In this bug, the key reason is that the dead code analysis pass determined the entire IR to be dead.This led to unexpected results in liveness analysis

Can you describe a bit more the underlying issue?

mlir/lib/Transforms/RemoveDeadValues.cpp

linuxlonelyeagle · 2025-10-01T16:12:49Z

I believe such a fix is reasonable.In this bug, the key reason is that the dead code analysis pass determined the entire IR to be dead.This led to unexpected results in liveness analysis

Can you describe a bit more the underlying issue?

mlir-opt test.mlir -remove-dead-values -allow-unregistered-dialect -debug &> log   
...
[liveness-analysis LivenessAnalysis.cpp:299 1] RunLivenessAnalysis initialized for op: builtin.module check on unreachable code now:
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %c0_i64 = arith.constant 0 : i64 has no liveness info (unreachable), mark dead
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %0 = arith.cmpi eq, %arg0, %c0_i64 : i64 has no liveness info (unreachable), mark dead
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %c1_i64 = arith.constant 1 : i64 has no liveness info (unreachable), mark dead
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %c3_i64 = arith.constant 3 : i64 has no liveness info (unreachable), mark dead

Why did the above result occur in the liveness analysis?
Because they are dead code in dead code analysis.
Why are they dead code?
Because private functions have no caller.
This is the motivation behind why this PR was done in this manner.
cc: @ftynse @joker-eph

joker-eph · 2025-10-01T16:17:21Z

That does not really explain the crash though: why wasn't the cond_branch also dead and removed?

linuxlonelyeagle · 2025-10-01T16:25:57Z

That does not really explain the crash though: why wasn't the cond_branch also dead and removed?

Good question. Give me a moment to look into the code.

linuxlonelyeagle · 2025-10-01T16:51:55Z

That does not really explain the crash though: why wasn't the cond_branch also dead and removed?

[dataflow SparseAnalysis.cpp:431 1] Visiting operation: cf.cond_br with 1 operands and 0 results
[dataflow SparseAnalysis.cpp:439 1] Operation is in dead block, bailing out
[dataflow SparseAnalysis.cpp:431 1] Visiting operation: arith.cmpi with 2 operands and 1 results
[dataflow SparseAnalysis.cpp:439 1] Operation is in dead block, bailing out
[dataflow SparseAnalysis.cpp:431 1] Visiting operation: arith.constant with 0 operands and 1 results
[dataflow SparseAnalysis.cpp:439 1] Operation is in dead block, bailing out
[dataflow SparseAnalysis.cpp:431 1] Visiting operation: func.return with 1 operands and 0 results
[dataflow SparseAnalysis.cpp:439 1] Operation is in dead block, bailing out
[dataflow SparseAnalysis.cpp:431 1] Visiting operation: arith.constant with 0 operands and 1 results
[dataflow SparseAnalysis.cpp:439 1] Operation is in dead block, bailing out
[dataflow SparseAnalysis.cpp:431 1] Visiting operation: func.return with 1 operands and 0 results
[dataflow SparseAnalysis.cpp:439 1] Operation is in dead block, bailing out
[dataflow SparseAnalysis.cpp:431 1] Visiting operation: arith.constant with 0 operands and 1 results
[dataflow SparseAnalysis.cpp:439 1] Operation is in dead block, bailing out
[liveness-analysis LivenessAnalysis.cpp:299 1] RunLivenessAnalysis initialized for op: builtin.module check on unreachable code now:
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %c0_i64 = arith.constant 0 : i64 has no liveness info (unreachable), mark dead
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %0 = arith.cmpi eq, %arg0, %c0_i64 : i64 has no liveness info (unreachable), mark dead
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %c1_i64 = arith.constant 1 : i64 has no liveness info (unreachable), mark dead
[liveness-analysis LivenessAnalysis.cpp:307 1] Result: 0 of %c3_i64 = arith.constant 3 : i64 has no liveness info (unreachable), mark dead
[liveness-analysis LivenessAnalysis.cpp:317 1] Block argument: 0 of func.func private @test(%arg0: i64) -> i64 {...} has no liveness info, mark dead
[pass-manager Pass.cpp:1102 2] PassManager run completed with result: success
test_tag: cf:
 operand #0: not live
module {
  func.func private @test(%arg0: i64) -> i64 {
    %c0_i64 = arith.constant 0 : i64
    %0 = arith.cmpi eq, %arg0, %c0_i64 : i64
    cf.cond_br %0, ^bb1, ^bb2 {tag = "cf"}
  ^bb1:  // pred: ^bb0
    %c1_i64 = arith.constant 1 : i64
    return %c1_i64 : i64
  ^bb2:  // pred: ^bb0
    %c3_i64 = arith.constant 3 : i64
    return %c3_i64 : i64
  }
}

llvm-project/mlir/lib/Analysis/DataFlow/LivenessAnalysis.cpp

Line 317 in a9b8dfe

LDBG() << "Block argument: " << blockArg.index() << " of "

The liveness lattice will by default set the SSA values without a liveness lattice to dead.
Why isn't the br.cond op explicitly set to dead?
Because its operand, which is the result of arith.cmpi, is explicitly set to dead. In fact, it is dead.

linuxlonelyeagle · 2025-10-01T16:53:37Z

That does not really explain the crash though: why wasn't the cond_branch also dead and removed?

#161117 This PR maybe also need you.Thank you.

linuxlonelyeagle · 2025-10-06T15:38:29Z

@ftynse @joker-eph I've rewritten the code, and I think this should fix the issue nicely.

joker-eph · 2025-10-06T18:59:33Z

mlir/lib/Transforms/RemoveDeadValues.cpp

+      if (funcOp.isPublic() || funcOp.isExternal())
+        return;
+
+      SymbolTable::UseRange uses = *funcOp.getSymbolUses(module);


Isn't this super expensive to build?

It certainly feels super expensive.I've updated the code so that it now only runs on functions that are likely to be dead functions.

I accidentally discovered that the -symbol-dce pass includes the functionality of this PR.

This was mentioned to you earlier actually: #161471 (review)

I must admit I wasn't particularly well-versed in many of the MLIR passes, but I've gained a much better understanding now. Ha ha.😘
Actually, I just took another look at the code for this pass. The most immediate issue stems from directly deleting the operands of cond_br. The underlying problem remains the data flow analysis issue mentioned earlier.

It's a bit of a pity that -symbol-dce isn't implemented via patterns; otherwise, it might have been possible to incorporate it into remove-dead-values.To be perfectly honest, I know what the best fix is.Indeed, deleting dead functions is not the best approach.

I think perhaps we could wrap the logic of symbol-dce in a function and then use it in remove-dead-values.

linuxlonelyeagle requested review from ftynse and joker-eph October 1, 2025 01:55

llvmbot added mlir:core MLIR Core Infrastructure mlir labels Oct 1, 2025

linuxlonelyeagle requested a review from srishti-pm October 1, 2025 01:55

linuxlonelyeagle requested review from Sirraide and krzysz00 October 1, 2025 01:56

linuxlonelyeagle force-pushed the fix-private-func-remove-dead-values-bug branch from 5bb13ee to cee600a Compare October 1, 2025 01:58

ftynse requested changes Oct 1, 2025

View reviewed changes

joker-eph changed the title ~~[mlir] Make remove-dead-values can delate useless function~~ [mlir] Enable remove-dead-values to delete unused private function Oct 1, 2025

joker-eph reviewed Oct 1, 2025

View reviewed changes

mlir/lib/Transforms/RemoveDeadValues.cpp Outdated Show resolved Hide resolved

linuxlonelyeagle force-pushed the fix-private-func-remove-dead-values-bug branch from 11eb3f8 to 16af35c Compare October 1, 2025 16:05

add deleteDeadFunction function.

8f974c9

linuxlonelyeagle force-pushed the fix-private-func-remove-dead-values-bug branch from 16af35c to 8f974c9 Compare October 6, 2025 15:33

joker-eph reviewed Oct 6, 2025

View reviewed changes

update code.

f31761b

[mlir] Enable remove-dead-values to delete unused private function #161471

Are you sure you want to change the base?

[mlir] Enable remove-dead-values to delete unused private function #161471

Uh oh!

Conversation

linuxlonelyeagle commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ftynse left a comment

Choose a reason for hiding this comment

Uh oh!

linuxlonelyeagle commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joker-eph commented Oct 1, 2025

Uh oh!

Uh oh!

linuxlonelyeagle commented Oct 1, 2025

Uh oh!

joker-eph commented Oct 1, 2025

Uh oh!

linuxlonelyeagle commented Oct 1, 2025

Uh oh!

linuxlonelyeagle commented Oct 1, 2025

Uh oh!

linuxlonelyeagle commented Oct 1, 2025

Uh oh!

linuxlonelyeagle commented Oct 6, 2025

Uh oh!

joker-eph Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

linuxlonelyeagle Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

linuxlonelyeagle Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

joker-eph Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

linuxlonelyeagle Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

linuxlonelyeagle Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

linuxlonelyeagle Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

linuxlonelyeagle commented Oct 1, 2025 •

edited

Loading

llvmbot commented Oct 1, 2025 •

edited

Loading

linuxlonelyeagle commented Oct 1, 2025 •

edited

Loading