Skip to content

[MLIR][Bufferization] Bail on automatic deallocation to enable reentrant behaviour #72289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -861,9 +861,16 @@ BufferDeallocation::handleInterface(MemoryEffectOpInterface op) {

for (auto operand : llvm::make_filter_range(op->getOperands(), isMemref)) {
if (op.getEffectOnValue<MemoryEffects::Free>(operand).has_value()) {
if (!op->hasAttr(BufferizationDialect::kManualDeallocation))
return op->emitError(
"memory free side-effect on MemRef value not supported!");
// If we encounter an automatic allocation, some other pass (or this one
// in a previous invocation) has added it and we shouldn't try to guess it
// if was right or wrong, but we can't do anything, so we just ignore this
// operand.
if (!op->hasAttr(BufferizationDialect::kManualDeallocation)) {
state.resetOwnerships(operand, block);
state.updateOwnership(operand,
buildBoolValue(builder, op.getLoc(), false));
continue;
}

// Buffers that were allocated under "manual deallocation" may be
// manually deallocated. We insert a runtime assertion to cover certain
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: mlir-opt -ownership-based-buffer-deallocation -split-input-file %s | \
// RUN: mlir-opt -ownership-based-buffer-deallocation -split-input-file %s

// This should not be an error because the ownership based buffer deallocation introduces
// deallocs itself, so running it twice over (say when piping IR over different tools with
// their own pipelines) crashes the compiler on perfectly valid code.

func.func @free_effect() {
%alloc = memref.alloc() : memref<2xi32>
%new_alloc = memref.realloc %alloc : memref<2xi32> to memref<4xi32>
return
}

// -----

func.func @free_effect() {
%alloc = memref.alloc() : memref<2xi32>
memref.dealloc %alloc : memref<2xi32>
return
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,24 +66,6 @@ func.func @do_loop_alloc(

// -----

func.func @free_effect() {
%alloc = memref.alloc() : memref<2xi32>
// expected-error @below {{memory free side-effect on MemRef value not supported!}}
%new_alloc = memref.realloc %alloc : memref<2xi32> to memref<4xi32>
return
}

// -----

func.func @free_effect() {
%alloc = memref.alloc() : memref<2xi32>
// expected-error @below {{memory free side-effect on MemRef value not supported!}}
memref.dealloc %alloc : memref<2xi32>
return
}

// -----

func.func @free_effect() {
%true = arith.constant true
%alloc = memref.alloc() : memref<2xi32>
Expand Down