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.

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