This is an archive of the discontinued LLVM Phabricator instance.

[mlir] add types to the transform dialect
ClosedPublic

Authored by ftynse on Oct 4 2022, 8:31 AM.

Details

Summary

Introduce a type system for the transform dialect. A transform IR type
captures the expectations of the transform IR on the payload IR
operations that are being transformed, such as being of a certain kind
or implementing an interface that enables the transformation. This
provides stricter checking and better readability of the transform IR
than using the catch-all "handle" type.

This change implements the basic support for a type system amendable to
dialect extensions and adds a drop-in replacement for the unrestricted
"handle" type. The actual switch of transform dialect ops to that type
will happen in a separate commit.

See https://discourse.llvm.org/t/rfc-type-system-for-the-transform-dialect/65702

Diff Detail

Event Timeline

ftynse created this revision.Oct 4 2022, 8:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 4 2022, 8:31 AM
ftynse requested review of this revision.Oct 4 2022, 8:31 AM
nicolasvasilache added inline comments.
mlir/include/mlir/Dialect/Transform/IR/TransformDialect.td
380

Does this need to be a public method?
Also, add an Impl suffix to make it clearer it is an impl detail?

398

nit: I like to add a redundant public: to separate the methods from contained objects

407

Can you comment on the asymmetry here ?
Why StringRef for one set of hooks and typeId for the second?

This revision is now accepted and ready to land.Oct 10 2022, 11:32 PM
ftynse added inline comments.Oct 11 2022, 1:16 AM
mlir/include/mlir/Dialect/Transform/IR/TransformDialect.td
380

It's not public, there is a private: in line 324.

398

This one is not public either, I wouldn't want to make it such.

ftynse updated this revision to Diff 466732.Oct 11 2022, 1:51 AM
ftynse marked 3 inline comments as done.

Address review.

This revision was automatically updated to reflect the committed changes.