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

[Stella Laurenzo]

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

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.

>From 73257ee16a7f030d98d9b631fd7753b65c3946e9 Mon Sep 17 00:00:00 2001
From: Stella Laurenzo <stellaraccident at gmail.com>
Date: Thu, 27 Jun 2024 19:24:30 -0700
Subject: [PATCH] [mlir][python] Correctly handle python passes that emit
 errors without signalling failure.

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.
---
 mlir/lib/Bindings/Python/Pass.cpp | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

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 "