Page MenuHomePhabricator

[mlir][DialectConversion] Refactor how block argument types get converted

Authored by rriddle on Thu, Jun 11, 12:05 PM.



This revision removes the TypeConverter parameter passed to the apply* methods, and instead moves the responsibility of region type conversion to patterns. The types of a region can be converted using the 'convertRegionTypes' method, which acts similarly to the existing 'applySignatureConversion'. This method ensures that all blocks within, and including those moved into, a region will have the block argument types converted using the provided converter.

This has the benefit of making more of the legalization logic controlled by patterns, instead of being handled explicitly by the driver. It also opens up the possibility to support multiple type conversions at some point in the future.

This revision also adds a new utility class FailureOr<T> that provides a LogicalResult friendly facility for returning a failure or a valid result value.

Depends On D81680

Diff Detail

Event Timeline

rriddle created this revision.Thu, Jun 11, 12:05 PM
rriddle updated this revision to Diff 270914.Mon, Jun 15, 4:46 PM

Update doc to mention convertRegionTypes.

Can you update the doc as well for the apply* replacement:

Done. Type conversion documentation in general should be getting a revamp over the coming weeks.

ftynse accepted this revision.Tue, Jun 16, 2:58 AM
ftynse added inline comments.

Could we initialize it to nullptr?


Nit: this does not seem to return nullptr anymore


Nit: could you please update the doc to say why converter is necessary, in addition to singatureConversion (it is used to materialize casts, and stored for eventual undo).


Detached blocks are assumed to be converted? Or deleted?


Nit: I don't understand what is a "parent converter". Is this left over from some previous version that was looking up the converter in regionToConverter (which would also explain the check for the block to have a parent)


API Nit: this takes TypeConverter * but convrtRegionTypes below takes TypeConverter &. Would be nice to have them homogenized.


Nit: operationsToIgnore ?

This revision is now accepted and ready to land.Tue, Jun 16, 2:58 AM
This revision was automatically updated to reflect the committed changes.
rriddle marked 8 inline comments as done.
rriddle added inline comments.Thu, Jun 18, 4:29 PM

Ooops, thanks for the catch.