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

[Matthias Springer]

================
@@ -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
----------------
matthias-springer wrote:

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

A source materialization should not be needed at all during `applySignatureConversion` (apart from when block arguments are dropped entirely; but even that could probably be moved to a later point of time). It is only needed when some IR, that still expects the old type, survives the conversion process. That is being taken care of at the end of the conversion process (what used to be "finalize").

The reason why an argument materialization behaves like a source materialization (i.e., back to the original type) is because there's not really any other type it can map to. During `applySignatureConversion` and `replaceOpWithMultiple` we have to decide on an argument materialization result type and the original type is readily available. In theory, any arbitrary type could work, the important thing is that it's a single type that carries enough semantic information for the materialization callbacks to do their work.

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

I agree, there is an asymmetry and it makes sense to keep argument materialization until we remove it everywhere in one go. I added that code path back to the PR. (Also does not require any LLVM type converter changes any more, as you said.)


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