Skip to content

Commit 6ea1d14

Browse files
bcardosolopeslanza
authored andcommitted
[CIR][NFC] Add cleanup region to ScopeOp
This does not change anything in practice, work in that direction should come next. We also want this to not affect existing tests to isolate upcoming changes.
1 parent 8efa1d0 commit 6ea1d14

File tree

7 files changed

+92
-26
lines changed

7 files changed

+92
-26
lines changed

clang/include/clang/CIR/Dialect/IR/CIROps.td

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,23 +1066,29 @@ def ScopeOp : CIR_Op<"scope", [
10661066
The blocks can be terminated by `cir.yield`, `cir.return` or `cir.throw`.
10671067
If `cir.scope` yields no value, the `cir.yield` can be left out, and
10681068
will be inserted implicitly.
1069+
1070+
The scope might also have an associated `cleanup` region, providing code
1071+
that run destruction of automatic variables. Note that in order to lower the
1072+
cleanup region while keeping C++ semantics, all immediate control-flow
1073+
breaking operations not under a children scope should jump to this cleanup
1074+
code.
10691075
}];
10701076

10711077
let results = (outs Optional<CIR_AnyType>:$results);
1072-
let regions = (region AnyRegion:$scopeRegion);
1078+
let regions = (region AnyRegion:$scopeRegion, AnyRegion:$cleanupRegion);
10731079

10741080
let hasVerifier = 1;
10751081
let skipDefaultBuilders = 1;
10761082
let assemblyFormat = [{
1077-
custom<OmittedTerminatorRegion>($scopeRegion) (`:` type($results)^)? attr-dict
1083+
custom<OmittedTerminatorRegion>($scopeRegion, $cleanupRegion) (`:` type($results)^)? attr-dict
10781084
}];
10791085

10801086
let extraClassDeclaration = [{
10811087
/// Determine whether the scope is empty, meaning it contains a single block
10821088
/// terminated by a cir.yield.
10831089
bool isEmpty() {
1084-
auto &entry = getRegion().front();
1085-
return getRegion().hasOneBlock() &&
1090+
auto &entry = getScopeRegion().front();
1091+
return getScopeRegion().hasOneBlock() &&
10861092
llvm::isa<YieldOp>(entry.front());
10871093
}
10881094
}];

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2842,7 +2842,7 @@ mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
28422842
if (auto tryOp =
28432843
llvm::dyn_cast_if_present<cir::TryOp>(entryBlock->getParentOp())) {
28442844
if (auto scopeOp = llvm::dyn_cast<cir::ScopeOp>(tryOp->getParentOp()))
2845-
entryBlock = &scopeOp.getRegion().front();
2845+
entryBlock = &scopeOp.getScopeRegion().front();
28462846
}
28472847

28482848
return emitAlloca(name, ty, loc, alignment,

clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -826,13 +826,16 @@ void CIRGenItaniumCXXABI::emitBeginCatch(CIRGenFunction &CGF,
826826
auto getCatchParamAllocaIP = [&]() {
827827
auto currIns = CGF.getBuilder().saveInsertionPoint();
828828
auto currParent = currIns.getBlock()->getParentOp();
829-
mlir::Operation *scopeLikeOp = currParent->getParentOfType<cir::ScopeOp>();
830-
if (!scopeLikeOp)
831-
scopeLikeOp = currParent->getParentOfType<cir::FuncOp>();
832-
assert(scopeLikeOp && "unknown outermost scope-like parent");
833-
assert(scopeLikeOp->getNumRegions() == 1 && "expected single region");
834829

835-
auto *insertBlock = &scopeLikeOp->getRegion(0).getBlocks().back();
830+
mlir::Block *insertBlock = nullptr;
831+
if (auto scopeOp = currParent->getParentOfType<cir::ScopeOp>()) {
832+
insertBlock = &scopeOp.getScopeRegion().getBlocks().back();
833+
} else if (auto fnOp = currParent->getParentOfType<cir::FuncOp>()) {
834+
insertBlock = &fnOp.getRegion().getBlocks().back();
835+
} else {
836+
llvm_unreachable("unknown outermost scope-like parent");
837+
}
838+
836839
return CGF.getBuilder().getBestAllocaInsertPoint(insertBlock);
837840
};
838841

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -276,22 +276,41 @@ void parseVisibilityAttr(OpAsmParser &parser, cir::VisibilityAttr &visibility) {
276276
// CIR Custom Parsers/Printers
277277
//===----------------------------------------------------------------------===//
278278

279-
static mlir::ParseResult parseOmittedTerminatorRegion(mlir::OpAsmParser &parser,
280-
mlir::Region &region) {
279+
static mlir::ParseResult
280+
parseOmittedTerminatorRegion(mlir::OpAsmParser &parser,
281+
mlir::Region &scopeRegion,
282+
mlir::Region &cleanupRegion) {
281283
auto regionLoc = parser.getCurrentLocation();
282-
if (parser.parseRegion(region))
284+
if (parser.parseRegion(scopeRegion))
283285
return failure();
284-
if (ensureRegionTerm(parser, region, regionLoc).failed())
286+
if (ensureRegionTerm(parser, scopeRegion, regionLoc).failed())
285287
return failure();
288+
289+
// Parse optional cleanup region.
290+
if (parser.parseOptionalKeyword("cleanup").succeeded()) {
291+
if (parser.parseRegion(cleanupRegion, /*arguments=*/{}, /*argTypes=*/{}))
292+
return failure();
293+
if (ensureRegionTerm(parser, cleanupRegion, regionLoc).failed())
294+
return failure();
295+
}
296+
286297
return success();
287298
}
288299

289300
static void printOmittedTerminatorRegion(mlir::OpAsmPrinter &printer,
290301
cir::ScopeOp &op,
291-
mlir::Region &region) {
292-
printer.printRegion(region,
302+
mlir::Region &scopeRegion,
303+
mlir::Region &cleanupRegion) {
304+
printer.printRegion(scopeRegion,
293305
/*printEntryBlockArgs=*/false,
294-
/*printBlockTerminators=*/!omitRegionTerm(region));
306+
/*printBlockTerminators=*/!omitRegionTerm(scopeRegion));
307+
if (!op.getCleanupRegion().empty()) {
308+
printer << " cleanup ";
309+
printer.printRegion(
310+
cleanupRegion,
311+
/*printEntryBlockArgs=*/false,
312+
/*printBlockTerminators=*/!omitRegionTerm(cleanupRegion));
313+
}
295314
}
296315

297316
//===----------------------------------------------------------------------===//
@@ -1251,6 +1270,7 @@ void cir::ScopeOp::getSuccessorRegions(
12511270

12521271
// If the condition isn't constant, both regions may be executed.
12531272
regions.push_back(RegionSuccessor(&getScopeRegion()));
1273+
regions.push_back(RegionSuccessor(&getCleanupRegion()));
12541274
}
12551275

12561276
void cir::ScopeOp::build(
@@ -1261,6 +1281,8 @@ void cir::ScopeOp::build(
12611281
OpBuilder::InsertionGuard guard(builder);
12621282
Region *scopeRegion = result.addRegion();
12631283
builder.createBlock(scopeRegion);
1284+
// cleanup region, do not eagarly create blocks, do it upon demand.
1285+
(void)result.addRegion();
12641286

12651287
mlir::Type yieldTy;
12661288
scopeBuilder(builder, yieldTy, result.location);
@@ -1276,17 +1298,22 @@ void cir::ScopeOp::build(
12761298
OpBuilder::InsertionGuard guard(builder);
12771299
Region *scopeRegion = result.addRegion();
12781300
builder.createBlock(scopeRegion);
1301+
// cleanup region, do not eagarly create blocks, do it upon demand.
1302+
(void)result.addRegion();
12791303
scopeBuilder(builder, result.location);
12801304
}
12811305

12821306
LogicalResult cir::ScopeOp::verify() {
1283-
if (getRegion().empty()) {
1307+
if (getScopeRegion().empty()) {
12841308
return emitOpError() << "cir.scope must not be empty since it should "
12851309
"include at least an implicit cir.yield ";
12861310
}
12871311

1288-
if (getRegion().back().empty() ||
1289-
!getRegion().back().getTerminator()->hasTrait<OpTrait::IsTerminator>())
1312+
if (getScopeRegion().back().empty() ||
1313+
!getScopeRegion()
1314+
.back()
1315+
.getTerminator()
1316+
->hasTrait<OpTrait::IsTerminator>())
12901317
return emitOpError() << "last block of cir.scope must be terminated";
12911318
return success();
12921319
}

clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,9 @@ class CIRScopeOpFlattening : public mlir::OpRewritePattern<cir::ScopeOp> {
141141
continueBlock->addArguments(scopeOp.getResultTypes(), loc);
142142

143143
// Inline body region.
144-
auto *beforeBody = &scopeOp.getRegion().front();
145-
auto *afterBody = &scopeOp.getRegion().back();
146-
rewriter.inlineRegionBefore(scopeOp.getRegion(), continueBlock);
144+
auto *beforeBody = &scopeOp.getScopeRegion().front();
145+
auto *afterBody = &scopeOp.getScopeRegion().back();
146+
rewriter.inlineRegionBefore(scopeOp.getScopeRegion(), continueBlock);
147147

148148
// Save stack and then branch into the body of the region.
149149
rewriter.setInsertionPointToEnd(currentBlock);

clang/lib/CIR/Lowering/ThroughMLIR/LowerCIRToMLIR.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ class CIRScopeOpLowering : public mlir::OpConversionPattern<cir::ScopeOp> {
853853
return mlir::success();
854854
}
855855

856-
for (auto &block : scopeOp.getRegion()) {
856+
for (auto &block : scopeOp.getScopeRegion()) {
857857
rewriter.setInsertionPointToEnd(&block);
858858
auto *terminator = block.getTerminator();
859859
rewriter.replaceOpWithNewOp<mlir::memref::AllocaScopeReturnOp>(

clang/test/CIR/IR/scope.cir

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,18 @@
44

55
module {
66
// Should properly print/parse scope with implicit empty yield.
7+
// CHECK-LABEL: implicit_yield
78
cir.func @implicit_yield() {
89
cir.scope {
910
}
1011
// CHECK: cir.scope {
11-
// CHECK: }
12+
// CHECK-NEXT: }
13+
// CHECK-NEXT: cir.return
1214
cir.return
1315
}
1416

1517
// Should properly print/parse scope with explicit yield.
18+
// CHECK-LABEL: explicit_yield
1619
cir.func @explicit_yield() {
1720
%0 = cir.scope {
1821
%1 = cir.alloca !u32i, !cir.ptr<!u32i>, ["a", init] {alignment = 4 : i64}
@@ -24,4 +27,31 @@ module {
2427
// CHECK: } : !cir.ptr<!u32i>
2528
cir.return
2629
}
30+
31+
// Handle optional cleanup presence
32+
// CHECK-LABEL: empty_cleanup
33+
cir.func @empty_cleanup() {
34+
cir.scope {
35+
} cleanup {
36+
}
37+
// CHECK: cir.scope {
38+
// CHECK-NEXT: } cleanup {
39+
// CHECK-NEXT: }
40+
// CHECK-NEXT: cir.return
41+
cir.return
42+
}
43+
44+
// Handle optional cleanup presence
45+
// CHECK-LABEL: some_cleanup
46+
cir.func @some_cleanup() {
47+
cir.scope {
48+
} cleanup {
49+
%1 = cir.alloca !u32i, !cir.ptr<!u32i>, ["a", init] {alignment = 4 : i64}
50+
}
51+
// CHECK: cir.scope {
52+
// CHECK: } cleanup {
53+
// CHECK: cir.alloca
54+
// CHECK: }
55+
cir.return
56+
}
2757
}

0 commit comments

Comments
 (0)