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

This reverts commit 2aa96fc.

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).

Author: akuegel

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);

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@below {{failed to materialize conversion for block argument #0 that remained live after conversion, type was 'f32'}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion}}
   ^bb0(%arg0: f32):
-    // expected-note@below{{see existing live user here}}
     "test.type_consumer"(%arg0) : (f32) -> ()
     "test.return"(%arg0) : (f32) -> ()
   }) : () -> ()