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

I am quite concerned about the necessity and legality of this. https://mlir.llvm.org/docs/DialectConversion/#type-converter states:
> A target materialization converts from a value with an “illegal” source type, to a value of a “legal” type.

My expectation would have been that `!llvm.ptr` is a legal type with no further need for conversion.

Am I right that this is an issue with the `builtin.unrealized_cast` simplifications? I.e. `!llvm.ptr -> memref -> !llvm.struct` being simplified to `!llvm.ptr -> !llvm.struct`, making this target materialization required? I think ideally we would be smarter about this simplification for the purpose of composeability.
I.e. if I have type system `A` that I can convert and materialize to type system `B` which itself can be materialized to type system `C`, ideally I'd not have to implement materializations from `A` to `C`.

(Would still be happy to accept this as a temporary measure if there is some kind of plan and TODO as fixing this goes beyond the scope of this PR)

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