Page MenuHomePhabricator

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

Authored by ftynse on Jan 30 2020, 6:16 AM.



Change the conversion materialization hook in TypeConverter to support the materialization of 1-1 conversions. Instead of having the hook as a virtual function, keep it as a callback, (1) it would compose better with different type conversion extensions and (2) this makes the hook optional. Change the hook signature to return Value instead of operation, allowing it to return the original value unmodified when no materialization is necessary for 1-1 conversion.

This diff also includes the changes to the LLVM and GPU dialect conversion to illustrate how this new support makes them simpler.

Diff Detail

Event Timeline

ftynse created this revision.Jan 30 2020, 6:16 AM
ftynse edited the summary of this revision. (Show Details)Jan 30 2020, 6:20 AM
ftynse edited reviewers, added: rriddle; removed: nicolasvasilache.

Unit tests: pass. 62332 tests passed, 0 failed and 838 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

rriddle added inline comments.Jan 30 2020, 10:01 AM

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.

rriddle added inline comments.Jan 30 2020, 10:03 AM

Do you intend for this to apply to PHI/non-entry Block arguments as well?

ftynse marked an inline comment as done.Jan 30 2020, 10:55 AM
ftynse added inline comments.

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.


Actually, no. Is there an easy way to differentiate here? Look at the owner of pointer?

rriddle added inline comments.Jan 31 2020, 10:47 AM

I think we will likely need some kind of indicator on the context of the conversion, in this case a Region signature conversion for the given operation. Ideally the pattern doing the conversion would handle this, but I that doesn't really scale/isn't extensible.

nicolasvasilache resigned from this revision.Jan 31 2020, 11:59 AM