Page MenuHomePhabricator

[mlir][bufferization] Better propagation of bufferizesToMemoryWrite through regions

Authored by springerm on Jan 20 2023, 8:15 AM.



bufferizesToMemoryWrite(OpResult) looks for OpOperands that bufferize to memory writes inside the region of the defining op (if it has one). Currently, if the reverse use-def chain stops at any value inside of the region, the OpResult is considered to bufferize to a memory write.

It is always safe to have false positives among bufferizesToMemoryWrite, so the previous implementation is also correct. However, it can lead to additional buffer copies.

Depends On: D141684

Diff Detail

Event Timeline

springerm created this revision.Jan 20 2023, 8:15 AM
springerm requested review of this revision.Jan 20 2023, 8:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2023, 8:15 AM
stevenvar accepted this revision.EditedFeb 13 2023, 3:57 AM
stevenvar added a subscriber: stevenvar.

LGTM, but might need someone more knowledgeable to confirm that the changes are safe.


What was the reasoning before for returning the leaves (even when they did not satisfy the condition) when using this function?

After this change, are there any uses for /*alwaysIncludeLeaves=*/true left?

This revision is now accepted and ready to land.Feb 13 2023, 3:57 AM
springerm added inline comments.Feb 13 2023, 7:56 AM

There is no particular reason. We just never thought about doing this optimization. The reason why you were suddenly seeing copies was due to another change that fixed conflict detection in loops. (There should have been a copy there before.)

We may not have any uses for /*alwaysIncludeLeaves=*/true at the moment, but I have a few local changes that I haven't sent out that may need it again.

This revision was landed with ongoing or failed builds.Feb 13 2023, 7:56 AM
This revision was automatically updated to reflect the committed changes.