This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Rewrite and modernize the documentation for defining Attributes/Types
ClosedPublic

Authored by rriddle on Feb 16 2022, 10:53 PM.

Details

Summary

The current documentation is super old, crusty, and at times wrong. This commit
rewrites the documentation to focus on the TableGen declarative definition,
expounds on various components, and moves the doc out of Tutorials/ and into
a new top level AttributesAndTypes.md doc. As part of this, the AttrDef/TypeDef
documentation in OpDefinitions.md is removed.

Diff Detail

Event Timeline

rriddle created this revision.Feb 16 2022, 10:53 PM
rriddle requested review of this revision.Feb 16 2022, 10:53 PM

Some bits of the docs were rewritten more than others, so it's likely I missed some hardcoded references to just "types" and other things that are just out of date.

mehdi_amini accepted this revision.Feb 21 2022, 2:00 PM

Thanks!

mlir/docs/AttributesAndTypes.md
39
This revision is now accepted and ready to land.Feb 21 2022, 2:00 PM
jpienaar added inline comments.Feb 22 2022, 6:34 AM
mlir/docs/AttributesAndTypes.md
4

extensions is a bit confusing here (and contrasts with title), couldn't this be phrased same as title?

8

This seems like something that is either assumed or needs to be said in all docs.

16

Nit: the C++ data structure used to represent them are value-typed. This may also be confusing considering Value which these are not allowed to have.

19

This feels like a bit of dive into implementation details right of the bat. I think you should include a sentence or two from langref about these to set stage, and you could then move this later post defining and to a using section.

24

proper is a weasel word, please remove.

30

implicit internal feels magical, these are pointers to memory in MLIRContext with helper functions.

38

I think this may be useful place to make.the distinction between the Attribute and AttrDef hierarchies in ODS, folks have ran into that and been confused.

42

May also be good to highlight that these are normally in separate files and why we do that.

64

This feels too much like copy pasta from an existing project.

71

Comment explaining what this does and how it would look and pointing to where the syntax is defined?

72

Comment?

83

I feel this can be reduced to "It's common ... same as for Types." But fine either way.

127

Used for by?

128

s/!/./

136

Link to special?

154

Move this up as this would be preferred/easiest to use.

162

Additional functional?

177

It may be a bug, it makes an assumption about lifetimes which is not enforced.

rriddle updated this revision to Diff 414788.Mar 11 2022, 7:09 PM
rriddle edited the summary of this revision. (Show Details)
rriddle marked 19 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2022, 7:09 PM
mlir/docs/OpDefinitions.md