Page MenuHomePhabricator

[mlir][ods] Add base class for simple types
AbandonedPublic

Authored by jpienaar on May 24 2022, 9:36 PM.

Details

Summary

Make it simpler to define types where membership is simply isa and
they are trivial to create. Use in Shape dialect as example.

Diff Detail

Event Timeline

jpienaar created this revision.May 24 2022, 9:36 PM
jpienaar requested review of this revision.May 24 2022, 9:36 PM

We should really be moving away from DialectType, can we move ShapeDialect types to ODS instead?

rriddle added inline comments.May 24 2022, 9:40 PM
mlir/include/mlir/IR/OpBase.td
355

I should really get back to removing these in favor of using the ones defined in BuiltinTypes.td.

We should really be moving away from DialectType, can we move ShapeDialect types to ODS instead?

DialectType and DialectAttribute is only used for docgen at this point.

Sorry didn't get notification of comments.

We should really be moving away from DialectType, can we move ShapeDialect types to ODS instead?

DialectType and DialectAttribute is only used for docgen at this point.

We could, I mostly just used these as an example of the other as was simple change. Would preference be leaving the shape dialect ones as is and just adding the helpers, or use the helpers an migrate them to ODS post? (It's not too many lines either way).

Re usage docgen, yeah I didn't quite see where all it was used else. I would have assumed we could also use it when generating list of types to add to dialect registration. Unless we make some assumptions about file of origin of type definition in ODS and restrict file to single dialect and only it's types, I think we need to know the dialect in generating such a list. Would need to look at that again.

No preference from me

Sorry didn't get notification of comments.

We should really be moving away from DialectType, can we move ShapeDialect types to ODS instead?

DialectType and DialectAttribute is only used for docgen at this point.

We could, I mostly just used these as an example of the other as was simple change. Would preference be leaving the shape dialect ones as is and just adding the helpers, or use the helpers an migrate them to ODS post? (It's not too many lines either way).

I don't have a huge preference, but switching shape dialect to ODS would negate the need for these helpers AFAIK (it already handles setting up predicate and builder).

Flipped Shape.

But I think this is good point for the general case (e.g., Shape dialect wasn't the motivation for the change, just easy test case in core) that where one has such a simple type, to just define the type in ODS instead. And with that in mind, making this case easier is less preferred than migrating folks over, so will abandon this.

jpienaar abandoned this revision.Sat, Jun 25, 9:10 AM