[Mlir-commits] [mlir] [mlir][Transforms] Dialect Conversion: No target mat. for 1:N replacement (PR #117513)

[Markus Böck]

================
@@ -199,24 +211,34 @@ LLVMTypeConverter::LLVMTypeConverter(MLIRContext *ctx,
     // original memref type.
     return builder.create<UnrealizedConversionCastOp>(loc, resultType, desc)
         .getResult(0);
-  });
-  // Add generic source and target materializations to handle cases where
-  // non-LLVM types persist after an LLVM conversion.
-  addSourceMaterialization([&](OpBuilder &builder, Type resultType,
-                               ValueRange inputs, Location loc) {
-    if (inputs.size() != 1)
-      return Value();
+  };
 
-    return builder.create<UnrealizedConversionCastOp>(loc, resultType, inputs)
-        .getResult(0);
-  });
+  // Argument materializations convert from the new block argument types
+  // (multiple SSA values that make up a memref descriptor) back to the
+  // original block argument type.
+  addArgumentMaterialization(unrakedMemRefMaterialization);
+  addArgumentMaterialization(rankedMemRefMaterialization);
+  addSourceMaterialization(unrakedMemRefMaterialization);
+  addSourceMaterialization(rankedMemRefMaterialization);
+
+  // Bare pointer -> Packed MemRef descriptor
----------------
zero9178 wrote:

> > Could you explain why it is desireable to get rid of the argument materailization?
> 
> The purpose of an argument materialization is to convert N replacement values into 1 SSA value. It is a workaround around missing 1:N support in the conversion value mapping. In case of a 1:1 conversion, that workaround is not needed because the conversion value mapping can store 1:1 mappings directly.
> 
> This is also prefetching a bit from #116524, which removes argument materializations entirely. We are going to need the same type converter setup then.
> 
> Basically I'm trying to slowing phase argument materializations over the next 2-3 PRs.

Doesn't it do more than that though? I was under the impression that argument materialization was a special 1:N source materialization, and that the goal of getting rid of it is to merge its functionality into source materializations. 

Prior to this PR, the code in `DialectConversion.cpp` would also cast to the original SSA value type, both in the 1:1 and 1:N case, (assuming no replacement values), as a source materialization would, which is now no longer the case for 1:1 but is still the case for 1:N (assuming `argument == source materailization`).

Regardless of whether my impression with the source and argument materialization check is correct, it feels odd to me that this code would have different semantics, even if just temporarily, for the 1:1 and 1:N cases. 

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