Page MenuHomePhabricator

[mlir] Move AllocationOpInterface to Bufferize/IR/AllocationOpInterface.td.
ClosedPublic

Authored by pifon2a on Thu, Nov 18, 1:37 AM.

Details

Summary

Remove it from op defs in MemRefOps.td and make it an external model.

This is the first PR of many that will move bufferization-related ops,
interfaces, passes to Dialect/Bufferize.

RFC: https://llvm.discourse.group/t/rfc-dialect-for-bufferization-related-ops/4712
It is still debated if the comprehensive bufferization has to be moved there as
well, so for now I am just moving the "gradual" bufferization.

Diff Detail

Event Timeline

pifon2a created this revision.Thu, Nov 18, 1:37 AM
pifon2a requested review of this revision.Thu, Nov 18, 1:37 AM

It wasn't clear to me that an agreement had been reached in the RFC to start populating anything here.

rriddle requested changes to this revision.Thu, Nov 18, 1:50 AM
This revision now requires changes to proceed.Thu, Nov 18, 1:50 AM
pifon2a updated this revision to Diff 388812.Mon, Nov 22, 1:13 AM

Rename the dialect to "bufferization".

It wasn't clear to me that an agreement had been reached in the RFC to start populating anything here.

Could you please unblock it?

herhut accepted this revision.Mon, Nov 22, 1:57 AM

Just minor nits. Thank you!

mlir/include/mlir/Dialect/Bufferization/IR/AllocationOpInterface.td
44

This should not be a default implementation because it only applies to buffers allocated with memref::alloc. So it should be a specific implementation for that op.

For clone, I could envision it grows into a more generic clone, especially as it now is moving to the bufferize dialect and is no longer tied to memref. So there using it as default seems more reasonable, but we also need to weaken its type requirements then.

mlir/lib/Transforms/BufferDeallocation.cpp
621

Is this not generated somewhere?

pifon2a updated this revision to Diff 388839.Mon, Nov 22, 2:52 AM

Address Stephan's comments.

pifon2a added inline comments.Mon, Nov 22, 3:07 AM
mlir/include/mlir/Dialect/Bufferization/IR/AllocationOpInterface.td
44

Removed default implementation for buildDealloc.

mlir/lib/Transforms/BufferDeallocation.cpp
621

Yes, it is. I should have used FallbackModel for that.

ftynse accepted this revision.Mon, Nov 22, 4:20 AM
mehdi_amini accepted this revision.Mon, Nov 22, 11:49 AM
rriddle accepted this revision.Mon, Nov 22, 11:55 AM
This revision is now accepted and ready to land.Mon, Nov 22, 11:55 AM
This revision was landed with ongoing or failed builds.Mon, Nov 22, 12:01 PM
This revision was automatically updated to reflect the committed changes.

This broke the CMake buildbot FYI