[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:

> "Legality" is a bit of a misnomer in the context of types. Legality is a property of operations (specified via ConversionTarget), not types. The type converter converts a type A to a type B. When when we say "type B is legal", we usually mean that "type B is converted to type B". But we should really say "type B is legal for type A".

Thank you, this makes a lot of sense actually. Rereading the docs using that lense also helps. We should update the docs to clarify in the future.

> Before this PR, we used to insert an argument materialization and a target materialization. But these materializations are not necessary and increase the complexity of the driver. The argument materialization is not needed because it's a 1:1 replacement. The target materialization is not needed because the lowering pattern of the user of the block argument will insert it.

Removing the target materialization makes sense to me. I am not sure I understand the removal of the 1:1 argument materialization and as to why the 1:1 case is special compared to 1:N. Testing locally it also seems to make the `!llvm.ptr -> !llvm.struct` materialization unnecessary. 

Could you explain why it is desireable to get rid of the argument materailization? 

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