Author: Andy Kaylor Date: 2026-03-31T10:54:56-07:00 New Revision: 49dbeca38d8faeef42d7cb03db649fbfb4c195c4
URL: https://github.com/llvm/llvm-project/commit/49dbeca38d8faeef42d7cb03db649fbfb4c195c4 DIFF: https://github.com/llvm/llvm-project/commit/49dbeca38d8faeef42d7cb03db649fbfb4c195c4.diff LOG: [CIR] Fix missing terminators in regions with cleanup (#187604) When a cleanup scope was created within an if operation's then or else region and the source scope ended with a return, we would fail to terminate the region following the cleanup scope, which surrounded the return operation, resulting in an MLIR verification error. This change forces the addition of terminators in the if's then and else regions. Added: clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp Modified: clang/lib/CIR/CodeGen/CIRGenExpr.cpp clang/lib/CIR/CodeGen/CIRGenFunction.h clang/lib/CIR/CodeGen/CIRGenStmt.cpp Removed: ################################################################################ diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp index 7f3e476b006c1..65644bc1a3fd9 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp @@ -2278,9 +2278,15 @@ cir::IfOp CIRGenFunction::emitIfOnBoolExpr( // Emit the code with the fully general case. mlir::Value condV = emitOpOnBoolExpr(loc, cond); - return cir::IfOp::create(builder, loc, condV, elseLoc.has_value(), - /*thenBuilder=*/thenBuilder, - /*elseBuilder=*/elseBuilder); + cir::IfOp ifOp = cir::IfOp::create(builder, loc, condV, elseLoc.has_value(), + /*thenBuilder=*/thenBuilder, + /*elseBuilder=*/elseBuilder); + terminateStructuredRegionBody(ifOp.getThenRegion(), thenLoc); + assert((elseLoc.has_value() || ifOp.getElseRegion().empty()) && + "else region created with no else location"); + if (elseLoc.has_value()) + terminateStructuredRegionBody(ifOp.getElseRegion(), *elseLoc); + return ifOp; } /// TODO(cir): see EmitBranchOnBoolExpr for extra ideas). diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index d80c2d635a89f..0bd440a61db20 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -972,6 +972,8 @@ class CIRGenFunction : public CIRGenTypeCache { ArrayRef<mlir::Value *> valuesToReload = {}); void popCleanupBlock(); + void terminateStructuredRegionBody(mlir::Region &r, mlir::Location loc); + /// Deactivates the given cleanup block. The block cannot be reactivated. Pops /// it if it's the top of the stack. /// diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp index a33a0b8bdd50c..07d1d62053ea6 100644 --- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp @@ -488,8 +488,8 @@ mlir::LogicalResult CIRGenFunction::emitLabelStmt(const clang::LabelStmt &s) { } // Add a terminating yield on a body region if no other terminators are used. -static void terminateBody(CIRGenBuilderTy &builder, mlir::Region &r, - mlir::Location loc) { +void CIRGenFunction::terminateStructuredRegionBody(mlir::Region &r, + mlir::Location loc) { if (r.empty()) return; @@ -979,7 +979,7 @@ CIRGenFunction::emitCXXForRangeStmt(const CXXForRangeStmt &s, if (res.failed()) return res; - terminateBody(builder, forOp.getBody(), getLoc(s.getEndLoc())); + terminateStructuredRegionBody(forOp.getBody(), getLoc(s.getEndLoc())); return mlir::success(); } @@ -1051,7 +1051,7 @@ mlir::LogicalResult CIRGenFunction::emitForStmt(const ForStmt &s) { if (res.failed()) return res; - terminateBody(builder, forOp.getBody(), getLoc(s.getEndLoc())); + terminateStructuredRegionBody(forOp.getBody(), getLoc(s.getEndLoc())); return mlir::success(); } @@ -1102,7 +1102,7 @@ mlir::LogicalResult CIRGenFunction::emitDoStmt(const DoStmt &s) { if (res.failed()) return res; - terminateBody(builder, doWhileOp.getBody(), getLoc(s.getEndLoc())); + terminateStructuredRegionBody(doWhileOp.getBody(), getLoc(s.getEndLoc())); return mlir::success(); } @@ -1158,7 +1158,7 @@ mlir::LogicalResult CIRGenFunction::emitWhileStmt(const WhileStmt &s) { if (res.failed()) return res; - terminateBody(builder, whileOp.getBody(), getLoc(s.getEndLoc())); + terminateStructuredRegionBody(whileOp.getBody(), getLoc(s.getEndLoc())); return mlir::success(); } @@ -1253,8 +1253,8 @@ mlir::LogicalResult CIRGenFunction::emitSwitchStmt(const clang::SwitchStmt &s) { llvm::SmallVector<CaseOp> cases; swop.collectCases(cases); for (auto caseOp : cases) - terminateBody(builder, caseOp.getCaseRegion(), caseOp.getLoc()); - terminateBody(builder, swop.getBody(), swop.getLoc()); + terminateStructuredRegionBody(caseOp.getCaseRegion(), caseOp.getLoc()); + terminateStructuredRegionBody(swop.getBody(), swop.getLoc()); swop.setAllEnumCasesCovered(s.isAllEnumCasesCovered()); diff --git a/clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp b/clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp new file mode 100644 index 0000000000000..c6a36797004c6 --- /dev/null +++ b/clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp @@ -0,0 +1,23 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --input-file=%t.cir %s + +// Out-of-line member + `if` + EH cleanups + early `return` used to emit a +// `cir.if` "then" region whose block ended on `cir.cleanup.scope` with no +// following `cir.yield`, tripping MLIR's "block with no terminator" verifier. + +struct S { + ~S(); +}; + +int f(bool b) { + if (b) { + S temp; + return 1; + } + return 0; +} + +// CHECK-LABEL: cir.func{{.*}} @_Z1fb +// CHECK: cir.if +// CHECK: cir.cleanup.scope +// CHECK: cir.yield _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
