From 23eec1216993f599f90e259e339228ba8b69c58a Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Fri, 27 Mar 2026 17:22:47 +0100 Subject: [PATCH] [MLIR] Fix outdated restriction comment in RemoveDeadValuesPass (#189041) The RemoveDeadValuesPass previously emitted an error and skipped optimization when the IR contained non-function symbol ops, non-call symbol user ops, or branch ops. This restriction was later removed, but the comments in RemoveDeadValues.cpp and Passes.td still described the pass as operating "iff the IR doesn't have any non-function symbol ops, non-call symbol user ops and branch ops." Remove the stale restriction text from both the .cpp file comment and the Passes.td description. Also add a test that verifies dead function arguments are correctly removed inside a module that defines a symbol (has a sym_name attribute), which was the original failure case reported in issue #98700. Fixes #98700 Assisted-by: Claude Code --- mlir/include/mlir/Transforms/Passes.td | 7 +------ mlir/lib/Transforms/RemoveDeadValues.cpp | 7 +------ mlir/test/Transforms/remove-dead-values.mlir | 21 ++++++++++++++++++++ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/mlir/include/mlir/Transforms/Passes.td b/mlir/include/mlir/Transforms/Passes.td index 8a6228e00aa0..1474e580cfc0 100644 --- a/mlir/include/mlir/Transforms/Passes.td +++ b/mlir/include/mlir/Transforms/Passes.td @@ -106,12 +106,7 @@ def RemoveDeadValuesPass : Pass<"remove-dead-values"> { (C) Removes unneccesary operands, results, region arguments, and region terminator operands of region branch ops, and, (D) Removes simple and region branch ops that have all non-live results and - don't affect memory in any way, - - iff - - the IR doesn't have any non-function symbol ops, non-call symbol user ops - and branch ops. + don't affect memory in any way. Here, a "simple op" refers to an op that isn't a symbol op, symbol-user op, region branch op, branch op, region branch terminator op, or return-like. diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index ade764789083..f0a210a2eded 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -19,12 +19,7 @@ // (C) Removes unneccesary operands, results, region arguments, and region // terminator operands of region branch ops, and, // (D) Removes simple and region branch ops that have all non-live results and -// don't affect memory in any way, -// -// iff -// -// the IR doesn't have any non-function symbol ops, non-call symbol user ops and -// branch ops. +// don't affect memory in any way. // // Here, a "simple op" refers to an op that isn't a symbol op, symbol-user op, // region branch op, branch op, region branch terminator op, or return-like. diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir index 19bc6b2fddd6..64088ce15cd4 100644 --- a/mlir/test/Transforms/remove-dead-values.mlir +++ b/mlir/test/Transforms/remove-dead-values.mlir @@ -29,6 +29,27 @@ module @named_module_acceptable { // ----- +// Dead function arguments are removed even if the enclosing module defines a +// symbol (has a sym_name attribute). Fixes #98700. +// +// CHECK: func.func private @compute(%arg0: i32) -> i32 +// CHECK-NOT: %dead_arg +module @named_module_with_dead_args { + func.func private @compute(%arg0: i32, %dead_arg: i32) -> i32 { + return %arg0 : i32 + } + // CHECK: func.func @main() -> i32 + func.func @main() -> i32 { + %c1 = arith.constant 1 : i32 + %c2 = arith.constant 2 : i32 + // CHECK: call @compute(%c1_i32) : (i32) -> i32 + %result = call @compute(%c1, %c2) : (i32, i32) -> i32 + return %result : i32 + } +} + +// ----- + // The IR contains both conditional and unconditional branches with a loop // in which the last cf.cond_br is referncing the first cf.br //