This is an archive of the discontinued LLVM Phabricator instance.

[mlir][memref] Add support for erasing dead allocations.
ClosedPublic

Authored by hanchung on Aug 29 2023, 1:30 PM.

Diff Detail

Event Timeline

hanchung created this revision.Aug 29 2023, 1:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2023, 1:30 PM
hanchung requested review of this revision.Aug 29 2023, 1:30 PM
hanchung updated this revision to Diff 554499.Aug 29 2023, 2:51 PM

fix bazel

hanchung updated this revision to Diff 554521.Aug 29 2023, 4:44 PM

fix bazel

springerm added inline comments.Aug 30 2023, 12:14 AM
mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td
177

Add: It modifies the payload. Dead allocations, loads and stores are silently dropped from all mappings.

mlir/lib/Dialect/MemRef/Utils/MemRefUtils.cpp
128

I would implement in terms of: static bool valueIsNotRead(OpResult opResult, std::vector<Operation *> &users) and call with the result of the AllocOp.

132–133

You can query memory side effects instead of hard-coding the ops here:

Add a templatized copy of bool isMemoryEffectFree(Operation *op); to SideEffectInterface.h and SideEffectInterfaces.cpp, where the side effect can be specified as template parameter. In the implementation, use !hasEffect<EffectTy>(op) instead of !memInterface.hasNoEffect().

Then call this function here:

if (useOp->getNumResults() == 0 && mlir::isSideEffectFree<MemoryEffects::Read>(useOp)) {
  // There are no reads and no alias is created.
  ...
}

(Still need special handling for memref::DeallocOp.)

134

If it's a memref.subview, you can call allUsesAreStores recursively. (Or have a worklist of operations in this function.) Then you can also handle memref.subview(memref.subview(alloc)).

144

You need a RewriterBase &rewriter here and modify the IR with that rewriter. (Pass it from applyToOne.) Otherwise, the transform dialect state will be inconsistent.

146

Could also do this for memref::AllocaOp.

hanchung updated this revision to Diff 555137.Aug 31 2023, 12:13 PM
hanchung marked 3 inline comments as done.

address comments

mlir/lib/Dialect/MemRef/Utils/MemRefUtils.cpp
132–133

I used SideEffect. I'm not very familiar with them, please take a look if I implemented something wrong. Thanks!

134

I don't follow the suggestion. Isn't it already doing what you mentioned? If it is a memref.subview op, it calls the method recursively.

hanchung updated this revision to Diff 555138.Aug 31 2023, 12:15 PM

fix bazel deps

springerm accepted this revision.Aug 31 2023, 10:40 PM
springerm added inline comments.
mlir/lib/Dialect/MemRef/Utils/MemRefUtils.cpp
128

If you keep the Operation * argument, I would rename it to resultIsNotRead.

133

I would also check that useOp has no regions because there could be another use of op inside of a region.

155

rewriter.eraseOp(op)

This revision is now accepted and ready to land.Aug 31 2023, 10:40 PM
hanchung updated this revision to Diff 555468.Sep 1 2023, 12:47 PM
hanchung marked 2 inline comments as done.

address comments

This revision was automatically updated to reflect the committed changes.