[PATCH] D73702: [WIP][mlir] enable conversion materialization for 1-1 type conversions

Thu Jan 30 11:03:16 PST 2020

ftynse marked an inline comment as done.
ftynse added inline comments.

================
Comment at: mlir/include/mlir/Transforms/DialectConversion.h:46
+  /// Construct a TypeConverter. Derived classes must provide an implementation
+  /// of the ConversionMaterializer hook materializing a conversion from a set
+  /// of types into one result type by generating a cast operation of some kind.
----------------
rriddle wrote:
> Do we forsee instances where we may want multiple materializers registered? I want to move TypeConverter closer to how ConversionTarget works and allow for it to be more easily extended. I can imagine a situation where we may want to materialize differently depending on the type.
Good point. The usecase I have only needs to materialize memref conversion, hence the change to return values. I'll look into conversion target to make this similarly more extendable.

================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:47

+/// Materializes the "load" operation from a pointer to a memref descriptor.
+/// This is used as a hook for type conversion.
----------------
rriddle wrote:
> Do you intend for this to apply to PHI/non-entry Block arguments as well?
Actually, no. Is there an easy way to differentiate here? Look at the owner of `pointer`?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73702/new/

https://reviews.llvm.org/D73702