Page MenuHomePhabricator

[MLIR] Added documentation and manual to use bufferization features.
ClosedPublic

Authored by dfki-jugr on Nov 3 2020, 3:41 AM.

Details

Summary

Added documentation about the bufferization features.
Furthermore, the usage of pre- and post-processing is described.
This also includes information about optimization functionalities.

Diff Detail

Event Timeline

dfki-jugr created this revision.Nov 3 2020, 3:41 AM
dfki-jugr requested review of this revision.Nov 3 2020, 3:41 AM
mehdi_amini added inline comments.Nov 3 2020, 11:06 AM
mlir/docs/Bufferization.md
4

Nit: Can you run the file through mdformat or a similar tool that would wrap the lines?

28

Do you need this escaping? Here and elsewhere?
Can you write it memref<T> (with backquotes)?

37

Nit spurious empty lines (I assumed mdformat would remove these)

73

Is this intended to be a title of some sort?

mehdi_amini added inline comments.Nov 3 2020, 1:35 PM
mlir/docs/Bufferization.md
92

Is this what Sean is removing here: https://reviews.llvm.org/D89990 ?

dfki-jugr marked 5 inline comments as done.Nov 5 2020, 1:17 AM
dfki-jugr added inline comments.
mlir/docs/Bufferization.md
92

Seems that it will not be removed. So, there is no need to remove it here as well, since the funtionality is still provided. However, if it will be removed in the future, this section can be adapted.

dfki-jugr updated this revision to Diff 303057.Nov 5 2020, 1:20 AM
dfki-jugr marked an inline comment as done.

Addressed comments

herhut accepted this revision.Nov 11 2020, 5:18 AM

Thanks for adding documentation. I think we should land this and improve once landed. That way, it is more visible and more people can work on it if desired.

mlir/docs/Bufferization.md
74

There seems to be something missing here.

242

mega-nit: space after , before memref

382

nit: converts to

385

The last sentence is hard to read. Maybe just remove.

408

Drop the rewrite pattern comment, as this is currently being added.

414

nit: allocation

569

Is this a heading?

758

Thereby? Just drop.

792

Thereby? I think you are looking for a different word.

1040

there is only the...

1169

Are there still limitations wrt. loops and unstructured control flow? That should be mentioned here, too. Another assumption worth mentioning when using copies is that buffers, after creation in a block, are essentially immutable.

mlir/docs/includes/img/branch_example_post_move.svg
96

Can you use more descriptive names here and below?

Just kidding :-)

This revision is now accepted and ready to land.Nov 11 2020, 5:18 AM
dfki-mako accepted this revision.Nov 11 2020, 6:31 AM
silvas accepted this revision.Nov 11 2020, 6:36 PM

Thanks! It would be nice to move the buffer optimization stuff to its own .md file for clarity. I'll update most of the non-optimization stuff after I finish the last part of my refactoring (would appreciate a review on that: https://reviews.llvm.org/D90899).

This revision was landed with ongoing or failed builds.Nov 12 2020, 1:43 AM
This revision was automatically updated to reflect the committed changes.
dfki-jugr marked 12 inline comments as done.

Hello! It looks like this documentation is a bit outdated. For example, setResultConversionKind method was removed and replaced with separate pass (see https://github.com/llvm/llvm-project/commit/f7bc56826616814a656866fd50e90a35a8e461eb). Is there any plans to update this documentation?

Hello! It looks like this documentation is a bit outdated. For example, setResultConversionKind method was removed and replaced with separate pass (see https://github.com/llvm/llvm-project/commit/f7bc56826616814a656866fd50e90a35a8e461eb). Is there any plans to update this documentation?

Hey, we wanted to provide an initial documentation about the bufferization features. Since several parts have been changed in the last days, some of these updates could be missed. I think, an update to the latest changes will be provided soon.