This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Interfaces] Tidy up the documentation for interfaces
ClosedPublic

Authored by rriddle on Dec 8 2020, 3:31 PM.

Details

Summary

The documentation has become a bit stale with age, and doesn't include great documentation for some newer concepts. This revision tidies up a majority of it, with some more cleanup to come in the future. The documentation for the declarative specification is also moved from OpDefinitions.md to Interfaces.md, which is a much more logical place for it to live.

Diff Detail

Event Timeline

rriddle created this revision.Dec 8 2020, 3:31 PM
rriddle requested review of this revision.Dec 8 2020, 3:31 PM
mehdi_amini added inline comments.Dec 8 2020, 4:59 PM
mlir/docs/Interfaces.md
148

Is the text clearly describing the difference / the point between the two types of hooks?

Especially here where the name says "static" but the implementation is just a regular virtual method,.

163

Why do we need an OperationName as argument here?

255
262
269
300

The description is a bit "dry" here: it'd be nice to provide more info to contrast MethodBody and DefaultImplementation so that the reader really understands when to use each.

I'd also clarify the impact of defining the implementation for MethodBody/DefaultImplementation on what is/isn't generated in the trait (mentioned in the example below)

rriddle updated this revision to Diff 310403.Dec 8 2020, 6:28 PM
rriddle marked 5 inline comments as done.

Resolve comments

rriddle marked an inline comment as done.Dec 8 2020, 6:28 PM
rriddle added inline comments.
mlir/docs/Interfaces.md
300

Added more detail and completely revamped the old examples, PTAL.

mehdi_amini accepted this revision.Dec 8 2020, 6:40 PM

Awesome, thanks!

This revision is now accepted and ready to land.Dec 8 2020, 6:40 PM
This revision was landed with ongoing or failed builds.Dec 9 2020, 3:34 PM
This revision was automatically updated to reflect the committed changes.
rriddle marked an inline comment as done.