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

> Arguably, the issue here with bare pointers is that we have two different type systems, one where memrefs are bare pointers and one where it is the struct.

Interestingly, in the type converter, there is only one "type system". The LLVM type converter never converts a memref type to an LLVM pointer type. It's a lowering pattern that chooses to replace a memref value with a bare pointer, bypassing the type conversion rules (which is totally fine).

The LLVM type converter and the LLVM lowering patterns were somewhat co-designed. They are used together.

> From that perspective it makes sense that the LLVM type converter would convert a bare pointer whose original type was a memref to the struct.

I think it makes sense for "bare pointer -> struct". I'd say that's pretty "standard" materialization. Both "bare pointer" and "memref descriptor struct" are LLVM types and the two concepts are well documented in the MLIR -> LLVM lowering infrastructure.

Hypothetically speaking, a conversion pattern could replace a memref value with a value of another type, let's say `XYZ`. In that case, the conversion framework would ask the type converter to materialize a target conversion from "`XYZ` -> struct", which will fail. Expectedly. That's not a standard conversion. If users want to support such a conversion in a custom pattern, they can always register additional target materializations.

Not quite sure where I'm going with this. I think there are some "standard" materializations that we can expect the type converter to support out of the box (e.g., "bare ptr -> struct"). But not other stuff such as "`i42` -> struct" (which doesn't even make sense...).

> Am I correct that the flang issue now is one of those cases that happened to work and in your original version continue to "happens to work"?

Yes that's correct. But this Flang issue is a known problem, see #118066 for details. We should not let it affect the design here. Either way, we're either going to fix it or find a temporary workaround in Flang.

I think I'm going to revert this PR back to the original version. And also finalize the `ConversionValueMapping` PR, which is the last missing piece for the 1:1 and 1:N merging. Then we can look at both of them and decide whether we want to merge them separately (with a few days in-between), or squash everything together into one commit. 


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