[Mlir-commits] [mlir] [mlir][Transforms] Dialect conversion: Skip materializations when running without converter (PR #101318)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

Experiment only. Do not merge.

TODO: test case


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


2 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+22-16) 
- (modified) mlir/test/Transforms/test-legalize-type-conversion.mlir (+2-1) 


``````````diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index f26aa0a1516a6..fdd0175ffae53 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -1316,37 +1316,43 @@ Block *ConversionPatternRewriterImpl::applySignatureConversion(
       continue;
     }
 
-    // This is a 1->1+ mapping. 1->N mappings are not fully supported in the
-    // dialect conversion. Therefore, we need an argument materialization to
-    // turn the replacement block arguments into a single SSA value that can be
-    // used as a replacement.
+    // This is a 1->1+ mapping.
     auto replArgs =
         newBlock->getArguments().slice(inputMap->inputNo, inputMap->size);
+    
+    // When there is no type converter, assume that the new block argument
+    // types are legal. This is reasonable to assume because they were
+    // specified by the user.
+    // FIXME: This won't work for 1->N conversions because multiple output
+    // types are not supported in parts of the dialect conversion. In such a
+    // case, we currently use the original block argument type (produced by
+    // the argument materialization).
+    if (!converter && replArgs.size() == 1) {
+      mapping.map(origArg, replArgs[0]);
+      appendRewrite<ReplaceBlockArgRewrite>(block, origArg);
+      continue;
+    }
+
+    // 1->N mappings are not fully supported in the dialect conversion.
+    // Therefore, we need an argument materialization to turn the replacement
+    // block arguments into a single SSA value (of the original type) that can
+    // be used as a replacement.
     Value argMat = buildUnresolvedMaterialization(
         MaterializationKind::Argument, newBlock, newBlock->begin(),
         origArg.getLoc(), /*inputs=*/replArgs, origArgType, converter);
     mapping.map(origArg, argMat);
     appendRewrite<ReplaceBlockArgRewrite>(block, origArg);
 
+    // Now legalize the type by building a target materialization.
     Type legalOutputType;
-    if (converter) {
+    if (converter)
       legalOutputType = converter->convertType(origArgType);
-    } else if (replArgs.size() == 1) {
-      // When there is no type converter, assume that the new block argument
-      // types are legal. This is reasonable to assume because they were
-      // specified by the user.
-      // FIXME: This won't work for 1->N conversions because multiple output
-      // types are not supported in parts of the dialect conversion. In such a
-      // case, we currently use the original block argument type (produced by
-      // the argument materialization).
-      legalOutputType = replArgs[0].getType();
-    }
     if (legalOutputType && legalOutputType != origArgType) {
       Value targetMat = buildUnresolvedTargetMaterialization(
           origArg.getLoc(), argMat, legalOutputType, converter);
       mapping.map(argMat, targetMat);
+      appendRewrite<ReplaceBlockArgRewrite>(block, origArg);
     }
-    appendRewrite<ReplaceBlockArgRewrite>(block, origArg);
   }
 
   appendRewrite<BlockTypeConversionRewrite>(newBlock, block, converter);
diff --git a/mlir/test/Transforms/test-legalize-type-conversion.mlir b/mlir/test/Transforms/test-legalize-type-conversion.mlir
index d0563fed8e5d9..07dfb49473f5e 100644
--- a/mlir/test/Transforms/test-legalize-type-conversion.mlir
+++ b/mlir/test/Transforms/test-legalize-type-conversion.mlir
@@ -103,8 +103,9 @@ func.func @test_block_argument_not_converted() {
 // Make sure argument type changes aren't implicitly forwarded.
 func.func @test_signature_conversion_no_converter() {
   "test.signature_conversion_no_converter"() ({
-  // expected-error at below {{failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion}}
+  // expected-error at below {{failed to materialize conversion for block argument #0 that remained live after conversion, type was 'f32'}}
   ^bb0(%arg0: f32):
+    // expected-note at below{{see existing live user here}}
     "test.type_consumer"(%arg0) : (f32) -> ()
     "test.return"(%arg0) : (f32) -> ()
   }) : () -> ()

``````````

</details>


https://github.com/llvm/llvm-project/pull/101318