This is an archive of the discontinued LLVM Phabricator instance.

[mlir][ods] Simplify useDefaultType/AttributePrinterParser
ClosedPublic

Authored by Mogball on May 17 2022, 9:57 AM.

Details

Summary

The current behaviour of useDefaultTypePrinterParser and useDefaultAttributePrinterParser is that they are set by default, but the dialect generator only generates the declarations for the parsing and printing hooks if it sees dialect types and attributes. Same goes for the definitions generated by the AttrOrTypeDef generator.

This can lead to confusing and undesirable behaviour if the dialect generator doesn't see the definitions of the attributes and types, for example, if they are sensibly separated into different files: Dialect.td, Ops.td, Attributes.td, and Types.td.

Now, these bits are unset by default. Setting them will always result in the dialect generator emitting the declarations for the parsing hooks. And if the AttrOrTypeDef generator sees it set, it will generate the default implementations.

Diff Detail

Event Timeline

Mogball created this revision.May 17 2022, 9:57 AM
Mogball requested review of this revision.May 17 2022, 9:57 AM
rriddle added inline comments.May 17 2022, 10:23 AM
mlir/tools/mlir-tblgen/DialectGen.cpp
188

Do we need dialectAttrs or dialectTypes anymore?

Mogball updated this revision to Diff 430137.May 17 2022, 11:26 AM

remove unused args/variables

Mogball marked an inline comment as done.May 17 2022, 11:26 AM
rriddle accepted this revision.May 17 2022, 11:08 PM
This revision was not accepted when it landed; it landed in state Needs Review.May 18 2022, 10:22 AM
This revision was automatically updated to reflect the committed changes.

I am not totally sure I quite grasp the problem you were solving, but I find the current solution unsatisfactory in that we are making the default much less friendly to use. Can we revisit this?

Can you provide an example of the problem?