[Mlir-commits] [mlir] [mlir] Improve dialect conversion failure diagnostics (PR #182729)
Jeongseok Son
llvmlistbot at llvm.org
Sun Feb 22 14:26:11 PST 2026
https://github.com/jeongseokson updated https://github.com/llvm/llvm-project/pull/182729
>From 58a4aab6366e7cbea2405aa0a9a90a17e47d79f7 Mon Sep 17 00:00:00 2001
From: Jeongseok Son <jeongseok.son at gmail.com>
Date: Sat, 14 Feb 2026 12:42:54 -0800
Subject: [PATCH 1/4] [mlir] Improve dialect conversion failure diagnostics
---
.../Transforms/Utils/DialectConversion.cpp | 57 +++++++++++++++++--
mlir/test/Transforms/test-legalizer.mlir | 2 +-
2 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index c5facc32e4461..c38441e91ae1d 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -25,7 +25,9 @@
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
#include <optional>
+#include <string>
#include <utility>
using namespace mlir;
@@ -57,6 +59,20 @@ static void logFailure(llvm::ScopedPrinter &os, StringRef fmt, Args &&...args) {
});
}
+/// Format a conversion pattern as "root-op -> (generated-op, ...)".
+static std::string formatPatternForDiagnostics(const Pattern &pattern) {
+ std::string patternStr;
+ llvm::raw_string_ostream os(patternStr);
+ if (std::optional<OperationName> root = pattern.getRootKind())
+ os << root->getStringRef();
+ else
+ os << "*";
+ os << " -> (";
+ llvm::interleaveComma(pattern.getGeneratedOps(), os);
+ os << ")";
+ return patternStr;
+}
+
/// Helper function that computes an insertion point where the given value is
/// defined and can be used without a dominance violation.
static OpBuilder::InsertPoint computeInsertPoint(Value value) {
@@ -2434,6 +2450,10 @@ class OperationLegalizer {
/// was legalized, failure otherwise.
LogicalResult legalize(Operation *op);
+ /// Returns the most recently attempted conversion pattern on this operation
+ /// that failed to match.
+ std::optional<StringRef> getLastFailurePattern(Operation *op) const;
+
/// Returns the conversion target in use by the legalizer.
const ConversionTarget &getTarget() { return target; }
@@ -2510,6 +2530,9 @@ class OperationLegalizer {
/// The pattern applicator to use for conversions.
PatternApplicator applicator;
+
+ /// The last conversion pattern that failed to match for a given op.
+ DenseMap<Operation *, std::string> failedPatternByOp;
};
} // namespace
@@ -2530,6 +2553,14 @@ bool OperationLegalizer::isIllegal(Operation *op) const {
return target.isIllegal(op);
}
+std::optional<StringRef>
+OperationLegalizer::getLastFailurePattern(Operation *op) const {
+ auto it = failedPatternByOp.find(op);
+ if (it == failedPatternByOp.end())
+ return std::nullopt;
+ return StringRef(it->second);
+}
+
LogicalResult OperationLegalizer::legalize(Operation *op) {
#ifndef NDEBUG
const char *logLineComment =
@@ -2540,6 +2571,7 @@ LogicalResult OperationLegalizer::legalize(Operation *op) {
// Check to see if the operation is ignored and doesn't need to be converted.
bool isIgnored = rewriter.getImpl().isOpIgnored(op);
+ failedPatternByOp.erase(op);
LLVM_DEBUG({
logger.getOStream() << "\n";
@@ -2761,6 +2793,7 @@ LogicalResult OperationLegalizer::legalizeWithPattern(Operation *op) {
RewriterState curState = rewriterImpl.getCurrentState();
auto onFailure = [&](const Pattern &pattern) {
assert(rewriterImpl.pendingRootUpdates.empty() && "dangling root updates");
+ failedPatternByOp[op] = formatPatternForDiagnostics(pattern);
if (!rewriterImpl.config.allowPatternRollback) {
// Erase all unresolved materializations.
for (auto op : rewriterImpl.patternMaterializations) {
@@ -2824,6 +2857,8 @@ LogicalResult OperationLegalizer::legalizeWithPattern(Operation *op) {
}
if (config.listener)
config.listener->notifyPatternEnd(pattern, result);
+ if (succeeded(result))
+ failedPatternByOp.erase(op);
return result;
};
@@ -3304,6 +3339,22 @@ struct OperationConverter {
LogicalResult OperationConverter::convert(Operation *op,
bool isRecursiveLegalization) {
const ConversionConfig &config = rewriter.getConfig();
+ auto emitFailedToLegalizeDiag = [&](bool wasExplicitlyIllegal) {
+ InFlightDiagnostic diag =
+ op->emitError() << "failed to legalize operation '" << op->getName()
+ << "'";
+ if (wasExplicitlyIllegal)
+ diag << " that was explicitly marked illegal";
+
+ diag << "; operands (" << op->getOperandTypes() << "), results ("
+ << op->getResultTypes() << ")";
+ if (std::optional<StringRef> pattern =
+ opLegalizer.getLastFailurePattern(op)) {
+ diag << "; rejected by conversion pattern '" << *pattern << "'";
+ } else {
+ diag << "; no conversion pattern matched";
+ }
+ };
// Legalize the given operation.
if (failed(opLegalizer.legalize(op))) {
@@ -3311,8 +3362,7 @@ LogicalResult OperationConverter::convert(Operation *op,
// Full conversions expect all operations to be converted.
if (mode == OpConversionMode::Full) {
if (!isRecursiveLegalization)
- op->emitError() << "failed to legalize operation '" << op->getName()
- << "'";
+ emitFailedToLegalizeDiag(/*wasExplicitlyIllegal=*/false);
return failure();
}
// Partial conversions allow conversions to fail iff the operation was not
@@ -3321,8 +3371,7 @@ LogicalResult OperationConverter::convert(Operation *op,
if (mode == OpConversionMode::Partial) {
if (opLegalizer.isIllegal(op)) {
if (!isRecursiveLegalization)
- op->emitError() << "failed to legalize operation '" << op->getName()
- << "' that was explicitly marked illegal";
+ emitFailedToLegalizeDiag(/*wasExplicitlyIllegal=*/true);
return failure();
}
if (config.unlegalizedOps && !isRecursiveLegalization)
diff --git a/mlir/test/Transforms/test-legalizer.mlir b/mlir/test/Transforms/test-legalizer.mlir
index 8d854aff1992f..4d108f1474b89 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -432,7 +432,7 @@ func.func @test_lookup_without_converter() {
// expected-remark at -1 {{applyPartialConversion failed}}
func.func @test_skip_1to1_pattern(%arg0: f32) {
- // expected-error at +1 {{failed to legalize operation 'test.type_consumer'}}
+ // expected-error at +1 {{failed to legalize operation 'test.type_consumer'.*operands \(f32\), results \(\).*rejected by conversion pattern 'test.type_consumer -> \(\)'}}
"test.type_consumer"(%arg0) : (f32) -> ()
return
}
>From f69c2cace6a12f7bca8b224e4baead5394a60ae4 Mon Sep 17 00:00:00 2001
From: Jeongseok Son <jeongseok.son at gmail.com>
Date: Sat, 14 Feb 2026 12:42:55 -0800
Subject: [PATCH 2/4] [mlir] Fix legalizer diagnostic test expectation
---
mlir/test/Transforms/test-legalizer.mlir | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/test/Transforms/test-legalizer.mlir b/mlir/test/Transforms/test-legalizer.mlir
index 4d108f1474b89..505bb54cd3f8d 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -432,7 +432,7 @@ func.func @test_lookup_without_converter() {
// expected-remark at -1 {{applyPartialConversion failed}}
func.func @test_skip_1to1_pattern(%arg0: f32) {
- // expected-error at +1 {{failed to legalize operation 'test.type_consumer'.*operands \(f32\), results \(\).*rejected by conversion pattern 'test.type_consumer -> \(\)'}}
+ // expected-error at +1 {{failed to legalize operation 'test.type_consumer' that was explicitly marked illegal; operands ('f32'), results (); rejected by conversion pattern 'test.type_consumer -> ()'}}
"test.type_consumer"(%arg0) : (f32) -> ()
return
}
>From eba9e12c564c62f871b52fbd1b290fca6b54906a Mon Sep 17 00:00:00 2001
From: Jeongseok Son <jeongseok.son at gmail.com>
Date: Sun, 22 Feb 2026 00:06:14 -0800
Subject: [PATCH 3/4] [mlir] Refine dialect conversion failure diagnostics
---
.../Transforms/Utils/DialectConversion.cpp | 19 +++++++++----------
mlir/test/Transforms/test-legalizer.mlir | 2 +-
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index c38441e91ae1d..dda33918c742d 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -2452,7 +2452,7 @@ class OperationLegalizer {
/// Returns the most recently attempted conversion pattern on this operation
/// that failed to match.
- std::optional<StringRef> getLastFailurePattern(Operation *op) const;
+ const Pattern *getLastFailurePattern(Operation *op) const;
/// Returns the conversion target in use by the legalizer.
const ConversionTarget &getTarget() { return target; }
@@ -2532,7 +2532,7 @@ class OperationLegalizer {
PatternApplicator applicator;
/// The last conversion pattern that failed to match for a given op.
- DenseMap<Operation *, std::string> failedPatternByOp;
+ DenseMap<Operation *, const Pattern *> failedPatternByOp;
};
} // namespace
@@ -2553,12 +2553,11 @@ bool OperationLegalizer::isIllegal(Operation *op) const {
return target.isIllegal(op);
}
-std::optional<StringRef>
-OperationLegalizer::getLastFailurePattern(Operation *op) const {
+const Pattern *OperationLegalizer::getLastFailurePattern(Operation *op) const {
auto it = failedPatternByOp.find(op);
if (it == failedPatternByOp.end())
- return std::nullopt;
- return StringRef(it->second);
+ return nullptr;
+ return it->second;
}
LogicalResult OperationLegalizer::legalize(Operation *op) {
@@ -2793,7 +2792,7 @@ LogicalResult OperationLegalizer::legalizeWithPattern(Operation *op) {
RewriterState curState = rewriterImpl.getCurrentState();
auto onFailure = [&](const Pattern &pattern) {
assert(rewriterImpl.pendingRootUpdates.empty() && "dangling root updates");
- failedPatternByOp[op] = formatPatternForDiagnostics(pattern);
+ failedPatternByOp[op] = &pattern;
if (!rewriterImpl.config.allowPatternRollback) {
// Erase all unresolved materializations.
for (auto op : rewriterImpl.patternMaterializations) {
@@ -3348,9 +3347,9 @@ LogicalResult OperationConverter::convert(Operation *op,
diag << "; operands (" << op->getOperandTypes() << "), results ("
<< op->getResultTypes() << ")";
- if (std::optional<StringRef> pattern =
- opLegalizer.getLastFailurePattern(op)) {
- diag << "; rejected by conversion pattern '" << *pattern << "'";
+ if (const Pattern *pattern = opLegalizer.getLastFailurePattern(op)) {
+ diag << "; rejected by conversion pattern '"
+ << formatPatternForDiagnostics(*pattern) << "'";
} else {
diag << "; no conversion pattern matched";
}
diff --git a/mlir/test/Transforms/test-legalizer.mlir b/mlir/test/Transforms/test-legalizer.mlir
index 505bb54cd3f8d..d385dcc1dcbeb 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -224,7 +224,7 @@ func.func @bounded_recursion() {
builtin.module {
func.func @fail_to_convert_illegal_op() -> i32 {
- // expected-error at +1 {{failed to legalize operation 'test.illegal_op_f'}}
+ // expected-error at +1 {{failed to legalize operation 'test.illegal_op_f'; operands (), results ('i32'); no conversion pattern matched}}
%result = "test.illegal_op_f"() : () -> (i32)
return %result : i32
}
>From 0c455db921fd7ec873d69f796255ad78bcb17e90 Mon Sep 17 00:00:00 2001
From: Jeongseok Son <jeongseok.son at gmail.com>
Date: Sun, 22 Feb 2026 14:25:42 -0800
Subject: [PATCH 4/4] [mlir] Fix premerge diagnostics test and formatting
---
mlir/lib/Transforms/Utils/DialectConversion.cpp | 6 +++---
mlir/test/Transforms/test-legalizer.mlir | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index dda33918c742d..1aedb07189fd6 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -3339,9 +3339,9 @@ LogicalResult OperationConverter::convert(Operation *op,
bool isRecursiveLegalization) {
const ConversionConfig &config = rewriter.getConfig();
auto emitFailedToLegalizeDiag = [&](bool wasExplicitlyIllegal) {
- InFlightDiagnostic diag =
- op->emitError() << "failed to legalize operation '" << op->getName()
- << "'";
+ InFlightDiagnostic diag = op->emitError()
+ << "failed to legalize operation '"
+ << op->getName() << "'";
if (wasExplicitlyIllegal)
diag << " that was explicitly marked illegal";
diff --git a/mlir/test/Transforms/test-legalizer.mlir b/mlir/test/Transforms/test-legalizer.mlir
index d385dcc1dcbeb..9049698b8e075 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -224,7 +224,7 @@ func.func @bounded_recursion() {
builtin.module {
func.func @fail_to_convert_illegal_op() -> i32 {
- // expected-error at +1 {{failed to legalize operation 'test.illegal_op_f'; operands (), results ('i32'); no conversion pattern matched}}
+ // expected-error at +1 {{failed to legalize operation 'test.illegal_op_f' that was explicitly marked illegal; operands (), results ('i32'); no conversion pattern matched}}
%result = "test.illegal_op_f"() : () -> (i32)
return %result : i32
}
More information about the Mlir-commits
mailing list