[Mlir-commits] [mlir] 8bc0d4d - [mlir][Transforms] Dialect conversion: Add flag to disable rollback (#136490)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Apr 22 00:45:15 PDT 2025
Author: Matthias Springer
Date: 2025-04-22T09:45:11+02:00
New Revision: 8bc0d4d1ccb708d20413876258885a48d3ca6b3d
URL: https://github.com/llvm/llvm-project/commit/8bc0d4d1ccb708d20413876258885a48d3ca6b3d
DIFF: https://github.com/llvm/llvm-project/commit/8bc0d4d1ccb708d20413876258885a48d3ca6b3d.diff
LOG: [mlir][Transforms] Dialect conversion: Add flag to disable rollback (#136490)
This commit adds a new flag to `ConversionConfig` to disallow the
rollback of IR modification. This commit is in preparation of the
One-Shot Dialect Conversion refactoring, which will remove the ability
to roll back IR modifications from the conversion driver.
RFC:
https://discourse.llvm.org/t/rfc-a-new-one-shot-dialect-conversion-driver/79083/46
By default, this flag is set to "true". I.e., the rollback of IR
modifications is allowed. When set to "false", the conversion driver
will report a fatal LLVM error when an IR rollback is requested. The
name of the rolled back pattern is included in the error message.
Moreover, the original IR is no longer restored after a failed
conversion.
Example:
```
within split at llvm-project/mlir/test/Conversion/ArithToSPIRV/fast-math.mlir:1 offset :11:8: error: pattern '(anonymous namespace)::CmpFOpNanKernelPattern' produced IR that could not be legalized
%0 = arith.cmpf ord, %arg0, %arg1 fastmath<fast> : f32
^
within split at llvm-project/mlir/test/Conversion/ArithToSPIRV/fast-math.mlir:1 offset :11:8: note: see current operation: %1 = "arith.cmpf"(%arg0, %arg1) <{fastmath = #arith.fastmath<fast>, predicate = 7 : i64}> : (f32, f32) -> i1
pattern '(anonymous namespace)::CmpFOpNanKernelPattern' rollback of IR modifications requested
UNREACHABLE executed at llvm-project/mlir/lib/Transforms/Utils/DialectConversion.cpp:1231!
```
The majority of patterns in MLIR have already been updated such that
they do not trigger any rollbacks, but a few SPIRV patterns remain. More
information in the RFC.
Added:
Modified:
mlir/include/mlir/Transforms/DialectConversion.h
mlir/lib/Transforms/Utils/DialectConversion.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h b/mlir/include/mlir/Transforms/DialectConversion.h
index b6ab252456e70..b65b3ea971f91 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -1219,6 +1219,26 @@ struct ConversionConfig {
/// materializations and instead inserts "builtin.unrealized_conversion_cast"
/// ops to ensure that the resulting IR is valid.
bool buildMaterializations = true;
+
+ /// If set to "true", pattern rollback is allowed. The conversion driver
+ /// rolls back IR modifications in the following situations.
+ ///
+ /// 1. Pattern implementation returns "failure" after modifying IR.
+ /// 2. Pattern produces IR (in-place modification or new IR) that is illegal
+ /// and cannot be legalized by subsequent foldings / pattern applications.
+ ///
+ /// If set to "false", the conversion driver will produce an LLVM fatal error
+ /// instead of rolling back IR modifications. Moreover, in case of a failed
+ /// conversion, the original IR is not restored. The resulting IR may be a
+ /// mix of original and rewritten IR. (Same as a failed greedy pattern
+ /// rewrite.)
+ ///
+ /// Note: This flag was added in preparation of the One-Shot Dialect
+ /// Conversion refactoring, which will remove the ability to roll back IR
+ /// modifications from the conversion driver. Use this flag to ensure that
+ /// your patterns do not trigger any IR rollbacks. For details, see
+ /// https://discourse.llvm.org/t/rfc-a-new-one-shot-dialect-conversion-driver/79083.
+ bool allowPatternRollback = true;
};
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 4d250329c6f45..eb3ca01a462a6 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -861,8 +861,10 @@ struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
/// conversion process succeeds.
void applyRewrites();
- /// Reset the state of the rewriter to a previously saved point.
- void resetState(RewriterState state);
+ /// Reset the state of the rewriter to a previously saved point. Optionally,
+ /// the name of the pattern that triggered the rollback can specified for
+ /// debugging purposes.
+ void resetState(RewriterState state, StringRef patternName = "");
/// Append a rewrite. Rewrites are committed upon success and rolled back upon
/// failure.
@@ -873,8 +875,9 @@ struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
}
/// Undo the rewrites (motions, splits) one by one in reverse order until
- /// "numRewritesToKeep" rewrites remains.
- void undoRewrites(unsigned numRewritesToKeep = 0);
+ /// "numRewritesToKeep" rewrites remains. Optionally, the name of the pattern
+ /// that triggered the rollback can specified for debugging purposes.
+ void undoRewrites(unsigned numRewritesToKeep = 0, StringRef patternName = "");
/// Remap the given values to those with potentially
diff erent types. Returns
/// success if the values could be remapped, failure otherwise. `valueDiagTag`
@@ -1204,9 +1207,10 @@ RewriterState ConversionPatternRewriterImpl::getCurrentState() {
return RewriterState(rewrites.size(), ignoredOps.size(), replacedOps.size());
}
-void ConversionPatternRewriterImpl::resetState(RewriterState state) {
+void ConversionPatternRewriterImpl::resetState(RewriterState state,
+ StringRef patternName) {
// Undo any rewrites.
- undoRewrites(state.numRewrites);
+ undoRewrites(state.numRewrites, patternName);
// Pop all of the recorded ignored operations that are no longer valid.
while (ignoredOps.size() != state.numIgnoredOperations)
@@ -1216,10 +1220,18 @@ void ConversionPatternRewriterImpl::resetState(RewriterState state) {
replacedOps.pop_back();
}
-void ConversionPatternRewriterImpl::undoRewrites(unsigned numRewritesToKeep) {
+void ConversionPatternRewriterImpl::undoRewrites(unsigned numRewritesToKeep,
+ StringRef patternName) {
for (auto &rewrite :
- llvm::reverse(llvm::drop_begin(rewrites, numRewritesToKeep)))
+ llvm::reverse(llvm::drop_begin(rewrites, numRewritesToKeep))) {
+ if (!config.allowPatternRollback &&
+ !isa<UnresolvedMaterializationRewrite>(rewrite)) {
+ // Unresolved materializations can always be rolled back (erased).
+ llvm::report_fatal_error("pattern '" + patternName +
+ "' rollback of IR modifications requested");
+ }
rewrite->rollback();
+ }
rewrites.resize(numRewritesToKeep);
}
@@ -2158,7 +2170,7 @@ OperationLegalizer::legalizeWithPattern(Operation *op,
});
if (config.listener)
config.listener->notifyPatternEnd(pattern, failure());
- rewriterImpl.resetState(curState);
+ rewriterImpl.resetState(curState, pattern.getDebugName());
appliedPatterns.erase(&pattern);
};
@@ -2168,8 +2180,13 @@ OperationLegalizer::legalizeWithPattern(Operation *op,
assert(rewriterImpl.pendingRootUpdates.empty() && "dangling root updates");
auto result = legalizePatternResult(op, pattern, rewriter, curState);
appliedPatterns.erase(&pattern);
- if (failed(result))
- rewriterImpl.resetState(curState);
+ if (failed(result)) {
+ if (!rewriterImpl.config.allowPatternRollback)
+ op->emitError("pattern '")
+ << pattern.getDebugName()
+ << "' produced IR that could not be legalized";
+ rewriterImpl.resetState(curState, pattern.getDebugName());
+ }
if (config.listener)
config.listener->notifyPatternEnd(pattern, result);
return result;
@@ -2674,9 +2691,20 @@ LogicalResult OperationConverter::convertOperations(ArrayRef<Operation *> ops) {
ConversionPatternRewriter rewriter(ops.front()->getContext(), config);
ConversionPatternRewriterImpl &rewriterImpl = rewriter.getImpl();
- for (auto *op : toConvert)
- if (failed(convert(rewriter, op)))
- return rewriterImpl.undoRewrites(), failure();
+ for (auto *op : toConvert) {
+ if (failed(convert(rewriter, op))) {
+ // Dialect conversion failed.
+ if (rewriterImpl.config.allowPatternRollback) {
+ // Rollback is allowed: restore the original IR.
+ rewriterImpl.undoRewrites();
+ } else {
+ // Rollback is not allowed: apply all modifications that have been
+ // performed so far.
+ rewriterImpl.applyRewrites();
+ }
+ return failure();
+ }
+ }
// After a successful conversion, apply rewrites.
rewriterImpl.applyRewrites();
More information about the Mlir-commits
mailing list