[mlir][transforms] Add signalPassFailure in RemoveDeadValues

[CoTinker]

This PR adds signalPassFailure in RemoveDeadValues to ensure that a pipeline would stop here.
Fixes #111757.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Longsheng Mou (CoTinker)

Changes

This PR adds signalPassFailure in RemoveDeadValues to ensure that a pipeline would stop here.
Fixes #111757.

---

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

1 Files Affected:

• (modified) mlir/lib/Transforms/RemoveDeadValues.cpp (+1-1)

diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 3de4fb75ed831c..7e45f18b660ba7 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -589,7 +589,7 @@ void RemoveDeadValues::runOnOperation() {
   });
 
   if (acceptableIR.wasInterrupted())
-    return;
+    return signalPassFailure();
 
   module->walk([&](Operation *op) {
     if (auto funcOp = dyn_cast<FunctionOpInterface>(op)) {

[llvmbot]

@llvm/pr-subscribers-mlir-core

Author: Longsheng Mou (CoTinker)

Changes

This PR adds signalPassFailure in RemoveDeadValues to ensure that a pipeline would stop here.
Fixes #111757.

---

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

1 Files Affected:

• (modified) mlir/lib/Transforms/RemoveDeadValues.cpp (+1-1)

diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 3de4fb75ed831c..7e45f18b660ba7 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -589,7 +589,7 @@ void RemoveDeadValues::runOnOperation() {
   });
 
   if (acceptableIR.wasInterrupted())
-    return;
+    return signalPassFailure();
 
   module->walk([&](Operation *op) {
     if (auto funcOp = dyn_cast<FunctionOpInterface>(op)) {

[MaheshRavishankar]

Could you add a test here please?

[CoTinker]

Could you add a test here please?

I don't know how to add test for signalPassFailure, could you please give me some advice.

[MaheshRavishankar]

You can add a test with // expected-error and use --verify-diagnostics (I think).

[joker-eph]

LG with a test

[joker-eph]

Mmmmm there is already a test for the diagnostic, somehow the test does not check the return of mlir-opt though!

[CoTinker]

Sorry, I'm still having trouble understanding how to check for signalPassFailure in mlir-opt. I know expected-error and verify-diagnostics can check for emitError, but I'm not sure how to handle signalPassFailure.

[MaheshRavishankar]

Sorry, I'm still having trouble understanding how to check for signalPassFailure in mlir-opt. I know expected-error and verify-diagnostics can check for emitError, but I'm not sure how to handle signalPassFailure.

Emit an error message before the return signalPassFailure and check for the error message?

[CoTinker]

Sorry, I'm still having trouble understanding how to check for signalPassFailure in mlir-opt. I know expected-error and verify-diagnostics can check for emitError, but I'm not sure how to handle signalPassFailure.

Emit an error message before the return signalPassFailure and check for the error message?

llvm-project/mlir/test/Transforms/remove-dead-values.mlir

Lines 3 to 12 in 57d109c

	// The IR remains untouched because of the presence of a non-function-like
	// symbol op inside the module (const @__dont_touch_unacceptable_ir).
	//
	module {
	// expected-error @+1 {{cannot optimize an IR with non-function symbol ops, non-call symbol user ops or branch ops}}
	memref.global "private" constant @__dont_touch_unacceptable_ir : memref<i32> = dense<0>
	func.func @main(%arg0: i32) -> i32 {
	return %arg0 : i32
	}
	}

It has already been checked.

[CoTinker]

Error message is here.

llvm-project/mlir/lib/Transforms/RemoveDeadValues.cpp

Lines 578 to 592 in 57d109c

	WalkResult acceptableIR = module->walk([&](Operation *op) {
	if (op == module)
	return WalkResult::advance();
	if (isa<BranchOpInterface>(op) ||
	(isa<SymbolOpInterface>(op) && !isa<FunctionOpInterface>(op)) ||
	(isa<SymbolUserOpInterface>(op) && !isa<CallOpInterface>(op))) {
	op->emitError() << "cannot optimize an IR with non-function symbol ops, "
	"non-call symbol user ops or branch ops\n";
	return WalkResult::interrupt();
	}
	return WalkResult::advance();
	});
	if (acceptableIR.wasInterrupted())
	return;

[CoTinker]

Thanks for your review.