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

[Adrian Kuegel]

Author: Adrian Kuegel
Date: 2024-08-01T08:43:59Z
New Revision: 17ba4f4053e303be3e5408d34eaf687a49cefb06

URL: https://github.com/llvm/llvm-project/commit/17ba4f4053e303be3e5408d34eaf687a49cefb06
DIFF: https://github.com/llvm/llvm-project/commit/17ba4f4053e303be3e5408d34eaf687a49cefb06.diff

LOG: Revert "[mlir][Transforms] Dialect conversion: Skip materializations when running without converter (#101318)"

This reverts commit 2aa96fcf751ee948702e8447de62d6bea8235e3a.

This was merged without a test. Also it seems it was only fixing an
issue for users which used a particular workaround that is not actually
needed anymore (skipping UnrealizedConversionCast operands).

Added: 
    

Modified: 
    mlir/lib/Transforms/Utils/DialectConversion.cpp
    mlir/test/Transforms/test-legalize-type-conversion.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index fdd0175ffae53f..f26aa0a1516a69 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -1316,43 +1316,37 @@ Block *ConversionPatternRewriterImpl::applySignatureConversion(
       continue;
     }
 
-    // This is a 1->1+ mapping.
+    // 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.
     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 07dfb49473f5eb..d0563fed8e5d94 100644
--- a/mlir/test/Transforms/test-legalize-type-conversion.mlir
+++ b/mlir/test/Transforms/test-legalize-type-conversion.mlir
@@ -103,9 +103,8 @@ 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 materialize conversion for block argument #0 that remained live after conversion, type was 'f32'}}
+  // expected-error at below {{failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion}}
   ^bb0(%arg0: f32):
-    // expected-note at below{{see existing live user here}}
     "test.type_consumer"(%arg0) : (f32) -> ()
     "test.return"(%arg0) : (f32) -> ()
   }) : () -> ()