This is an archive of the discontinued LLVM Phabricator instance.

[mlir] support materialization for 1-1 type conversions
ClosedPublic

Authored by ftynse on May 11 2020, 10:10 AM.

Details

Summary
Dialect conversion infrastructure supports 1->N type conversions by requiring
individual conversions to provide facilities to generate operations
retrofitting N values into 1 of the original type when N > 1. This
functionality can also be used to materialize explicit "cast"-like operations,
but it did not support 1->1 type conversions until now. Modify TypeConverter to
support materialization of cast operations for 1-1 conversions.

This also makes materialization specification more extensible following the
same pattern as type conversions. Instead of overloading a virtual function,
users or subclasses of TypeConversion can now register type-specific
materialization callbacks that will be called in order for the given type.

Diff Detail

Event Timeline

ftynse created this revision.May 11 2020, 10:10 AM
ftynse updated this revision to Diff 263459.May 12 2020, 9:44 AM

Clean up and prepare for review

ftynse retitled this revision from [mlir] WIP: support materialization for 1-1 type conversions to [mlir] support materialization for 1-1 type conversions.May 12 2020, 9:46 AM
ftynse edited the summary of this revision. (Show Details)
yhls added a subscriber: yhls.May 20 2020, 11:18 AM
rriddle accepted this revision.Jun 1 2020, 2:29 PM

(Sorry for the delay)

Looks really clean, thanks!

mlir/include/mlir/Transforms/DialectConversion.h
116

typo: convertibe

117

nit: form:

162

Can we remove the virtual here? That should enable removing the vtable completely.

mlir/lib/Transforms/DialectConversion.cpp
319

Are these duplicated from below?

387–396

nit: Can you just use an if for the initialization?

Value newArg;
if (typeConverter)
  newArg = ...
This revision is now accepted and ready to land.Jun 1 2020, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2020, 2:29 PM
ftynse updated this revision to Diff 267842.Jun 2 2020, 4:24 AM
ftynse marked 6 inline comments as done.
ftynse edited the summary of this revision. (Show Details)

rebase

ftynse added a comment.Jun 2 2020, 4:25 AM

(Sorry for the delay)

No worries! Flang folks seemed to need it, but they did not insist in the end.

mlir/include/mlir/Transforms/DialectConversion.h
162

I don't see any in-tree overrides of this, but let's do it as a separate commit in case somebody depends on it.

mlir/lib/Transforms/DialectConversion.cpp
319

Indeed, removed.

This revision was automatically updated to reflect the committed changes.