Page MenuHomePhabricator

[mlir][DialectConversion] Enable deeper integration of type conversions
Needs ReviewPublic

Authored by rriddle on Mon, Jun 29, 5:56 PM.

Details

Reviewers
ftynse
Summary

This revision adds support for much deeper type conversion integration into the conversion process, and enables auto-generating cast operations when necessary. Type conversions are now largely automatically managed by the conversion infra when using a ConversionPattern with a provided TypeConverter. This removes the need for patterns to do type cast wrapping themselves and moves the burden to the infra. This makes it much easier to perform partial lowerings when type conversions are involved, as any lingering type conversions will be automatically resolved/legalized by the conversion infra.

To support this new integration, a few changes have been made to the type materialization API on TypeConverter. Materialization has been split into three separate categories:

  • Argument Materialization: This type of materialization is used when converting the type of block arguments when calling convertRegionTypes. This is useful for contextually inserting additional conversion operations when converting a block argument type, such as when converting the types of a function signature.
  • Source Materialization: This type of materialization is used to convert a legal type of the converter into a non-legal type, generally a source type. This may be called when uses of a non-legal type persist after the conversion process has finished.
  • Target Materialization: This type of materialization is used to convert a non-legal, or source, type into a legal, or target, type. This type of materialization is used when applying a pattern on an operation, but the types of the operands have not yet been converted.

Depends On D82830

Diff Detail

Event Timeline

rriddle created this revision.Mon, Jun 29, 5:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Jun 29, 5:56 PM

@ftynse I still need to add tests and rewrite the documentation, but it should be good to start looking over. Thanks.

ftynse added a comment.Thu, Jul 2, 3:42 AM

Thanks, River! Looks good in general. Two high-level questions:

  1. What is the convention on inserting materializations, i.e. who is in charge of inserting one, at which point in the conversion and using which type converter? For example, legalizeChangedResultType checks with the type converter of the user (which it takes from opToConverter, so it is only available for ops created by the infra), but attempts materialization with replConverter that it accepts as an argument, and which may be different.
  2. This looks like it can get into infinite recursion or iteration if materialization produces operations that need more materialization in order to be legalized. Is there something the infra does to guard against it?
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
179

Hmm, can we already call this->convertType() from here? DialectCastOp should be constructible if convertType(resultType) == inputs[0] for source conversions and if convertType(inputs[0]) == resultType for target conversions. I was thinking about how one can check this in the verifier of DialectCastOp, but this may actually be a better place.

mlir/lib/Transforms/DialectConversion.cpp
109

Nit: spurious backslash

399

Nit: might be useful to attach a note pointing to the first live user

580

Initialize to nullptr?

1002

assert(legalTypes.size() <= 1) then?

1008

I'm not sure I read this correctly. A lot (all?) patterns rely on receiving new operands to create new operations, which require Values (sometimes, of some specific type) to be passed in. Is the idea here to fail the conversion if there is a type mismatch, no type converter is provided, and we can't find a mapping to the desired (original) type?

2243

What if the user doesn't have a converter, and the types don't match?