https://github.com/andykaylor updated https://github.com/llvm/llvm-project/pull/210212
>From 99e53ca838a3cb81108310ca00a0601cd511f895 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Wed, 15 Jul 2026 15:08:36 -0700 Subject: [PATCH 1/4] [CIR] Introduce loop cleanup regions For loops and while loops can create variables in their condition regions that require per-iteration cleanup. The CIR dialect previously had no clean way to represent these cleanups while maintaining a separate condition region for the loop operation. This change introduces an optional cleanup region to these loop ops and updates the relevant region successor handling to reflect the insertion of the cleanup region in the control flow when a non-empty cleanup region is present. The CFG flattening pass will handle routing the control flow through the cleanup region in both the normal and EH unwind cases, but this is not yet implemented. That will be added in a follow-up change, as will creation of the cleanup region when it is needed during IR generation. Assisted-by: Cursor / various models --- .../CIR/Dialect/Builder/CIRBaseBuilder.h | 23 ++++ clang/include/clang/CIR/Dialect/IR/CIROps.td | 86 +++++++++++-- .../CIR/Interfaces/CIRLoopOpInterface.td | 13 ++ clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 36 +++++- .../lib/CIR/Interfaces/CIRLoopOpInterface.cpp | 51 +++++--- clang/test/CIR/IR/invalid-loop-cleanup.cir | 81 +++++++++++++ clang/test/CIR/IR/loop-cleanup.cir | 114 ++++++++++++++++++ clang/unittests/CIR/ControlFlowTest.cpp | 70 +++++++++++ 8 files changed, 447 insertions(+), 27 deletions(-) create mode 100644 clang/test/CIR/IR/invalid-loop-cleanup.cir create mode 100644 clang/test/CIR/IR/loop-cleanup.cir diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h index f8a3aca76f102..425fa68ad157e 100644 --- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h +++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h @@ -267,6 +267,17 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { return cir::WhileOp::create(*this, loc, condBuilder, bodyBuilder); } + /// Create a while operation with a per-iteration cleanup region. + cir::WhileOp createWhile( + mlir::Location loc, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> condBuilder, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> bodyBuilder, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> cleanupBuilder, + cir::CleanupKind cleanupKind) { + return cir::WhileOp::create(*this, loc, condBuilder, bodyBuilder, + cleanupBuilder, cleanupKind); + } + /// Create a for operation. cir::ForOp createFor( mlir::Location loc, @@ -277,6 +288,18 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { stepBuilder); } + /// Create a for operation with a per-iteration cleanup region. + cir::ForOp createFor( + mlir::Location loc, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> condBuilder, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> bodyBuilder, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> stepBuilder, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> cleanupBuilder, + cir::CleanupKind cleanupKind) { + return cir::ForOp::create(*this, loc, condBuilder, bodyBuilder, stepBuilder, + cleanupBuilder, cleanupKind); + } + /// Create a break operation. cir::BreakOp createBreak(mlir::Location loc) { return cir::BreakOp::create(*this, loc); diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 762ef56248e4c..976b173db8f94 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -2200,15 +2200,24 @@ class CIR_WhileOpBase<string mnemonic> : CIR_LoopOpBase<mnemonic> { } def CIR_WhileOp : CIR_WhileOpBase<"while"> { - let regions = (region MinSizedRegion<1>:$cond, MinSizedRegion<1>:$body); - let assemblyFormat = "$cond `do` $body attr-dict"; + let arguments = (ins OptionalAttr<CIR_CleanupKindAttr>:$cleanupKind); + let regions = (region MinSizedRegion<1>:$cond, MinSizedRegion<1>:$body, + MaxSizedRegion<1>:$cleanup); + let assemblyFormat = [{ + $cond `do` $body (`cleanup` $cleanupKind $cleanup^)? attr-dict + }]; let description = [{ - Represents a C/C++ while loop. It consists of two regions: + Represents a C/C++ while loop. It consists of two or three regions: - `cond`: single block region with the loop's condition. Should be terminated with a `cir.condition` operation. - `body`: contains the loop body and an arbitrary number of blocks. + - `cleanup`: optional region that runs on every per-iteration exit edge + (condition-false exit, end-of-iteration, break/continue, and EH unwinding + when the cleanup kind includes EH). This is used to destroy a condition + variable whose lifetime is a single iteration. When present, it carries a + cleanup kind matching `cir.cleanup.scope` (`normal` or `all`). Example: @@ -2223,6 +2232,40 @@ def CIR_WhileOp : CIR_WhileOpBase<"while"> { ``` }]; + let builders = [ + OpBuilder<(ins "BuilderCallbackRef":$condBuilder, + "BuilderCallbackRef":$bodyBuilder, + CArg<"BuilderCallbackRef", "nullptr">:$cleanupBuilder, + CArg<"cir::CleanupKind", "cir::CleanupKind::All">:$cleanupKind), [{ + mlir::OpBuilder::InsertionGuard guard($_builder); + $_builder.createBlock($_state.addRegion()); + condBuilder($_builder, $_state.location); + $_builder.createBlock($_state.addRegion()); + bodyBuilder($_builder, $_state.location); + mlir::Region *cleanupRegion = $_state.addRegion(); + if (cleanupBuilder) { + $_state.addAttribute( + getCleanupKindAttrName($_state.name), + cir::CleanupKindAttr::get($_builder.getContext(), cleanupKind)); + $_builder.createBlock(cleanupRegion); + cleanupBuilder($_builder, $_state.location); + } + }]> + ]; + + let extraClassDeclaration = [{ + mlir::Region *maybeGetCleanup() { + return getCleanup().empty() ? nullptr : &getCleanup(); + } + llvm::SmallVector<mlir::Region *> getRegionsInExecutionOrder() { + llvm::SmallVector<mlir::Region *> regions{&getCond(), &getBody()}; + if (mlir::Region *cleanup = maybeGetCleanup()) + regions.push_back(cleanup); + return regions; + } + }]; + + let hasVerifier = 1; let hasLLVMLowering = false; } @@ -2261,12 +2304,18 @@ def CIR_DoWhileOp : CIR_WhileOpBase<"do"> { def CIR_ForOp : CIR_LoopOpBase<"for"> { let summary = "C/C++ for loop counterpart"; let description = [{ - Represents a C/C++ for loop. It consists of three regions: + Represents a C/C++ for loop. It consists of three or four regions: - `cond`: single block region with the loop's condition. Should be terminated with a `cir.condition` operation. - `body`: contains the loop body and an arbitrary number of blocks. - `step`: single block region with the loop's step. + - `cleanup`: optional region that runs on every per-iteration exit edge + (condition-false exit, end-of-iteration after the step, break/continue, + and EH unwinding when the cleanup kind includes EH). This is used to + destroy a condition variable whose lifetime is a single iteration. When + present, it carries a cleanup kind matching `cir.cleanup.scope` (`normal` + or `all`). Example: @@ -2283,20 +2332,25 @@ def CIR_ForOp : CIR_LoopOpBase<"for"> { ``` }]; + let arguments = (ins OptionalAttr<CIR_CleanupKindAttr>:$cleanupKind); let regions = (region MinSizedRegion<1>:$cond, MinSizedRegion<1>:$body, - MinSizedRegion<1>:$step); + MinSizedRegion<1>:$step, + MaxSizedRegion<1>:$cleanup); let assemblyFormat = [{ `:` `cond` $cond `body` $body `step` $step + (`cleanup` $cleanupKind $cleanup^)? attr-dict }]; let builders = [ OpBuilder<(ins "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>":$condBuilder, "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>":$bodyBuilder, - "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>":$stepBuilder), [{ + "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>":$stepBuilder, + CArg<"llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>", "nullptr">:$cleanupBuilder, + CArg<"cir::CleanupKind", "cir::CleanupKind::All">:$cleanupKind), [{ mlir::OpBuilder::InsertionGuard guard($_builder); // Build condition region. @@ -2310,16 +2364,34 @@ def CIR_ForOp : CIR_LoopOpBase<"for"> { // Build step region. $_builder.createBlock($_state.addRegion()); stepBuilder($_builder, $_state.location); + + // Build optional cleanup region. + mlir::Region *cleanupRegion = $_state.addRegion(); + if (cleanupBuilder) { + $_state.addAttribute( + getCleanupKindAttrName($_state.name), + cir::CleanupKindAttr::get($_builder.getContext(), cleanupKind)); + $_builder.createBlock(cleanupRegion); + cleanupBuilder($_builder, $_state.location); + } }]> ]; let extraClassDeclaration = [{ mlir::Region *maybeGetStep() { return &getStep(); } + mlir::Region *maybeGetCleanup() { + return getCleanup().empty() ? nullptr : &getCleanup(); + } llvm::SmallVector<mlir::Region *> getRegionsInExecutionOrder() { - return llvm::SmallVector<mlir::Region *, 3>{&getCond(), &getBody(), &getStep()}; + llvm::SmallVector<mlir::Region *> regions{&getCond(), &getBody(), + &getStep()}; + if (mlir::Region *cleanup = maybeGetCleanup()) + regions.push_back(cleanup); + return regions; } }]; + let hasVerifier = 1; let hasLLVMLowering = false; } diff --git a/clang/include/clang/CIR/Interfaces/CIRLoopOpInterface.td b/clang/include/clang/CIR/Interfaces/CIRLoopOpInterface.td index 71e3d934b9da1..01b78a2a8b413 100644 --- a/clang/include/clang/CIR/Interfaces/CIRLoopOpInterface.td +++ b/clang/include/clang/CIR/Interfaces/CIRLoopOpInterface.td @@ -45,6 +45,19 @@ def LoopOpInterface : OpInterface<"LoopOpInterface", [ /*methodBody=*/"", /*defaultImplementation=*/"return nullptr;" >, + InterfaceMethod<[{ + Returns a pointer to the loop's per-iteration cleanup region, or + nullptr if the loop has no (non-empty) cleanup region. When present, + this region runs on every per-iteration exit edge (condition-false + exit, end-of-iteration, break/continue, and EH unwinding when the + cleanup kind includes EH). + }], + /*retTy=*/"mlir::Region *", + /*methodName=*/"maybeGetCleanup", + /*args=*/(ins), + /*methodBody=*/"", + /*defaultImplementation=*/"return nullptr;" + >, InterfaceMethod<[{ Returns the first region to be executed in the loop. }], diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index c519229a02f26..057ec610bdc8a 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -502,10 +502,14 @@ void cir::ConditionOp::getSuccessorRegions( // TODO(cir): The condition value may be folded to a constant, narrowing // down its list of possible successors. - // Parent is a loop: condition may branch to the body or to the parent op. + // Parent is a loop: condition may branch to the body, or on the false edge + // to the per-iteration cleanup region if present, otherwise to the parent op. if (auto loopOp = dyn_cast<LoopOpInterface>(getOperation()->getParentOp())) { regions.emplace_back(&loopOp.getBody()); - regions.emplace_back(getOperation()); + if (mlir::Region *cleanup = loopOp.maybeGetCleanup()) + regions.emplace_back(cleanup); + else + regions.emplace_back(getOperation()); return; } @@ -533,6 +537,34 @@ LogicalResult cir::ConditionOp::verify() { return success(); } +//===----------------------------------------------------------------------===// +// WhileOp & ForOp +//===----------------------------------------------------------------------===// + +template <typename LoopOpTy> +static LogicalResult verifyLoopCleanup(LoopOpTy op) { + std::optional<cir::CleanupKind> cleanupKind = op.getCleanupKind(); + + // The cleanup kind attribute must be present exactly when a (non-empty) + // cleanup region is present. + if (cleanupKind.has_value() == op.getCleanup().empty()) + return op.emitOpError("cleanup kind must be present if and only if the " + "cleanup region is non-empty"); + + // A loop's per-iteration cleanup runs on every normal exit edge (loop exit, + // end of iteration, break/continue), so an EH-only cleanup is meaningless. + // Only 'normal' (exceptions disabled) and 'all' (normal + EH unwind) apply. + if (cleanupKind == cir::CleanupKind::EH) + return op.emitOpError("loop cleanup kind must be 'normal' or 'all', " + "not 'eh'"); + + return success(); +} + +LogicalResult cir::WhileOp::verify() { return verifyLoopCleanup(*this); } + +LogicalResult cir::ForOp::verify() { return verifyLoopCleanup(*this); } + //===----------------------------------------------------------------------===// // ConstantOp //===----------------------------------------------------------------------===// diff --git a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp b/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp index c2a6ef81f17b8..1dad9acb62e48 100644 --- a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp +++ b/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp @@ -28,25 +28,48 @@ void LoopOpInterface::getLoopOpSuccessorRegions( mlir::Region *parentRegion = point.getTerminatorPredecessorOrNull()->getParentRegion(); - // Branching from condition: go to body or exit. + mlir::Region *step = op.maybeGetStep(); + mlir::Region *cleanup = op.maybeGetCleanup(); + + // Branching from condition: go to body, or (on the false edge) route + // through the cleanup region if present, otherwise exit the loop. if (&op.getCond() == parentRegion) { - regions.emplace_back(op); + if (cleanup) + regions.emplace_back(cleanup); + else + regions.emplace_back(op); regions.emplace_back(&op.getBody()); return; } - // Branching from body: go to step (for) or condition. + // Branching from body: go to step (for), otherwise route through the + // cleanup region if present, otherwise go back to the condition. if (&op.getBody() == parentRegion) { // FIXME(cir): Should we consider break/continue statements here? - mlir::Region *afterBody = - (op.maybeGetStep() ? op.maybeGetStep() : &op.getCond()); - regions.emplace_back(afterBody); + if (step) + regions.emplace_back(step); + else if (cleanup) + regions.emplace_back(cleanup); + else + regions.emplace_back(&op.getCond()); + return; + } + + // Branching from step: route through the cleanup region if present, + // otherwise go back to the condition. + if (step == parentRegion) { + if (cleanup) + regions.emplace_back(cleanup); + else + regions.emplace_back(&op.getCond()); return; } - // Branching from step: go to condition. - if (op.maybeGetStep() == parentRegion) { + // Branching from cleanup: either loop back to the condition (normal + // end-of-iteration) or exit the loop (condition-false edge). + if (cleanup == parentRegion) { regions.emplace_back(&op.getCond()); + regions.emplace_back(op); return; } @@ -58,16 +81,8 @@ LoopOpInterface::getLoopOpSuccessorInputs(LoopOpInterface op, mlir::RegionSuccessor successor) { if (successor.isOperation()) return op->getResults(); - if (successor == &op.getEntry()) - return op.getEntry().getArguments(); - if (successor == &op.getBody()) - return op.getBody().getArguments(); - mlir::Region *afterBody = - (op.maybeGetStep() ? op.maybeGetStep() : &op.getCond()); - if (successor == afterBody) - return afterBody->getArguments(); - if (successor == &op.getCond()) - return op.getCond().getArguments(); + if (mlir::Region *region = successor.getSuccessor()) + return region->getArguments(); llvm_unreachable("invalid region successor"); } diff --git a/clang/test/CIR/IR/invalid-loop-cleanup.cir b/clang/test/CIR/IR/invalid-loop-cleanup.cir new file mode 100644 index 0000000000000..c8efedb5ddbd6 --- /dev/null +++ b/clang/test/CIR/IR/invalid-loop-cleanup.cir @@ -0,0 +1,81 @@ +// RUN: cir-opt %s -verify-diagnostics -split-input-file + +!s32i = !cir.int<s, 32> + +cir.func private @dtor(!cir.ptr<!s32i>) + +cir.func @while_cleanup_missing_kind(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.while { + cir.condition(%cond) + } do { + cir.yield + // expected-error @below {{expected valid keyword or string}} + // expected-error @below {{failed to parse CIR_CleanupKindAttr}} + } cleanup { + cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.yield + } + cir.return +} + +// ----- + +!s32i = !cir.int<s, 32> + +cir.func private @dtor(!cir.ptr<!s32i>) + +cir.func @for_cleanup_missing_kind(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.for : cond { + cir.condition(%cond) + } body { + cir.yield + } step { + cir.yield + // expected-error @below {{expected valid keyword or string}} + // expected-error @below {{failed to parse CIR_CleanupKindAttr}} + } cleanup { + cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.yield + } + cir.return +} + +// ----- + +!s32i = !cir.int<s, 32> + +cir.func private @dtor(!cir.ptr<!s32i>) + +cir.func @while_cleanup_eh(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + // expected-error @below {{loop cleanup kind must be 'normal' or 'all', not 'eh'}} + cir.while { + cir.condition(%cond) + } do { + cir.yield + } cleanup eh { + cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.yield + } + cir.return +} + +// ----- + +!s32i = !cir.int<s, 32> + +cir.func private @dtor(!cir.ptr<!s32i>) + +cir.func @for_cleanup_eh(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + // expected-error @below {{loop cleanup kind must be 'normal' or 'all', not 'eh'}} + cir.for : cond { + cir.condition(%cond) + } body { + cir.yield + } step { + cir.yield + } cleanup eh { + cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.yield + } + cir.return +} diff --git a/clang/test/CIR/IR/loop-cleanup.cir b/clang/test/CIR/IR/loop-cleanup.cir new file mode 100644 index 0000000000000..bdc896a7ba822 --- /dev/null +++ b/clang/test/CIR/IR/loop-cleanup.cir @@ -0,0 +1,114 @@ +// RUN: cir-opt %s --verify-roundtrip | FileCheck %s + +!s32i = !cir.int<s, 32> + +module { + cir.func private @dtor(!cir.ptr<!s32i>) + + // A while loop with a per-iteration cleanup region that runs on both the + // normal and EH exit edges. + cir.func @while_cleanup_all(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.while { + cir.condition(%cond) + } do { + cir.yield + } cleanup all { + cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.yield + } + cir.return + } + + // CHECK-LABEL: cir.func @while_cleanup_all + // CHECK: cir.while { + // CHECK: cir.condition(%[[COND:.*]]) + // CHECK: } do { + // CHECK: cir.yield + // CHECK: } cleanup all { + // CHECK: cir.call @dtor + // CHECK: cir.yield + // CHECK: } + + // A while loop with a normal-only cleanup region (exceptions disabled). + cir.func @while_cleanup_normal(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.while { + cir.condition(%cond) + } do { + cir.yield + } cleanup normal { + cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.yield + } + cir.return + } + + // CHECK-LABEL: cir.func @while_cleanup_normal + // CHECK: } cleanup normal { + // CHECK: cir.call @dtor + // CHECK: } + + // A while loop without a cleanup region still prints as before. + cir.func @while_no_cleanup(%cond : !cir.bool) { + cir.while { + cir.condition(%cond) + } do { + cir.yield + } + cir.return + } + + // CHECK-LABEL: cir.func @while_no_cleanup + // CHECK: cir.while { + // CHECK: cir.condition + // CHECK: } do { + // CHECK: cir.yield + // CHECK-NOT: cleanup + + // A for loop with a per-iteration cleanup region after the step region. + cir.func @for_cleanup_all(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.for : cond { + cir.condition(%cond) + } body { + cir.yield + } step { + cir.yield + } cleanup all { + cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.yield + } + cir.return + } + + // CHECK-LABEL: cir.func @for_cleanup_all + // CHECK: cir.for : cond { + // CHECK: cir.condition(%[[FCOND:.*]]) + // CHECK: } body { + // CHECK: cir.yield + // CHECK: } step { + // CHECK: cir.yield + // CHECK: } cleanup all { + // CHECK: cir.call @dtor + // CHECK: cir.yield + // CHECK: } + + // A for loop without a cleanup region still prints as before. + cir.func @for_no_cleanup(%cond : !cir.bool) { + cir.for : cond { + cir.condition(%cond) + } body { + cir.yield + } step { + cir.yield + } + cir.return + } + + // CHECK-LABEL: cir.func @for_no_cleanup + // CHECK: cir.for : cond { + // CHECK: cir.condition + // CHECK: } body { + // CHECK: cir.yield + // CHECK: } step { + // CHECK: cir.yield + // CHECK-NOT: cleanup +} diff --git a/clang/unittests/CIR/ControlFlowTest.cpp b/clang/unittests/CIR/ControlFlowTest.cpp index b31d425dd0555..2aa00a9c5089a 100644 --- a/clang/unittests/CIR/ControlFlowTest.cpp +++ b/clang/unittests/CIR/ControlFlowTest.cpp @@ -418,6 +418,76 @@ TEST_F(CIRControlFlowTest, DoWhileOp) { verifyControlFlowInterfaceConsistency(doWhileOp); } +TEST_F(CIRControlFlowTest, WhileOpWithCleanup) { + OwningOpRef<ModuleOp> module = parse(R"CIR( + cir.func @f(%cond : !cir.bool) { + cir.while { + cir.condition(%cond) + } do { + cir.yield + } cleanup all { + cir.yield + } + cir.return + } + )CIR"); + auto whileOp = findFirstOp<cir::WhileOp>(*module); + Region *cleanup = whileOp.maybeGetCleanup(); + ASSERT_NE(cleanup, nullptr); + + // Parent enters the condition region. + expectSuccessors(whileOp, RegionBranchPoint::parent(), {&whileOp.getCond()}); + + // Condition branches to the body or, on the false edge, the cleanup region. + expectTerminatorSuccessors(whileOp.getCond(), + {&whileOp.getBody(), cleanup}); + + // Body routes through the cleanup region (there is no step region). + expectTerminatorSuccessors(whileOp.getBody(), {cleanup}); + + // Cleanup loops back to the condition or exits the loop. + expectTerminatorSuccessors(*cleanup, {&whileOp.getCond(), nullptr}); + + verifyControlFlowInterfaceConsistency(whileOp); +} + +TEST_F(CIRControlFlowTest, ForOpWithCleanup) { + OwningOpRef<ModuleOp> module = parse(R"CIR( + cir.func @f(%cond : !cir.bool) { + cir.for : cond { + cir.condition(%cond) + } body { + cir.yield + } step { + cir.yield + } cleanup all { + cir.yield + } + cir.return + } + )CIR"); + auto forOp = findFirstOp<cir::ForOp>(*module); + Region *cleanup = forOp.maybeGetCleanup(); + ASSERT_NE(cleanup, nullptr); + + // Parent enters the condition region. + expectSuccessors(forOp, RegionBranchPoint::parent(), {&forOp.getCond()}); + + // Condition branches to the body or, on the false edge, the cleanup region. + expectTerminatorSuccessors(forOp.getCond(), {&forOp.getBody(), cleanup}); + + // Body goes to the step region. + expectTerminatorSuccessors(forOp.getBody(), {&forOp.getStep()}); + + // Step routes through the cleanup region. + expectTerminatorSuccessors(forOp.getStep(), {cleanup}); + + // Cleanup loops back to the condition or exits the loop. + expectTerminatorSuccessors(*cleanup, {&forOp.getCond(), nullptr}); + + verifyControlFlowInterfaceConsistency(forOp); +} + TEST_F(CIRControlFlowTest, TryOpWithCatchAll) { OwningOpRef<ModuleOp> module = parse(R"CIR( !void = !cir.void >From fea41b7f344f7451b8b16c3114120d79996afc92 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Thu, 16 Jul 2026 17:00:47 -0700 Subject: [PATCH 2/4] Fix formatting --- clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h | 6 ++++-- clang/unittests/CIR/ControlFlowTest.cpp | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h index 425fa68ad157e..fbbf54314191c 100644 --- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h +++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h @@ -272,7 +272,8 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { mlir::Location loc, llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> condBuilder, llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> bodyBuilder, - llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> cleanupBuilder, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> + cleanupBuilder, cir::CleanupKind cleanupKind) { return cir::WhileOp::create(*this, loc, condBuilder, bodyBuilder, cleanupBuilder, cleanupKind); @@ -294,7 +295,8 @@ class CIRBaseBuilderTy : public mlir::OpBuilder { llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> condBuilder, llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> bodyBuilder, llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> stepBuilder, - llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> cleanupBuilder, + llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)> + cleanupBuilder, cir::CleanupKind cleanupKind) { return cir::ForOp::create(*this, loc, condBuilder, bodyBuilder, stepBuilder, cleanupBuilder, cleanupKind); diff --git a/clang/unittests/CIR/ControlFlowTest.cpp b/clang/unittests/CIR/ControlFlowTest.cpp index 2aa00a9c5089a..07df0bb839364 100644 --- a/clang/unittests/CIR/ControlFlowTest.cpp +++ b/clang/unittests/CIR/ControlFlowTest.cpp @@ -439,8 +439,7 @@ TEST_F(CIRControlFlowTest, WhileOpWithCleanup) { expectSuccessors(whileOp, RegionBranchPoint::parent(), {&whileOp.getCond()}); // Condition branches to the body or, on the false edge, the cleanup region. - expectTerminatorSuccessors(whileOp.getCond(), - {&whileOp.getBody(), cleanup}); + expectTerminatorSuccessors(whileOp.getCond(), {&whileOp.getBody(), cleanup}); // Body routes through the cleanup region (there is no step region). expectTerminatorSuccessors(whileOp.getBody(), {cleanup}); >From 198dd2ef5be1f6d90714ce76e977c4c99e41f057 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Thu, 16 Jul 2026 18:25:40 -0700 Subject: [PATCH 3/4] Make tests more reflective of the expected actual structure --- clang/test/CIR/IR/invalid-loop-cleanup.cir | 60 ++++++++++---- clang/test/CIR/IR/loop-cleanup.cir | 93 +++++++++------------- 2 files changed, 83 insertions(+), 70 deletions(-) diff --git a/clang/test/CIR/IR/invalid-loop-cleanup.cir b/clang/test/CIR/IR/invalid-loop-cleanup.cir index c8efedb5ddbd6..f48ac44cd1541 100644 --- a/clang/test/CIR/IR/invalid-loop-cleanup.cir +++ b/clang/test/CIR/IR/invalid-loop-cleanup.cir @@ -1,18 +1,25 @@ // RUN: cir-opt %s -verify-diagnostics -split-input-file -!s32i = !cir.int<s, 32> +!u8i = !cir.int<u, 8> +!rec_S = !cir.struct<"S" padded {!u8i}> -cir.func private @dtor(!cir.ptr<!s32i>) +cir.func private @ctor(!cir.ptr<!rec_S>) +cir.func private @operatorBool(!cir.ptr<!rec_S>) -> !cir.bool +cir.func private @dtor(!cir.ptr<!rec_S>) func_info<#cir.cxx_dtor<!rec_S>> + attributes {nothrow} -cir.func @while_cleanup_missing_kind(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { +cir.func @while_cleanup_missing_kind() { + %s = cir.alloca "s" align(1) init : !cir.ptr<!rec_S> cir.while { + cir.call @ctor(%s) : (!cir.ptr<!rec_S>) -> () + %cond = cir.call @operatorBool(%s) : (!cir.ptr<!rec_S>) -> !cir.bool cir.condition(%cond) } do { cir.yield // expected-error @below {{expected valid keyword or string}} // expected-error @below {{failed to parse CIR_CleanupKindAttr}} } cleanup { - cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.call @dtor(%s) nothrow : (!cir.ptr<!rec_S>) -> () cir.yield } cir.return @@ -20,12 +27,19 @@ cir.func @while_cleanup_missing_kind(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { // ----- -!s32i = !cir.int<s, 32> +!u8i = !cir.int<u, 8> +!rec_S = !cir.struct<"S" padded {!u8i}> -cir.func private @dtor(!cir.ptr<!s32i>) +cir.func private @ctor(!cir.ptr<!rec_S>) +cir.func private @operatorBool(!cir.ptr<!rec_S>) -> !cir.bool +cir.func private @dtor(!cir.ptr<!rec_S>) func_info<#cir.cxx_dtor<!rec_S>> + attributes {nothrow} -cir.func @for_cleanup_missing_kind(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { +cir.func @for_cleanup_missing_kind() { + %s = cir.alloca "s" align(1) init : !cir.ptr<!rec_S> cir.for : cond { + cir.call @ctor(%s) : (!cir.ptr<!rec_S>) -> () + %cond = cir.call @operatorBool(%s) : (!cir.ptr<!rec_S>) -> !cir.bool cir.condition(%cond) } body { cir.yield @@ -34,7 +48,7 @@ cir.func @for_cleanup_missing_kind(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { // expected-error @below {{expected valid keyword or string}} // expected-error @below {{failed to parse CIR_CleanupKindAttr}} } cleanup { - cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.call @dtor(%s) nothrow : (!cir.ptr<!rec_S>) -> () cir.yield } cir.return @@ -42,18 +56,25 @@ cir.func @for_cleanup_missing_kind(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { // ----- -!s32i = !cir.int<s, 32> +!u8i = !cir.int<u, 8> +!rec_S = !cir.struct<"S" padded {!u8i}> -cir.func private @dtor(!cir.ptr<!s32i>) +cir.func private @ctor(!cir.ptr<!rec_S>) +cir.func private @operatorBool(!cir.ptr<!rec_S>) -> !cir.bool +cir.func private @dtor(!cir.ptr<!rec_S>) func_info<#cir.cxx_dtor<!rec_S>> + attributes {nothrow} -cir.func @while_cleanup_eh(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { +cir.func @while_cleanup_eh() { + %s = cir.alloca "s" align(1) init : !cir.ptr<!rec_S> // expected-error @below {{loop cleanup kind must be 'normal' or 'all', not 'eh'}} cir.while { + cir.call @ctor(%s) : (!cir.ptr<!rec_S>) -> () + %cond = cir.call @operatorBool(%s) : (!cir.ptr<!rec_S>) -> !cir.bool cir.condition(%cond) } do { cir.yield } cleanup eh { - cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.call @dtor(%s) nothrow : (!cir.ptr<!rec_S>) -> () cir.yield } cir.return @@ -61,20 +82,27 @@ cir.func @while_cleanup_eh(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { // ----- -!s32i = !cir.int<s, 32> +!u8i = !cir.int<u, 8> +!rec_S = !cir.struct<"S" padded {!u8i}> -cir.func private @dtor(!cir.ptr<!s32i>) +cir.func private @ctor(!cir.ptr<!rec_S>) +cir.func private @operatorBool(!cir.ptr<!rec_S>) -> !cir.bool +cir.func private @dtor(!cir.ptr<!rec_S>) func_info<#cir.cxx_dtor<!rec_S>> + attributes {nothrow} -cir.func @for_cleanup_eh(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { +cir.func @for_cleanup_eh() { + %s = cir.alloca "s" align(1) init : !cir.ptr<!rec_S> // expected-error @below {{loop cleanup kind must be 'normal' or 'all', not 'eh'}} cir.for : cond { + cir.call @ctor(%s) : (!cir.ptr<!rec_S>) -> () + %cond = cir.call @operatorBool(%s) : (!cir.ptr<!rec_S>) -> !cir.bool cir.condition(%cond) } body { cir.yield } step { cir.yield } cleanup eh { - cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.call @dtor(%s) nothrow : (!cir.ptr<!rec_S>) -> () cir.yield } cir.return diff --git a/clang/test/CIR/IR/loop-cleanup.cir b/clang/test/CIR/IR/loop-cleanup.cir index bdc896a7ba822..ee995f8ca3262 100644 --- a/clang/test/CIR/IR/loop-cleanup.cir +++ b/clang/test/CIR/IR/loop-cleanup.cir @@ -1,114 +1,99 @@ // RUN: cir-opt %s --verify-roundtrip | FileCheck %s -!s32i = !cir.int<s, 32> +!u8i = !cir.int<u, 8> +#true = #cir.bool<true> : !cir.bool +!rec_S = !cir.struct<"S" padded {!u8i}> module { - cir.func private @dtor(!cir.ptr<!s32i>) + cir.func private @ctor(!cir.ptr<!rec_S>) + cir.func private @operatorBool(!cir.ptr<!rec_S>) -> !cir.bool + cir.func private @dtor(!cir.ptr<!rec_S>) func_info<#cir.cxx_dtor<!rec_S>> + attributes {nothrow} - // A while loop with a per-iteration cleanup region that runs on both the - // normal and EH exit edges. - cir.func @while_cleanup_all(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.func @while_cleanup_all() { + %s = cir.alloca "s" align(1) init : !cir.ptr<!rec_S> cir.while { + cir.call @ctor(%s) : (!cir.ptr<!rec_S>) -> () + %cond = cir.call @operatorBool(%s) + : (!cir.ptr<!rec_S>) -> !cir.bool cir.condition(%cond) } do { cir.yield } cleanup all { - cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.call @dtor(%s) nothrow : (!cir.ptr<!rec_S>) -> () cir.yield } cir.return } // CHECK-LABEL: cir.func @while_cleanup_all + // CHECK: %[[S:.*]] = cir.alloca {{.*}}"s"{{.*}} : !cir.ptr<!rec_S> // CHECK: cir.while { - // CHECK: cir.condition(%[[COND:.*]]) + // CHECK: cir.call @ctor(%[[S]]) + // CHECK: %[[COND:.*]] = cir.call @operatorBool(%[[S]]) + // CHECK: cir.condition(%[[COND]]) // CHECK: } do { // CHECK: cir.yield // CHECK: } cleanup all { - // CHECK: cir.call @dtor + // CHECK: cir.call @dtor(%[[S]]) nothrow // CHECK: cir.yield // CHECK: } - // A while loop with a normal-only cleanup region (exceptions disabled). - cir.func @while_cleanup_normal(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.func @while_cleanup_normal() { + %s = cir.alloca "s" align(1) init : !cir.ptr<!rec_S> cir.while { + cir.call @ctor(%s) : (!cir.ptr<!rec_S>) -> () + %cond = cir.call @operatorBool(%s) + : (!cir.ptr<!rec_S>) -> !cir.bool cir.condition(%cond) } do { cir.yield } cleanup normal { - cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.call @dtor(%s) nothrow : (!cir.ptr<!rec_S>) -> () cir.yield } cir.return } // CHECK-LABEL: cir.func @while_cleanup_normal + // CHECK: cir.while { + // CHECK: cir.call @ctor(%[[S:[0-9]+]]) + // CHECK: cir.call @operatorBool(%[[S]]) + // CHECK: cir.condition // CHECK: } cleanup normal { // CHECK: cir.call @dtor // CHECK: } - // A while loop without a cleanup region still prints as before. - cir.func @while_no_cleanup(%cond : !cir.bool) { - cir.while { - cir.condition(%cond) - } do { - cir.yield - } - cir.return - } - - // CHECK-LABEL: cir.func @while_no_cleanup - // CHECK: cir.while { - // CHECK: cir.condition - // CHECK: } do { - // CHECK: cir.yield - // CHECK-NOT: cleanup - - // A for loop with a per-iteration cleanup region after the step region. - cir.func @for_cleanup_all(%cond : !cir.bool, %s : !cir.ptr<!s32i>) { + cir.func @for_cleanup_all() { + %s = cir.alloca "s" align(1) init : !cir.ptr<!rec_S> cir.for : cond { + cir.call @ctor(%s) : (!cir.ptr<!rec_S>) -> () + %cond = cir.call @operatorBool(%s) + : (!cir.ptr<!rec_S>) -> !cir.bool cir.condition(%cond) } body { cir.yield } step { cir.yield } cleanup all { - cir.call @dtor(%s) : (!cir.ptr<!s32i>) -> () + cir.call @dtor(%s) nothrow : (!cir.ptr<!rec_S>) -> () cir.yield } cir.return } // CHECK-LABEL: cir.func @for_cleanup_all + // CHECK: %[[S:.*]] = cir.alloca {{.*}}"s"{{.*}} : !cir.ptr<!rec_S> // CHECK: cir.for : cond { - // CHECK: cir.condition(%[[FCOND:.*]]) + // CHECK: cir.call @ctor(%[[S]]) + // CHECK: cir.call @operatorBool(%[[S]]) + // CHECK: cir.condition // CHECK: } body { // CHECK: cir.yield // CHECK: } step { // CHECK: cir.yield // CHECK: } cleanup all { - // CHECK: cir.call @dtor + // CHECK: cir.call @dtor(%[[S]]) nothrow // CHECK: cir.yield // CHECK: } - - // A for loop without a cleanup region still prints as before. - cir.func @for_no_cleanup(%cond : !cir.bool) { - cir.for : cond { - cir.condition(%cond) - } body { - cir.yield - } step { - cir.yield - } - cir.return - } - - // CHECK-LABEL: cir.func @for_no_cleanup - // CHECK: cir.for : cond { - // CHECK: cir.condition - // CHECK: } body { - // CHECK: cir.yield - // CHECK: } step { - // CHECK: cir.yield - // CHECK-NOT: cleanup } >From 5db0f8aa1992a927c9d45bfdafb4dd391c059f4f Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Fri, 17 Jul 2026 14:03:33 -0700 Subject: [PATCH 4/4] Document that DoWhileOp cannot have a cleanup region --- clang/include/clang/CIR/Dialect/IR/CIROps.td | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 976b173db8f94..296b2d54e30eb 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -2217,7 +2217,8 @@ def CIR_WhileOp : CIR_WhileOpBase<"while"> { (condition-false exit, end-of-iteration, break/continue, and EH unwinding when the cleanup kind includes EH). This is used to destroy a condition variable whose lifetime is a single iteration. When present, it carries a - cleanup kind matching `cir.cleanup.scope` (`normal` or `all`). + cleanup kind matching `cir.cleanup.scope` (`normal` or `all`). Note that + a `DoWhileOp` cannot contain a `cleanup` region. Example: @@ -2279,7 +2280,10 @@ def CIR_DoWhileOp : CIR_WhileOpBase<"do"> { let description = [{ Represents a C/C++ do-while loop. Identical to `cir.while` but the - condition is evaluated after the body. + condition is evaluated after the body. Because a variable cannot be + declared in the condition of a do-while loop, this operation cannot + have a `cleanup` region. A cleanup scope should be created within the + body region for any variables within the loop that require cleanup. Example: _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
