Page MenuHomePhabricator

Added documentation for SSA like property in Bufferization.
ClosedPublic

Authored by dfki-jugr on Jan 27 2021, 6:29 AM.

Details

Summary

Added additional information about the SSA like properties
that has to be fulfilled in the bufferization steps.

Diff Detail

Event Timeline

dfki-jugr created this revision.Jan 27 2021, 6:29 AM
dfki-jugr requested review of this revision.Jan 27 2021, 6:29 AM
silvas added inline comments.Jan 27 2021, 11:45 AM
mlir/docs/BufferDeallocationInternals.md
33

I found this wording confusing (such as the term "full buffer write"). I would suggest to just say that all writes to a memref introduced by bufferization must dominate all reads (you say this below in a parenthetical but it should be the primary definition of the constraint).

silvas added inline comments.Jan 27 2021, 11:51 AM
mlir/docs/BufferDeallocationInternals.md
33

And strictly speaking, I would say that all writes must dominate all tensor_load ops. I think it is okay to read and write arbitrarily before the tensor_load.

E.g. this is fine, despite having intermixed reads and writes:

%0 = alloc()
store %0[0] = 42
%1 = load %0[0]
%2 = add %1, 7
store %0[0] = %2
silvas added inline comments.Jan 27 2021, 11:53 AM
mlir/docs/BufferDeallocationInternals.md
33

For example, the bufferization of linalg.matmul effectively results in load/add/store similar to the above.

herhut added inline comments.Jan 28 2021, 8:31 AM
mlir/docs/BufferDeallocationInternals.md
33

I found this wording confusing (such as the term "full buffer write"). I would suggest to just say that all writes to a memref introduced by bufferization must dominate all reads (you say this below in a parenthetical but it should be the primary definition of the constraint).

As you mention yourself, this is not true for bufferization patterns that iteratively produce their value. So this is not good enough either. The goal of this restriction in essence is to ensure that if we need to insert a copy for an allocation as it escapes, we need to be sure that we do not hide mutations. If the allocation dominates all writes, then a copy should never be needed. I have not thought this through, though.

And for bufferization patterns, we would have the requirement that they may only write to buffers they allocated themselves.

herhut accepted this revision.Mar 17 2021, 4:28 AM

I think we should land this as it is strictly an improvement over the current documentation. We can improve further but having it documented avoids more confusion than it creates.

This revision is now accepted and ready to land.Mar 17 2021, 4:28 AM