Skip to content

[mlir][Transforms][NFC] Dialect Conversion: Manually populate unresolvedMaterializations #144664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

matthias-springer
Copy link
Member

Manually populate the unresolvedMaterializations map instead of automatically in the UnresolvedMaterializationRewrite constructor. This simplifies the constructor a bit.

This commit is in preparation of the One-Shot Dialect Conversion refactoring: allowPatternRollback = false will in the future trigger immediate materialization of all IR changes.

Depends on #144254.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Jun 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)

Changes

Manually populate the unresolvedMaterializations map instead of automatically in the UnresolvedMaterializationRewrite constructor. This simplifies the constructor a bit.

This commit is in preparation of the One-Shot Dialect Conversion refactoring: allowPatternRollback = false will in the future trigger immediate materialization of all IR changes.

Depends on #144254.


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

1 Files Affected:

  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+5-4)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index c4b85ec4f67d6..8b94bc564d169 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -869,9 +869,10 @@ struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
   /// Append a rewrite. Rewrites are committed upon success and rolled back upon
   /// failure.
   template <typename RewriteTy, typename... Args>
-  void appendRewrite(Args &&...args) {
+  RewriteTy *appendRewrite(Args &&...args) {
     rewrites.push_back(
         std::make_unique<RewriteTy>(*this, std::forward<Args>(args)...));
+    return static_cast<RewriteTy *>(rewrites.back().get());
   }
 
   /// Undo the rewrites (motions, splits) one by one in reverse order until
@@ -1181,7 +1182,6 @@ UnresolvedMaterializationRewrite::UnresolvedMaterializationRewrite(
       mappedValues(std::move(mappedValues)) {
   assert((!originalType || kind == MaterializationKind::Target) &&
          "original type is valid only for target materializations");
-  rewriterImpl.unresolvedMaterializations[op] = this;
 }
 
 void UnresolvedMaterializationRewrite::rollback() {
@@ -1471,8 +1471,9 @@ ValueRange ConversionPatternRewriterImpl::buildUnresolvedMaterialization(
     mapping.map(valuesToMap, convertOp.getResults());
   if (castOp)
     *castOp = convertOp;
-  appendRewrite<UnresolvedMaterializationRewrite>(
-      convertOp, converter, kind, originalType, std::move(valuesToMap));
+  unresolvedMaterializations[convertOp] =
+      appendRewrite<UnresolvedMaterializationRewrite>(
+          convertOp, converter, kind, originalType, std::move(valuesToMap));
   return convertOp.getResults();
 }
 

Copy link
Member

@zero9178 zero9178 left a comment

Choose a reason for hiding this comment

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

LGTM!

Base automatically changed from users/matthias-springer/dialect_conv_erase to main June 18, 2025 12:42
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