Skip to content

Conversation

Cubevoid
Copy link
Contributor

This patch adds an option to PassManager which simply controls whether an error will be emitted when a pass in the pipeline fails. This error includes the name of the pass that failed which could be very helpful for debugging.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Aug 27, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 27, 2025

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Colin De Vlieghere (Cubevoid)

Changes

This patch adds an option to PassManager which simply controls whether an error will be emitted when a pass in the pipeline fails. This error includes the name of the pass that failed which could be very helpful for debugging.


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

6 Files Affected:

  • (modified) mlir/include/mlir/Pass/PassManager.h (+7)
  • (modified) mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h (+10)
  • (modified) mlir/lib/Pass/Pass.cpp (+30-15)
  • (modified) mlir/lib/Pass/PassDetail.h (+11-7)
  • (modified) mlir/lib/Tools/mlir-opt/MlirOptMain.cpp (+6)
  • (added) mlir/test/Pass/pass-name-diagnostics.mlir (+7)
diff --git a/mlir/include/mlir/Pass/PassManager.h b/mlir/include/mlir/Pass/PassManager.h
index 6e59b0f32ac6f..a6021f1e6fd87 100644
--- a/mlir/include/mlir/Pass/PassManager.h
+++ b/mlir/include/mlir/Pass/PassManager.h
@@ -275,6 +275,9 @@ class PassManager : public OpPassManager {
   /// Runs the verifier after each individual pass.
   void enableVerifier(bool enabled = true);
 
+  /// Sets whether an error containing the failing pass name should be emitted upon failure.
+  void enableErrorOnFailure(bool enabled = true);
+
   //===--------------------------------------------------------------------===//
   // Instrumentations
   //===--------------------------------------------------------------------===//
@@ -497,6 +500,10 @@ class PassManager : public OpPassManager {
 
   /// A flag that indicates if the IR should be verified in between passes.
   bool verifyPasses : 1;
+
+  /// A flag that indicates if an error containing the pass name should be
+  /// emitted upon failure.
+  bool emitErrorOnFailure : 1;
 };
 
 /// Register a set of useful command-line options that can be used to configure
diff --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index 94231227599c9..67d6bf98bccea 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -207,6 +207,13 @@ class MlirOptMainConfig {
   }
   bool shouldVerifyPasses() const { return verifyPassesFlag; }
 
+  /// Set whether to emit and error upon pass failure.
+  MlirOptMainConfig &emitErrorOnPassFailure(bool emit) {
+    emitErrorOnPassFailureFlag = emit;
+    return *this;
+  }
+  bool shouldEmitErrorOnPassFailure() const { return emitErrorOnPassFailureFlag; }
+
   /// Set whether to run the verifier on parsing.
   MlirOptMainConfig &verifyOnParsing(bool verify) {
     disableVerifierOnParsingFlag = !verify;
@@ -290,6 +297,9 @@ class MlirOptMainConfig {
 
   /// Run the verifier after each transformation pass.
   bool verifyPassesFlag = true;
+  
+  /// Emit an error upon a pass failure.
+  bool emitErrorOnPassFailureFlag = false;
 
   /// Disable the verifier on parsing.
   bool disableVerifierOnParsingFlag = false;
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 7094c8e279f2d..ff068083e1f33 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -498,6 +498,7 @@ llvm::hash_code OpPassManager::hash() {
 
 LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
                                      AnalysisManager am, bool verifyPasses,
+                                     bool emitErrorOnFailure,
                                      unsigned parentInitGeneration) {
   std::optional<RegisteredOperationName> opInfo = op->getRegisteredInfo();
   if (!opInfo)
@@ -533,9 +534,9 @@ LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
     if (failed(pipeline.initialize(root->getContext(), parentInitGeneration)))
       return failure();
     AnalysisManager nestedAm = root == op ? am : am.nest(root);
-    return OpToOpPassAdaptor::runPipeline(pipeline, root, nestedAm,
-                                          verifyPasses, parentInitGeneration,
-                                          pi, &parentInfo);
+    return OpToOpPassAdaptor::runPipeline(
+        pipeline, root, nestedAm, verifyPasses, emitErrorOnFailure,
+        parentInitGeneration, pi, &parentInfo);
   };
   pass->passState.emplace(op, am, dynamicPipelineCallback);
 
@@ -548,7 +549,7 @@ LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
       [&]() {
         // Invoke the virtual runOnOperation method.
         if (auto *adaptor = dyn_cast<OpToOpPassAdaptor>(pass))
-          adaptor->runOnOperation(verifyPasses);
+          adaptor->runOnOperation(verifyPasses, emitErrorOnFailure);
         else
           pass->runOnOperation();
         passFailed = pass->passState->irAndPassFailed.getInt();
@@ -597,7 +598,8 @@ LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
 /// Run the given operation and analysis manager on a provided op pass manager.
 LogicalResult OpToOpPassAdaptor::runPipeline(
     OpPassManager &pm, Operation *op, AnalysisManager am, bool verifyPasses,
-    unsigned parentInitGeneration, PassInstrumentor *instrumentor,
+    bool emitErrorOnFailure, unsigned parentInitGeneration,
+    PassInstrumentor *instrumentor,
     const PassInstrumentation::PipelineParentInfo *parentInfo) {
   assert((!instrumentor || parentInfo) &&
          "expected parent info if instrumentor is provided");
@@ -616,8 +618,12 @@ LogicalResult OpToOpPassAdaptor::runPipeline(
   }
 
   for (Pass &pass : pm.getPasses())
-    if (failed(run(&pass, op, am, verifyPasses, parentInitGeneration)))
+    if (failed(run(&pass, op, am, verifyPasses, emitErrorOnFailure,
+                   parentInitGeneration))) {
+      if (emitErrorOnFailure)
+        return op->emitError("failed to run pass: ") << pass.getName();
       return failure();
+    }
 
   if (instrumentor) {
     instrumentor->runAfterPipeline(pm.getOpName(*op->getContext()),
@@ -735,15 +741,17 @@ void OpToOpPassAdaptor::runOnOperation() {
 }
 
 /// Run the held pipeline over all nested operations.
-void OpToOpPassAdaptor::runOnOperation(bool verifyPasses) {
+void OpToOpPassAdaptor::runOnOperation(bool verifyPasses,
+                                       bool emitErrorOnFailure) {
   if (getContext().isMultithreadingEnabled())
-    runOnOperationAsyncImpl(verifyPasses);
+    runOnOperationAsyncImpl(verifyPasses, emitErrorOnFailure);
   else
-    runOnOperationImpl(verifyPasses);
+    runOnOperationImpl(verifyPasses, emitErrorOnFailure);
 }
 
 /// Run this pass adaptor synchronously.
-void OpToOpPassAdaptor::runOnOperationImpl(bool verifyPasses) {
+void OpToOpPassAdaptor::runOnOperationImpl(bool verifyPasses,
+                                           bool emitErrorOnFailure) {
   auto am = getAnalysisManager();
   PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
                                                         this};
@@ -758,7 +766,8 @@ void OpToOpPassAdaptor::runOnOperationImpl(bool verifyPasses) {
         // Run the held pipeline over the current operation.
         unsigned initGeneration = mgr->impl->initializationGeneration;
         if (failed(runPipeline(*mgr, &op, am.nest(&op), verifyPasses,
-                               initGeneration, instrumentor, &parentInfo)))
+                               emitErrorOnFailure, initGeneration, instrumentor,
+                               &parentInfo)))
           signalPassFailure();
       }
     }
@@ -775,7 +784,8 @@ static bool hasSizeMismatch(ArrayRef<OpPassManager> lhs,
 }
 
 /// Run this pass adaptor synchronously.
-void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
+void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses,
+                                                bool emitErrorOnFailure) {
   AnalysisManager am = getAnalysisManager();
   MLIRContext *context = &getContext();
 
@@ -838,7 +848,7 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
     // Get the pass manager for this operation and execute it.
     OpPassManager &pm = asyncExecutors[pmIndex][opInfo.passManagerIdx];
     LogicalResult pipelineResult = runPipeline(
-        pm, opInfo.op, opInfo.am, verifyPasses,
+        pm, opInfo.op, opInfo.am, verifyPasses, emitErrorOnFailure,
         pm.impl->initializationGeneration, instrumentor, &parentInfo);
     if (failed(pipelineResult))
       hasFailure.store(true);
@@ -859,17 +869,21 @@ void OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) {
 PassManager::PassManager(MLIRContext *ctx, StringRef operationName,
                          Nesting nesting)
     : OpPassManager(operationName, nesting), context(ctx), passTiming(false),
-      verifyPasses(true) {}
+      verifyPasses(true), emitErrorOnFailure(false) {}
 
 PassManager::PassManager(OperationName operationName, Nesting nesting)
     : OpPassManager(operationName, nesting),
       context(operationName.getContext()), passTiming(false),
-      verifyPasses(true) {}
+      verifyPasses(true), emitErrorOnFailure(false) {}
 
 PassManager::~PassManager() = default;
 
 void PassManager::enableVerifier(bool enabled) { verifyPasses = enabled; }
 
+void PassManager::enableErrorOnFailure(bool enabled) {
+  emitErrorOnFailure = enabled;
+}
+
 /// Run the passes within this manager on the provided operation.
 LogicalResult PassManager::run(Operation *op) {
   MLIRContext *context = getContext();
@@ -931,6 +945,7 @@ void PassManager::addInstrumentation(std::unique_ptr<PassInstrumentation> pi) {
 
 LogicalResult PassManager::runPasses(Operation *op, AnalysisManager am) {
   return OpToOpPassAdaptor::runPipeline(*this, op, am, verifyPasses,
+                                        emitErrorOnFailure,
                                         impl->initializationGeneration);
 }
 
diff --git a/mlir/lib/Pass/PassDetail.h b/mlir/lib/Pass/PassDetail.h
index 5cc726295c9f1..6830debf707ca 100644
--- a/mlir/lib/Pass/PassDetail.h
+++ b/mlir/lib/Pass/PassDetail.h
@@ -29,7 +29,7 @@ class OpToOpPassAdaptor
   OpToOpPassAdaptor(const OpToOpPassAdaptor &rhs) = default;
 
   /// Run the held pipeline over all operations.
-  void runOnOperation(bool verifyPasses);
+  void runOnOperation(bool verifyPasses, bool emitErrorOnFailure);
   void runOnOperation() override;
 
   /// Try to merge the current pass adaptor into 'rhs'. This will try to append
@@ -60,25 +60,29 @@ class OpToOpPassAdaptor
 
 private:
   /// Run this pass adaptor synchronously.
-  void runOnOperationImpl(bool verifyPasses);
+  void runOnOperationImpl(bool verifyPasses, bool emitErrorOnFailure);
 
   /// Run this pass adaptor asynchronously.
-  void runOnOperationAsyncImpl(bool verifyPasses);
+  void runOnOperationAsyncImpl(bool verifyPasses, bool emitErrorOnFailure);
 
   /// Run the given operation and analysis manager on a single pass.
   /// `parentInitGeneration` is the initialization generation of the parent pass
   /// manager, and is used to initialize any dynamic pass pipelines run by the
-  /// given pass.
+  /// given pass. If `emitErrorOnFailure` is set, when a pass in
+  /// the pipeline fails its name will be emitted in an error.
   static LogicalResult run(Pass *pass, Operation *op, AnalysisManager am,
-                           bool verifyPasses, unsigned parentInitGeneration);
+                           bool verifyPasses, bool emitErrorOnFailure,
+                           unsigned parentInitGeneration);
 
   /// Run the given operation and analysis manager on a provided op pass
   /// manager. `parentInitGeneration` is the initialization generation of the
   /// parent pass manager, and is used to initialize any dynamic pass pipelines
-  /// run by the given passes.
+  /// run by the given passes. If `emitErrorOnFailure` is set, when a pass in
+  /// the pipeline fails its name will be emitted in an error.
   static LogicalResult runPipeline(
       OpPassManager &pm, Operation *op, AnalysisManager am, bool verifyPasses,
-      unsigned parentInitGeneration, PassInstrumentor *instrumentor = nullptr,
+      bool emitErrorOnFailure, unsigned parentInitGeneration,
+      PassInstrumentor *instrumentor = nullptr,
       const PassInstrumentation::PipelineParentInfo *parentInfo = nullptr);
 
   /// A set of adaptors to run.
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index de714d8b740af..026b560ac3c46 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -182,6 +182,11 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
         cl::desc("Run the verifier after each transformation pass"),
         cl::location(verifyPassesFlag), cl::init(true));
 
+    static cl::opt<bool, /*ExternalStorage=*/true> emitPassErrorOnFailure(
+        "emit-pass-error-on-failure",
+        cl::desc("Emit an error with the pass name when a pass fails"),
+        cl::location(emitErrorOnPassFailureFlag), cl::init(false));
+
     static cl::opt<bool, /*ExternalStorage=*/true> disableVerifyOnParsing(
         "mlir-very-unsafe-disable-verifier-on-parsing",
         cl::desc("Disable the verifier on parsing (very unsafe)"),
@@ -465,6 +470,7 @@ performActions(raw_ostream &os,
   // Prepare the pass manager, applying command-line and reproducer options.
   PassManager pm(op.get()->getName(), PassManager::Nesting::Implicit);
   pm.enableVerifier(config.shouldVerifyPasses());
+  pm.enableErrorOnFailure(config.shouldEmitErrorOnPassFailure());
   if (failed(applyPassManagerCLOptions(pm)))
     return failure();
   pm.enableTiming(timing);
diff --git a/mlir/test/Pass/pass-name-diagnostics.mlir b/mlir/test/Pass/pass-name-diagnostics.mlir
new file mode 100644
index 0000000000000..55e3ebed7426e
--- /dev/null
+++ b/mlir/test/Pass/pass-name-diagnostics.mlir
@@ -0,0 +1,7 @@
+// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -emit-pass-error-on-failure -verify-diagnostics=only-expected
+
+// expected-error@+2 {{failed to run pass: (anonymous namespace)::TestFailurePass}}
+// expected-error@+1 {{illegal operation}}
+func.func @TestAlwaysIllegalOperationPass1() {
+  return
+}

Copy link

github-actions bot commented Aug 27, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@Cubevoid Cubevoid force-pushed the mlir/pass/emit_errors branch from 1a72695 to 8e00a07 Compare August 27, 2025 00:13
@Cubevoid Cubevoid force-pushed the mlir/pass/emit_errors branch from 8e00a07 to 0ea3cc1 Compare August 27, 2025 00:33
Copy link
Collaborator

@joker-eph joker-eph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a lot of plumbing for this option, I'm not convinced this is pulling its weight right now.

Passes are supposed to diagnose before "signalPassFailure()", so this error is mostly redundant.
I found some passes that were not emitting a diagnostic before failing, but if the goal is just to debug this, then --mlir-print-ir-after-failure has always immediately provided the answer to me.

@Cubevoid
Copy link
Contributor Author

That's a lot of plumbing for this option, I'm not convinced this is pulling its weight right now.

Passes are supposed to diagnose before "signalPassFailure()", so this error is mostly redundant. I found some passes that were not emitting a diagnostic before failing, but if the goal is just to debug this, then --mlir-print-ir-after-failure has always immediately provided the answer to me.

I agree that passes should emit a diagnostic before failing, but in my experience that diagnostic is often too generic to pinpoint on its own without --mlir-print-ir-after-failure. For example, a legalization failure would simply print "failed to legalize <X> op", and emitting the name of the pass where this legalization failed is often helpful in a long compiler pipeline.

Obviously I don't think this option should be the default, but being able to emit this short error might be more convenient than dumping a potentially huge IR for an end user, IMO.

I am open to suggestions though, if there might be a simpler way to communicate this information to a user. There is definitely some nontrivial plumbing right now as you mentioned.

@joker-eph
Copy link
Collaborator

What about a LDBG() output in the pass manager instead? That would show up with --debug-only=pass-manager.
Actually I should add some logging there anyway, it's lacking right now.

@Cubevoid
Copy link
Contributor Author

What about a LDBG() output in the pass manager instead? That would show up with --debug-only=pass-manager. Actually I should add some logging there anyway, it's lacking right now.

I would prefer if these diagnostics are still available in release mode. Depending on the downstream compiler, the downstream might want to enable something like this by default.

@Cubevoid
Copy link
Contributor Author

Another option is to use an environment variable to control this, although that seems a bit hacky/nonconformant with the rest of the MLIR infrastructure.

@joker-eph
Copy link
Collaborator

That's a non-starter IMO.

@joker-eph
Copy link
Collaborator

For production use-cases, I would think that the action mechanism should be the right thing to provide the tracing needed here: https://mlir.llvm.org/docs/ActionTracing/

@joker-eph
Copy link
Collaborator

Or using a pass instrumentation with the runAfterPassFailed hook.

@joker-eph
Copy link
Collaborator

Wait, but that's kind of what --mlir-print-ir-after-failure already does... So now we're back to square one I think?

@joker-eph
Copy link
Collaborator

What about a LDBG() output in the pass manager instead? That would show up with --debug-only=pass-manager. Actually I should add some logging there anyway, it's lacking right now.

That part will be covered in #156205

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants