[Mlir-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify `replaceOp` implementation (PR #145155)

[Matthias Springer]

https://github.com/matthias-springer created https://github.com/llvm/llvm-project/pull/145155

Since #145030, `ConversionPatternRewriter::eraseBlock` no longer calls `ConversionPatternRewriter::eraseOp`. This now happens in the rewriter impl (during the cleanup phase). Therefore, a safety check in `replaceOp` can now be simplified.


>From 03ef9e53202bf37ccdaa61bf99ef87f9e6f755b4 Mon Sep 17 00:00:00 2001
From: Matthias Springer <me at m-sp.org>
Date: Sat, 21 Jun 2025 08:51:54 +0000
Subject: [PATCH] [mlir][Transforms] Dialect conversion: Simplify `replaceOp`
 implementation

---
 .../Transforms/Utils/DialectConversion.cpp    | 26 +++++--------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index ad82a007b7996..774d58973eb91 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -1580,21 +1580,18 @@ void ConversionPatternRewriterImpl::replaceOp(
 
   // Check if replaced op is an unresolved materialization, i.e., an
   // unrealized_conversion_cast op that was created by the conversion driver.
-  bool isUnresolvedMaterialization = false;
-  if (auto castOp = dyn_cast<UnrealizedConversionCastOp>(op))
-    if (unresolvedMaterializations.contains(castOp))
-      isUnresolvedMaterialization = true;
+  if (auto castOp = dyn_cast<UnrealizedConversionCastOp>(op)) {
+    // Make sure that the user does not mess with unresolved materializations
+    // that were inserted by the conversion driver. We keep track of these
+    // ops in internal data structures.
+    assert(!unresolvedMaterializations.contains(castOp) &&
+           "attempting to replace/erase an unresolved materialization");
+  }
 
   // Create mappings for each of the new result values.
   for (auto [repl, result] : llvm::zip_equal(newValues, op->getResults())) {
     if (repl.empty()) {
       // This result was dropped and no replacement value was provided.
-      if (isUnresolvedMaterialization) {
-        // Do not create another materializations if we are erasing a
-        // materialization.
-        continue;
-      }
-
       // Materialize a replacement value "out of thin air".
       buildUnresolvedMaterialization(
           MaterializationKind::Source, computeInsertPoint(result),
@@ -1602,15 +1599,6 @@ void ConversionPatternRewriterImpl::replaceOp(
           /*outputTypes=*/result.getType(), /*originalType=*/Type(),
           currentTypeConverter);
       continue;
-    } else {
-      // Make sure that the user does not mess with unresolved materializations
-      // that were inserted by the conversion driver. We keep track of these
-      // ops in internal data structures. Erasing them must be allowed because
-      // this can happen when the user is erasing an entire block (including
-      // its body). But replacing them with another value should be forbidden
-      // to avoid problems with the `mapping`.
-      assert(!isUnresolvedMaterialization &&
-             "attempting to replace an unresolved materialization");
     }
 
     // Remap result to replacement value.