llvm / llvm/llvm-project

[mlir][bufferization] Should `AllocationOpInterface::buildDealloc` be utilized somewhere in `buffer-deallocation-pipeline`?

Open
#172,237 17 comments 0 reactions 0 assignees View on GitHub
mlir:bufferization
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

The old, now removed, `buffer-deallocation` pass used to utilize the `AllocationOpInterface::buildDealloc` method so that users could have more control over how they dealloc memory that is allocated by an op in their own dialect. However, the `buffer-deallocation-pipeline` does not use this and the result is that it's not as easily extensible to custom dialects. Instead, the `populateBufferizationDeallocLoweringPattern` takes a `DeallocHelperMap` which to be honest I have a hard time understanding how this is supposed to work. The builtin `LowerDeallocationsPass` just populates this map using `bufferization::buildDeallocationLibraryFunction`, which I also find to be a bit confusing.

Before, it was so much simpler for users to specify their own dealloc while being able to use builtin passes unmodified (`buffer-deallocation`). Now it seems impossible with `buffer-deallocation-pipeline`. Please correct me if I'm wrong, but it seems like a user would have to define their own `DeallocHelperMap` and use `populateBufferizationDeallocLoweringPattern`. But if they do this then they can't take advantage of the `buffer-deallocation-pipeline` and would have to create their own.

I much prefer being able to use builtin passes that have been tested and vetted by people much more knowledgable about bufferization than I am. And I thought it was really nice that `buildDealloc` worked in tandem with `buffer-deallocation` pass. It would be great to treat `buffer-deallocation-pipeline` the same. Would it be a valid solution to just replace [this](https://github.com/llvm/llvm-project/blob/ecfdf8cb05d13e3be44a4c03adfe2771deb08edd/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp#L67) `memref::DeallocOp` hardcoding with `buildDealloc`? I'm sure this wouldn't be the only change, but I'm just wondering if `buildDealloc` would be an appropriate use here. It seems like it isn't really used at all anymore.

Thanks!

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.