[Mlir-commits] [mlir] [mlir][python] Correctly handle python passes that emit errors without signalling failure. (PR #96996)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Stella Laurenzo (stellaraccident)

<details>
<summary>Changes</summary>

It was previously possible for a pass to emit errors, which would be collected by the diagnostic handler. But if it did not then signal failure, this would trigger an assert (because the diagnostic handler collected errors which were not consumed).

There aren't great things to do in this case. After trying some options, I opted for just issuing a Python level warning when this happens. This isn't great but includes a way to further debug and doesn't change semantics on what actually constitutes an error from the API standpoint. It will at least be visible and is better than an assert.

Fixes https://github.com/llvm/torch-mlir/issues/3506 properly.

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


1 Files Affected:

- (modified) mlir/lib/Bindings/Python/Pass.cpp (+19-3) 


``````````diff
diff --git a/mlir/lib/Bindings/Python/Pass.cpp b/mlir/lib/Bindings/Python/Pass.cpp
index a68421b61641f..b3fa3d0bc39e0 100644
--- a/mlir/lib/Bindings/Python/Pass.cpp
+++ b/mlir/lib/Bindings/Python/Pass.cpp
@@ -123,12 +123,28 @@ void mlir::python::populatePassManagerSubmodule(py::module &m) {
               op.getOperation().getContext()->clearOperationsInside(op);
             }
             // Actually run the pass manager.
-            PyMlirContext::ErrorCapture errors(op.getOperation().getContext());
+            PyMlirContext::ErrorCapture error_capture(
+                op.getOperation().getContext());
             MlirLogicalResult status = mlirPassManagerRunOnOp(
                 passManager.get(), op.getOperation().get());
-            if (mlirLogicalResultIsFailure(status))
+            auto errors = error_capture.take();
+            if (mlirLogicalResultIsFailure(status)) {
               throw MLIRError("Failure while executing pass pipeline",
-                              errors.take());
+                              std::move(errors));
+            } else if (!errors.empty()) {
+              // If errors were emitted without a failure, issue a warning.
+              static const char *message =
+                  "An MLIR pass emitted an error but did not signal failure. "
+                  "Unfortunately, this error information has been lost as a "
+                  "result. Setting "
+                  "`context.emit_error_diagnostics = True` can ensure "
+                  "that all diagnostics are printed to stderr, even when being "
+                  "captured for Python exception handling.";
+              if (PyErr_WarnEx(PyExc_UserWarning, message, /*stack_level=*/1) !=
+                  0) {
+                throw py::error_already_set();
+              }
+            }
           },
           "operation"_a, "invalidate_ops"_a = true,
           "Run the pass manager on the provided operation, raising an "

``````````

</details>


https://github.com/llvm/llvm-project/pull/96996