[Mlir-commits] [mlir] Fix Bug in RemoveDeadValues Pass (PR #148437)

[llvmlistbot at llvm.org]

https://github.com/ronigoldman22 updated https://github.com/llvm/llvm-project/pull/148437

>From b1564d00b631f3f17a57af4095917abb73fc99a2 Mon Sep 17 00:00:00 2001
From: Roni Goldman <roni.goldman22 at gmail.com>
Date: Sun, 13 Jul 2025 17:46:56 +0300
Subject: [PATCH 1/2] fix bug in remove-dead-values pass and add test for the
 case

---
 mlir/lib/Transforms/RemoveDeadValues.cpp     |  4 +-
 mlir/test/Transforms/remove-dead-values.mlir | 40 ++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 608bdcb948176..ddd5f2ba1a7b7 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -345,8 +345,6 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module,
   // since it forwards only to non-live value(s) (%1#1).
   Operation *lastReturnOp = funcOp.back().getTerminator();
   size_t numReturns = lastReturnOp->getNumOperands();
-  if (numReturns == 0)
-    return;
   BitVector nonLiveRets(numReturns, true);
   for (SymbolTable::SymbolUse use : uses) {
     Operation *callOp = use.getUser();
@@ -368,6 +366,8 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module,
   cl.functions.push_back({funcOp, nonLiveArgs, nonLiveRets});
 
   // Do (5) and (6).
+  if (numReturns == 0)
+    return;
   for (SymbolTable::SymbolUse use : uses) {
     Operation *callOp = use.getUser();
     assert(isa<CallOpInterface>(callOp) && "expected a call-like user");
diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir
index 3af95db3c0e24..dcaeefc3f2a6f 100644
--- a/mlir/test/Transforms/remove-dead-values.mlir
+++ b/mlir/test/Transforms/remove-dead-values.mlir
@@ -548,3 +548,43 @@ func.func @test_atomic_yield(%I: memref<10xf32>, %idx : index) {
   func.return
 }
 
+// CHECK-LABEL: module @return_void_with_unused_argument {
+// CHECK-LABEL:  func.func private @noop() {
+// CHECK-NEXT:    return
+// CHECK-NEXT:  }
+// CHECK-LABEL:  func.func private @fn_return_void_with_unused_argument(%arg0: memref<1x1xsi8, 1>, %arg1: memref<1x1xsi16, 1>) {
+// CHECK-NEXT:    %alloc = memref.alloc() {alignment = 8 : i64} : memref<1x73xsi8, 3>
+// CHECK-NEXT:    %alloc_0 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi16, 3>
+// CHECK-NEXT:    %alloc_1 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi8, 3>
+// CHECK-NEXT:    memref.copy %arg0, %alloc_1 : memref<1x1xsi8, 1> to memref<1x1xsi8, 3>
+// CHECK-NEXT:    memref.copy %arg1, %alloc_0 : memref<1x1xsi16, 1> to memref<1x1xsi16, 3>
+// CHECK-NEXT:    call @noop() : () -> ()
+// CHECK-NEXT:    return
+// CHECK-NEXT:  }
+// CHECK-LABEL:  func.func @main(%arg0: memref<1x73xsi8, 1>, %arg1: memref<1x1xsi8, 1>, %arg2: memref<1x1xsi16, 1>) -> memref<1x73xsi8, 1> {
+// CHECK-NEXT:    %alloc = memref.alloc() : memref<1x73xsi8, 1>
+// CHECK-NEXT:    call @fn_return_void_with_unused_argument(%arg1, %arg2) : (memref<1x1xsi8, 1>, memref<1x1xsi16, 1>) -> ()
+// CHECK-NEXT:    return %alloc : memref<1x73xsi8, 1>
+// CHECK-NEXT:  }
+// CHECK-NEXT:}
+module @return_void_with_unused_argument {
+  func.func private @noop(%arg0: memref<1x1xsi8, 3>, %arg1: memref<1x1xsi16, 3>, %arg2: memref<1x73xsi8, 1>) {
+    return
+  }
+  func.func private @fn_return_void_with_unused_argument(%arg0: memref<1x73xsi8, 1> , %arg1: memref<1x1xsi8, 1> , %arg2: memref<1x1xsi16, 1> , %arg3: memref<1x73xsi8, 1>) {
+    %alloc = memref.alloc() {alignment = 8 : i64} : memref<1x73xsi8, 3>
+    %alloc_0 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi16, 3>
+    %alloc_1 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi8, 3>
+    memref.copy %arg1, %alloc_1 : memref<1x1xsi8, 1> to memref<1x1xsi8, 3>
+    memref.copy %arg2, %alloc_0 : memref<1x1xsi16, 1> to memref<1x1xsi16, 3>
+    call @noop(%alloc_1, %alloc_0, %arg0) : 
+        (memref<1x1xsi8, 3>, memref<1x1xsi16, 3>, memref<1x73xsi8, 1>) -> ()
+    return
+  }
+  func.func @main(%arg0: memref<1x73xsi8, 1> , %arg1: memref<1x1xsi8, 1> , %arg2: memref<1x1xsi16, 1>) -> (memref<1x73xsi8, 1> ) {
+    %alloc = memref.alloc() : memref<1x73xsi8, 1>
+    call @fn_return_void_with_unused_argument(%arg0, %arg1, %arg2, %alloc) : (memref<1x73xsi8, 1>, memref<1x1xsi8, 1>, memref<1x1xsi16, 1>, memref<1x73xsi8, 1>) -> ()
+    return %alloc : memref<1x73xsi8, 1>
+  }
+}
+

>From 6ee686207f7f65bc451f6069d47ec2059a6687a5 Mon Sep 17 00:00:00 2001
From: Roni Goldman <roni.goldman22 at gmail.com>
Date: Mon, 14 Jul 2025 15:07:52 +0300
Subject: [PATCH 2/2] fix lit test

---
 mlir/test/Transforms/remove-dead-values.mlir | 49 ++++++--------------
 1 file changed, 15 insertions(+), 34 deletions(-)

diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir
index dcaeefc3f2a6f..eed1bbffa47e4 100644
--- a/mlir/test/Transforms/remove-dead-values.mlir
+++ b/mlir/test/Transforms/remove-dead-values.mlir
@@ -548,43 +548,24 @@ func.func @test_atomic_yield(%I: memref<10xf32>, %idx : index) {
   func.return
 }
 
-// CHECK-LABEL: module @return_void_with_unused_argument {
-// CHECK-LABEL:  func.func private @noop() {
-// CHECK-NEXT:    return
-// CHECK-NEXT:  }
-// CHECK-LABEL:  func.func private @fn_return_void_with_unused_argument(%arg0: memref<1x1xsi8, 1>, %arg1: memref<1x1xsi16, 1>) {
-// CHECK-NEXT:    %alloc = memref.alloc() {alignment = 8 : i64} : memref<1x73xsi8, 3>
-// CHECK-NEXT:    %alloc_0 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi16, 3>
-// CHECK-NEXT:    %alloc_1 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi8, 3>
-// CHECK-NEXT:    memref.copy %arg0, %alloc_1 : memref<1x1xsi8, 1> to memref<1x1xsi8, 3>
-// CHECK-NEXT:    memref.copy %arg1, %alloc_0 : memref<1x1xsi16, 1> to memref<1x1xsi16, 3>
-// CHECK-NEXT:    call @noop() : () -> ()
-// CHECK-NEXT:    return
-// CHECK-NEXT:  }
-// CHECK-LABEL:  func.func @main(%arg0: memref<1x73xsi8, 1>, %arg1: memref<1x1xsi8, 1>, %arg2: memref<1x1xsi16, 1>) -> memref<1x73xsi8, 1> {
-// CHECK-NEXT:    %alloc = memref.alloc() : memref<1x73xsi8, 1>
-// CHECK-NEXT:    call @fn_return_void_with_unused_argument(%arg1, %arg2) : (memref<1x1xsi8, 1>, memref<1x1xsi16, 1>) -> ()
-// CHECK-NEXT:    return %alloc : memref<1x73xsi8, 1>
-// CHECK-NEXT:  }
-// CHECK-NEXT:}
+// CHECK-LABEL: module @return_void_with_unused_argument
 module @return_void_with_unused_argument {
-  func.func private @noop(%arg0: memref<1x1xsi8, 3>, %arg1: memref<1x1xsi16, 3>, %arg2: memref<1x73xsi8, 1>) {
-    return
-  }
-  func.func private @fn_return_void_with_unused_argument(%arg0: memref<1x73xsi8, 1> , %arg1: memref<1x1xsi8, 1> , %arg2: memref<1x1xsi16, 1> , %arg3: memref<1x73xsi8, 1>) {
-    %alloc = memref.alloc() {alignment = 8 : i64} : memref<1x73xsi8, 3>
-    %alloc_0 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi16, 3>
-    %alloc_1 = memref.alloc() {alignment = 8 : i64} : memref<1x1xsi8, 3>
-    memref.copy %arg1, %alloc_1 : memref<1x1xsi8, 1> to memref<1x1xsi8, 3>
-    memref.copy %arg2, %alloc_0 : memref<1x1xsi16, 1> to memref<1x1xsi16, 3>
-    call @noop(%alloc_1, %alloc_0, %arg0) : 
-        (memref<1x1xsi8, 3>, memref<1x1xsi16, 3>, memref<1x73xsi8, 1>) -> ()
+  // CHECK-LABEL: func.func private @fn_return_void_with_unused_argument
+  // CHECK-SAME: (%[[ARG0_FN:.*]]: i32)
+  func.func private @fn_return_void_with_unused_argument(%arg0: i32, %arg1: memref<4xi32>) -> () {
+    %sum = arith.addi %arg0, %arg0 : i32
+    %c0 = arith.constant 0 : index
+    %buf = memref.alloc() : memref<1xi32>
+    memref.store %sum, %buf[%c0] : memref<1xi32>
     return
   }
-  func.func @main(%arg0: memref<1x73xsi8, 1> , %arg1: memref<1x1xsi8, 1> , %arg2: memref<1x1xsi16, 1>) -> (memref<1x73xsi8, 1> ) {
-    %alloc = memref.alloc() : memref<1x73xsi8, 1>
-    call @fn_return_void_with_unused_argument(%arg0, %arg1, %arg2, %alloc) : (memref<1x73xsi8, 1>, memref<1x1xsi8, 1>, memref<1x1xsi16, 1>, memref<1x73xsi8, 1>) -> ()
-    return %alloc : memref<1x73xsi8, 1>
+  // CHECK-LABEL: func.func @main
+  // CHECK-SAME: (%[[ARG0_MAIN:.*]]: i32)
+  // CHECK: call @fn_return_void_with_unused_argument(%[[ARG0_MAIN]]) : (i32) -> ()
+  func.func @main(%arg0: i32) -> memref<4xi32> {
+    %unused = memref.alloc() : memref<4xi32>
+    call @fn_return_void_with_unused_argument(%arg0, %unused) : (i32, memref<4xi32>) -> ()
+    return %unused : memref<4xi32>
   }
 }