Author: Andy Kaylor Date: 2026-05-29T14:56:26-07:00 New Revision: 50ed21d39c0d0ca6d8ae4f7beed5f70d7e7f7343
URL: https://github.com/llvm/llvm-project/commit/50ed21d39c0d0ca6d8ae4f7beed5f70d7e7f7343 DIFF: https://github.com/llvm/llvm-project/commit/50ed21d39c0d0ca6d8ae4f7beed5f70d7e7f7343.diff LOG: [CIR] Add RunCleanupsScope RAII around loop bodies (#200461) This fixes yet another problem where a ternary operator in a loop body was leading to an unterminated region. We have long had a comment suggesting that we should consider loop-specific cleanup handling to mimic the cleanup exit block that classic codegen creates. I previously believed that wasn't really necessary because CIR's structured representation handles branching through cleanups during later lowering. That's true, but not having something to trigger the cleanup stack handling when we exit the loop's body region was causing us to miss emitting a yield after the loop operation. This change introduces the RAII object for cleanups. This also allows me to remove some handling in LexicalScope::cleanup that was basically there to smooth over terminator insertion problems. Assisted-by: Cursor / claude-opus-4.7 Added: Modified: clang/include/clang/CIR/MissingFeatures.h clang/lib/CIR/CodeGen/CIRGenFunction.cpp clang/lib/CIR/CodeGen/CIRGenStmt.cpp clang/test/CIR/CodeGen/loop-cond-cleanup.cpp Removed: ################################################################################ diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index 3cca63a83d10f..cb9ce23e74230 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -279,7 +279,6 @@ struct MissingFeatures { static bool pgoUse() { return false; } static bool pointerAuthentication() { return false; } static bool preservedAccessIndexRegion() { return false; } - static bool loopSpecificCleanupHandling() { return false; } static bool returnValueSlotFeatures() { return false; } static bool runCleanupsScope() { return false; } static bool sanitizers() { return false; } diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index 247c4ad131197..4ecb47a864146 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp @@ -275,22 +275,6 @@ void CIRGenFunction::LexicalScope::cleanup() { if (curBlock->mightHaveTerminator() && curBlock->getTerminator()) return; - // If the builder's current block lives in a region nested below this - // lexical scope's region, popCleanup has left the insertion point past - // a cir.cleanup.scope inside that nested region (for example, inside a - // cir.for body when the body cleanup was registered there). Emitting a - // cir.yield at the current position would terminate the nested region - // instead of this scope's region. Reposition to the back block of this - // scope's region so the terminator below lands in the correct place. - if (mlir::Region *scopeRegion = entryBlock->getParent(); - scopeRegion && !scopeRegion->empty() && - curBlock->getParent() != scopeRegion) { - builder.setInsertionPointToEnd(&scopeRegion->back()); - curBlock = builder.getBlock(); - if (curBlock->mightHaveTerminator() && curBlock->getTerminator()) - return; - } - // Get rid of any empty block at the end of the scope. An empty non-entry // block is created when a terminator (return/break/continue) is followed // by unreachable code. diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp index 4a4380dea70b9..6380758cb5bcb 100644 --- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp @@ -914,11 +914,6 @@ CIRGenFunction::emitCXXForRangeStmt(const CXXForRangeStmt &s, return mlir::failure(); assert(!cir::MissingFeatures::loopInfoStack()); - // From LLVM: if there are any cleanups between here and the loop-exit - // scope, create a block to stage a loop exit along. - // We probably already do the right thing because of ScopeOp, but make - // sure we handle all cases. - assert(!cir::MissingFeatures::loopSpecificCleanupHandling()); forOp = builder.createFor( getLoc(s.getSourceRange()), @@ -934,6 +929,7 @@ CIRGenFunction::emitCXXForRangeStmt(const CXXForRangeStmt &s, // https://en.cppreference.com/w/cpp/language/for // In C++ the scope of the init-statement and the scope of // statement are one and the same. + RunCleanupsScope bodyScope(*this); bool useCurrentScope = true; if (emitStmt(s.getLoopVarStmt(), useCurrentScope).failed()) loopRes = mlir::failure(); @@ -982,11 +978,6 @@ mlir::LogicalResult CIRGenFunction::emitForStmt(const ForStmt &s) { if (emitStmt(s.getInit(), /*useCurrentScope=*/true).failed()) return mlir::failure(); assert(!cir::MissingFeatures::loopInfoStack()); - // In the classic codegen, if there are any cleanups between here and the - // loop-exit scope, a block is created to stage the loop exit. We probably - // already do the right thing because of ScopeOp, but we need more testing - // to be sure we handle all cases. - assert(!cir::MissingFeatures::loopSpecificCleanupHandling()); forOp = builder.createFor( getLoc(s.getSourceRange()), @@ -1013,6 +1004,7 @@ mlir::LogicalResult CIRGenFunction::emitForStmt(const ForStmt &s) { [&](mlir::OpBuilder &b, mlir::Location loc) { // The scope of the for loop body is nested within the scope of the // for loop's init-statement and condition. + RunCleanupsScope bodyScope(*this); if (emitStmt(s.getBody(), /*useCurrentScope=*/false).failed()) loopRes = mlir::failure(); emitStopPoint(&s); @@ -1050,11 +1042,6 @@ mlir::LogicalResult CIRGenFunction::emitDoStmt(const DoStmt &s) { auto doStmtBuilder = [&]() -> mlir::LogicalResult { mlir::LogicalResult loopRes = mlir::success(); assert(!cir::MissingFeatures::loopInfoStack()); - // From LLVM: if there are any cleanups between here and the loop-exit - // scope, create a block to stage a loop exit along. - // We probably already do the right thing because of ScopeOp, but make - // sure we handle all cases. - assert(!cir::MissingFeatures::loopSpecificCleanupHandling()); doWhileOp = builder.createDoWhile( getLoc(s.getSourceRange()), @@ -1071,6 +1058,7 @@ mlir::LogicalResult CIRGenFunction::emitDoStmt(const DoStmt &s) { /*bodyBuilder=*/ [&](mlir::OpBuilder &b, mlir::Location loc) { // The scope of the do-while loop body is a nested scope. + RunCleanupsScope bodyScope(*this); if (emitStmt(s.getBody(), /*useCurrentScope=*/false).failed()) loopRes = mlir::failure(); emitStopPoint(&s); @@ -1101,11 +1089,6 @@ mlir::LogicalResult CIRGenFunction::emitWhileStmt(const WhileStmt &s) { auto whileStmtBuilder = [&]() -> mlir::LogicalResult { mlir::LogicalResult loopRes = mlir::success(); assert(!cir::MissingFeatures::loopInfoStack()); - // From LLVM: if there are any cleanups between here and the loop-exit - // scope, create a block to stage a loop exit along. - // We probably already do the right thing because of ScopeOp, but make - // sure we handle all cases. - assert(!cir::MissingFeatures::loopSpecificCleanupHandling()); whileOp = builder.createWhile( getLoc(s.getSourceRange()), @@ -1127,6 +1110,7 @@ mlir::LogicalResult CIRGenFunction::emitWhileStmt(const WhileStmt &s) { /*bodyBuilder=*/ [&](mlir::OpBuilder &b, mlir::Location loc) { // The scope of the while loop body is a nested scope. + RunCleanupsScope bodyScope(*this); if (emitStmt(s.getBody(), /*useCurrentScope=*/false).failed()) loopRes = mlir::failure(); emitStopPoint(&s); diff --git a/clang/test/CIR/CodeGen/loop-cond-cleanup.cpp b/clang/test/CIR/CodeGen/loop-cond-cleanup.cpp index 933f1926c9d8c..5dad07717b808 100644 --- a/clang/test/CIR/CodeGen/loop-cond-cleanup.cpp +++ b/clang/test/CIR/CodeGen/loop-cond-cleanup.cpp @@ -241,3 +241,205 @@ void range_for_cond_cleanup() { // OGCG: call void @_ZN1SD1Ev( // OGCG: [[END]]: // OGCG: ret void + +void while_body_temp_ref() { + while (0) + const S &op = 1 ? S() : S(); +} + +// CIR-LABEL: cir.func {{.*}} @_Z19while_body_temp_refv +// CIR: cir.scope { +// CIR: cir.while { +// CIR: } do { +// CIR: cir.if +// CIR: cir.cleanup.scope { +// CIR: } cleanup eh { +// CIR: cir.call @_ZN1SD1Ev({{.*}}) nothrow +// CIR: } +// CIR: cir.cleanup.scope { +// CIR: } cleanup all { +// CIR: cir.call @_ZN1SD1Ev({{.*}}) nothrow +// CIR: } +// CIR: } +// CIR: } +// CIR: cir.return + +// LLVM-LABEL: define dso_local void @_Z19while_body_temp_refv() +// LLVM: %[[REF_TMP:.*]] = alloca %struct.S +// LLVM: %[[OP:.*]] = alloca ptr +// LLVM: %[[SPILL:.*]] = alloca ptr +// LLVM: br label %{{.*}} +// LLVM: br label %[[LOOP_COND:.*]] +// LLVM: [[LOOP_COND]]: +// LLVM: br i1 false, label %[[BODY:.*]], label %[[AFTER:.*]] +// LLVM: [[BODY]]: +// LLVM: br i1 true, label %[[TRUE:.*]], label %[[FALSE:.*]] +// LLVM: [[TRUE]]: +// LLVM: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: [[FALSE]]: +// LLVM: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: store ptr %[[REF_TMP]], ptr %[[SPILL]] +// LLVM: call void @_ZN1SD1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: %[[RELOAD:.*]] = load ptr, ptr %[[SPILL]] +// LLVM: store ptr %[[RELOAD]], ptr %[[OP]] +// LLVM: br label %[[LOOP_COND]] +// LLVM: [[AFTER]]: +// LLVM: ret void + +// OGCG-LABEL: define dso_local void @_Z19while_body_temp_refv() +// OGCG: %[[OP:.*]] = alloca ptr +// OGCG: %[[REF_TMP:.*]] = alloca %struct.S +// OGCG: br label %[[WHILE_COND:.*]] +// OGCG: [[WHILE_COND]]: +// OGCG: br i1 false, label %[[WHILE_BODY:.*]], label %[[WHILE_END:.*]] +// OGCG: [[WHILE_BODY]]: +// OGCG: br i1 true, label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]] +// OGCG: [[COND_TRUE]]: +// OGCG: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: [[COND_FALSE]]: +// OGCG: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: store ptr %[[REF_TMP]], ptr %[[OP]] +// OGCG: call void @_ZN1SD1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: br label %[[WHILE_COND]] +// OGCG: [[WHILE_END]]: +// OGCG: ret void + +void for_body_temp_ref() { + for (int i = 0; i < 0; ++i) + const S &op = 1 ? S() : S(); +} + +// CIR-LABEL: cir.func {{.*}} @_Z17for_body_temp_refv +// CIR: cir.scope { +// CIR: cir.for : cond { +// CIR: } body { +// CIR: cir.if +// CIR: cir.cleanup.scope { +// CIR: } cleanup eh { +// CIR: cir.call @_ZN1SD1Ev({{.*}}) nothrow +// CIR: } +// CIR: cir.cleanup.scope { +// CIR: } cleanup all { +// CIR: cir.call @_ZN1SD1Ev({{.*}}) nothrow +// CIR: } +// CIR: } step { +// CIR: } +// CIR: } +// CIR: cir.return + +// LLVM-LABEL: define dso_local void @_Z17for_body_temp_refv() +// LLVM: %[[I:.*]] = alloca i32 +// LLVM: %[[REF_TMP:.*]] = alloca %struct.S +// LLVM: %[[OP:.*]] = alloca ptr +// LLVM: %[[SPILL:.*]] = alloca ptr +// LLVM: br label %{{.*}} +// LLVM: store i32 0, ptr %[[I]] +// LLVM: br label %[[LOOP_COND:.*]] +// LLVM: [[LOOP_COND]]: +// LLVM: %[[IVAL:.*]] = load i32, ptr %[[I]] +// LLVM: %[[CMP:.*]] = icmp slt i32 %[[IVAL]], 0 +// LLVM: br i1 %[[CMP]], label %[[BODY:.*]], label %[[AFTER:.*]] +// LLVM: [[BODY]]: +// LLVM: br i1 true, label %[[TRUE:.*]], label %[[FALSE:.*]] +// LLVM: [[TRUE]]: +// LLVM: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: [[FALSE]]: +// LLVM: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: store ptr %[[REF_TMP]], ptr %[[SPILL]] +// LLVM: call void @_ZN1SD1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: %[[RELOAD:.*]] = load ptr, ptr %[[SPILL]] +// LLVM: store ptr %[[RELOAD]], ptr %[[OP]] +// LLVM: %[[OLDI:.*]] = load i32, ptr %[[I]] +// LLVM: %[[NEWI:.*]] = add nsw i32 %[[OLDI]], 1 +// LLVM: store i32 %[[NEWI]], ptr %[[I]] +// LLVM: br label %[[LOOP_COND]] +// LLVM: [[AFTER]]: +// LLVM: ret void + +// OGCG-LABEL: define dso_local void @_Z17for_body_temp_refv() +// OGCG: %[[I:.*]] = alloca i32 +// OGCG: %[[OP:.*]] = alloca ptr +// OGCG: %[[REF_TMP:.*]] = alloca %struct.S +// OGCG: store i32 0, ptr %[[I]] +// OGCG: br label %[[FOR_COND:.*]] +// OGCG: [[FOR_COND]]: +// OGCG: %[[IVAL:.*]] = load i32, ptr %[[I]] +// OGCG: %[[CMP:.*]] = icmp slt i32 %[[IVAL]], 0 +// OGCG: br i1 %[[CMP]], label %[[FOR_BODY:.*]], label %[[FOR_END:.*]] +// OGCG: [[FOR_BODY]]: +// OGCG: br i1 true, label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]] +// OGCG: [[COND_TRUE]]: +// OGCG: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: [[COND_FALSE]]: +// OGCG: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: store ptr %[[REF_TMP]], ptr %[[OP]] +// OGCG: call void @_ZN1SD1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: br label %[[FOR_INC:.*]] +// OGCG: [[FOR_INC]]: +// OGCG: %[[OLDI:.*]] = load i32, ptr %[[I]] +// OGCG: %[[NEWI:.*]] = add nsw i32 %[[OLDI]], 1 +// OGCG: store i32 %[[NEWI]], ptr %[[I]] +// OGCG: br label %[[FOR_COND]] +// OGCG: [[FOR_END]]: +// OGCG: ret void + +void do_body_temp_ref() { + do + const S &op = 1 ? S() : S(); + while (0); +} + +// CIR-LABEL: cir.func {{.*}} @_Z16do_body_temp_refv +// CIR: cir.scope { +// CIR: cir.do { +// CIR: cir.if +// CIR: cir.cleanup.scope { +// CIR: } cleanup eh { +// CIR: cir.call @_ZN1SD1Ev({{.*}}) nothrow +// CIR: } +// CIR: cir.cleanup.scope { +// CIR: } cleanup all { +// CIR: cir.call @_ZN1SD1Ev({{.*}}) nothrow +// CIR: } +// CIR: } while { +// CIR: } +// CIR: } +// CIR: cir.return + +// LLVM-LABEL: define dso_local void @_Z16do_body_temp_refv() +// LLVM: %[[REF_TMP:.*]] = alloca %struct.S +// LLVM: %[[OP:.*]] = alloca ptr +// LLVM: %[[SPILL:.*]] = alloca ptr +// LLVM: br label %{{.*}} +// LLVM: br label %[[BODY:.*]] +// LLVM: [[LOOP_COND:.*]]: +// LLVM: br i1 false, label %[[BODY]], label %[[AFTER:.*]] +// LLVM: [[BODY]]: +// LLVM: br i1 true, label %[[TRUE:.*]], label %[[FALSE:.*]] +// LLVM: [[TRUE]]: +// LLVM: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: [[FALSE]]: +// LLVM: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: store ptr %[[REF_TMP]], ptr %[[SPILL]] +// LLVM: call void @_ZN1SD1Ev(ptr {{.*}} %[[REF_TMP]]) +// LLVM: %[[RELOAD:.*]] = load ptr, ptr %[[SPILL]] +// LLVM: store ptr %[[RELOAD]], ptr %[[OP]] +// LLVM: br label %[[LOOP_COND]] +// LLVM: [[AFTER]]: +// LLVM: ret void + +// OGCG-LABEL: define dso_local void @_Z16do_body_temp_refv() +// OGCG: %[[OP:.*]] = alloca ptr +// OGCG: %[[REF_TMP:.*]] = alloca %struct.S +// OGCG: br label %[[DO_BODY:.*]] +// OGCG: [[DO_BODY]]: +// OGCG: br i1 true, label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]] +// OGCG: [[COND_TRUE]]: +// OGCG: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: [[COND_FALSE]]: +// OGCG: call void @_ZN1SC1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: store ptr %[[REF_TMP]], ptr %[[OP]] +// OGCG: call void @_ZN1SD1Ev(ptr {{.*}} %[[REF_TMP]]) +// OGCG: br label %[[DO_END:.*]] +// OGCG: [[DO_END]]: +// OGCG: ret void _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
