Skip to content

Commit 43f34f5

Browse files
committed
Added check if there are regions that do not implement the RegionBranchOpInterface.
Add a check if regions do not implement the RegionBranchOpInterface. This is not allowed in the current deallocation steps. Furthermore, we handle edge-cases, where a single region is attached and the parent operation has no results. This fixes: https://bugs.llvm.org/show_bug.cgi?id=48575 Differential Revision: https://reviews.llvm.org/D94586
1 parent cf50f4f commit 43f34f5

File tree

2 files changed

+51
-8
lines changed

2 files changed

+51
-8
lines changed

mlir/lib/Transforms/BufferDeallocation.cpp

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,31 @@ static void walkReturnOperations(Region *region, const FuncT &func) {
7676
}
7777
}
7878

79+
/// Checks if all operations in a given region have at least one attached region
80+
/// that implements the RegionBranchOpInterface. This is not required in edge
81+
/// cases, where we have a single attached region and the parent operation has
82+
/// no results.
83+
static bool validateSupportedControlFlow(Region &region) {
84+
bool success = true;
85+
region.walk([&success](Operation *operation) {
86+
auto regions = operation->getRegions();
87+
// Walk over all operations in a region and check if the operation has at
88+
// least one region and implements the RegionBranchOpInterface. If there
89+
// is an operation that does not fulfill this condition, we cannot apply
90+
// the deallocation steps. Furthermore, we accept cases, where we have a
91+
// region that returns no results, since, in that case, the intra-region
92+
// control flow does not affect the transformation.
93+
size_t size = regions.size();
94+
if (((size == 1 && !operation->getResults().empty()) || size > 1) &&
95+
!dyn_cast<RegionBranchOpInterface>(operation)) {
96+
operation->emitError("All operations with attached regions need to "
97+
"implement the RegionBranchOpInterface.");
98+
success = false;
99+
}
100+
});
101+
return success;
102+
}
103+
79104
namespace {
80105

81106
//===----------------------------------------------------------------------===//
@@ -506,7 +531,12 @@ struct BufferDeallocationPass : BufferDeallocationBase<BufferDeallocationPass> {
506531
if (backedges.size()) {
507532
getFunction().emitError(
508533
"Structured control-flow loops are supported only.");
509-
return;
534+
return signalPassFailure();
535+
}
536+
537+
// Check that the control flow structures are supported.
538+
if (!validateSupportedControlFlow(getFunction().getRegion())) {
539+
return signalPassFailure();
510540
}
511541

512542
// Place all required temporary alloc, copy and dealloc nodes.

mlir/test/Transforms/buffer-deallocation.mlir

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: mlir-opt -buffer-deallocation -split-input-file %s | FileCheck %s
1+
// RUN: mlir-opt -verify-diagnostics -buffer-deallocation -split-input-file %s | FileCheck %s
22

33
// This file checks the behaviour of BufferDeallocation pass for moving and
44
// inserting missing DeallocOps in their correct positions. Furthermore,
@@ -1094,7 +1094,7 @@ func @loop_nested_alloc(
10941094
// The BufferDeallocation transformation should fail on this explicit
10951095
// control-flow loop since they are not supported.
10961096

1097-
// CHECK-LABEL: func @loop_dynalloc
1097+
// expected-error@+1 {{Structured control-flow loops are supported only}}
10981098
func @loop_dynalloc(
10991099
%arg0 : i32,
11001100
%arg1 : i32,
@@ -1121,15 +1121,13 @@ func @loop_dynalloc(
11211121
return
11221122
}
11231123

1124-
// expected-error@+1 {{Structured control-flow loops are supported only}}
1125-
11261124
// -----
11271125

11281126
// Test Case: explicit control-flow loop with a dynamically allocated buffer.
11291127
// The BufferDeallocation transformation should fail on this explicit
11301128
// control-flow loop since they are not supported.
11311129

1132-
// CHECK-LABEL: func @do_loop_alloc
1130+
// expected-error@+1 {{Structured control-flow loops are supported only}}
11331131
func @do_loop_alloc(
11341132
%arg0 : i32,
11351133
%arg1 : i32,
@@ -1155,8 +1153,6 @@ func @do_loop_alloc(
11551153
return
11561154
}
11571155

1158-
// expected-error@+1 {{Structured control-flow loops are supported only}}
1159-
11601156
// -----
11611157

11621158
// CHECK-LABEL: func @assumingOp(
@@ -1193,3 +1189,20 @@ func @assumingOp(
11931189
// CHECK-NEXT: shape.assuming_yield %[[RETURNING_ALLOC]]
11941190
// CHECK: test.copy(%[[ASSUMING_RESULT:.*]], %[[ARG2]])
11951191
// CHECK-NEXT: dealloc %[[ASSUMING_RESULT]]
1192+
1193+
// -----
1194+
1195+
// Test Case: The op "test.bar" does not implement the RegionBranchOpInterface.
1196+
// This is not allowed in buffer deallocation.
1197+
1198+
func @noRegionBranchOpInterface() {
1199+
// expected-error@+1 {{All operations with attached regions need to implement the RegionBranchOpInterface.}}
1200+
%0 = "test.bar"() ( {
1201+
// expected-error@+1 {{All operations with attached regions need to implement the RegionBranchOpInterface.}}
1202+
%1 = "test.bar"() ( {
1203+
"test.yield"() : () -> ()
1204+
}) : () -> (i32)
1205+
"test.yield"() : () -> ()
1206+
}) : () -> (i32)
1207+
"test.terminator"() : () -> ()
1208+
}

0 commit comments

Comments
 (0)