This is an archive of the discontinued LLVM Phabricator instance.

[mlir:docs] Add proper documentation for defining dialects
ClosedPublic

Authored by rriddle on Apr 6 2022, 2:44 PM.

Details

Summary

We don't actually have any documentation today for how to
declaratively define a dialect. This commit rectifies that and properly
documents how to define a Dialect in tablegen, and details all of
the possible fields.

Diff Detail

Event Timeline

rriddle created this revision.Apr 6 2022, 2:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 2:44 PM
rriddle requested review of this revision.Apr 6 2022, 2:44 PM
jpienaar accepted this revision.Apr 9 2022, 11:44 AM
jpienaar added inline comments.
mlir/docs/DefiningDialects.md
30

I'd remove the second powerful here, feels too much for one sentence.

I'm tempted to say we should mention maintenance as historically I believe the ODS definitions have changed less than the C++ side and also we try to autogenerate many of these details.

41

Interfaces I think it is even a requirement, else you end up with duplicate registrations (for the above, in case you reuse types in dialects you at least end with messy docs, but not sure beyond that)

90

Soon we can use C++17 nested namespace notation :-)

104

s/main// ?

126

Also we may start catering for some of those long tail cases and break this code as we have no visibility into it during generation.

133

s/this// ? (Just too many this in this sentence)

136

I would in life "for canonicalization" into the HREF text

139

Constant materialization logic can then defined in the source file ?

146

Operation generated?

154

A little bit asymmetric to have custom destructor but not constructor.

193

Generate feels weird here (and below)

288

Also it goes from snake_style to lowerCamel (well technically UpperCamel with get/set prefix resulting in lowerCamel). So function_name goes to getFunctionName

This revision is now accepted and ready to land.Apr 9 2022, 11:44 AM
rriddle updated this revision to Diff 422046.Apr 11 2022, 3:00 PM
rriddle edited the summary of this revision. (Show Details)
rriddle marked 9 inline comments as done.
rriddle added inline comments.Apr 11 2022, 3:00 PM
mlir/docs/DefiningDialects.md
41

Yeah, interfaces I think are the most common pitfall of this. I think it could happen with other constructs though, but slightly more difficult (or at least, less common).

90

I cannot wait to move out of C++14!!

154

Yeah, I think that's because we have the initialize method that's invoked by the constructor.

This revision was landed with ongoing or failed builds.Apr 11 2022, 3:33 PM
This revision was automatically updated to reflect the committed changes.