https://github.com/AmrDeveloper updated https://github.com/llvm/llvm-project/pull/183143
>From bae070fcb470e2af50033342d9b1044635c0d45f Mon Sep 17 00:00:00 2001 From: Amr Hesham <[email protected]> Date: Mon, 23 Feb 2026 21:54:00 +0100 Subject: [PATCH 1/2] [CIR] Fix codegen TernaryOp inside CleanupScopeOp --- clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp | 47 +++++++++-- clang/test/CIR/CodeGen/dtors.cpp | 93 ++++++++++++++-------- 2 files changed, 100 insertions(+), 40 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp index 567c60844af1e..1d8f1c0443e43 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp @@ -1306,19 +1306,35 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { CIRGenFunction::ConditionalEvaluation eval(cgf); mlir::Value lhsCondV = cgf.evaluateExprAsBool(e->getLHS()); + bool hasCleanupScopeParent = + mlir::isa<cir::CleanupScopeOp>(builder.getBlock()->getParentOp()); + auto resOp = cir::TernaryOp::create( builder, loc, lhsCondV, /*trueBuilder=*/ [&](mlir::OpBuilder &b, mlir::Location loc) { CIRGenFunction::LexicalScope lexScope{cgf, loc, b.getInsertionBlock()}; cgf.curLexScope->setAsTernary(); + + mlir::Value cleanupScopeTmpAlloca; + if (hasCleanupScopeParent) { + cleanupScopeTmpAlloca = builder.createAlloca( + loc, builder.getPointerTo(builder.getBoolTy()), + builder.getBoolTy(), "cleanup.scope.tmp", + clang::CharUnits::One()); + } + mlir::Value res = cgf.evaluateExprAsBool(e->getRHS()); + + if (hasCleanupScopeParent) + cir::StoreOp::create(builder, loc, res, cleanupScopeTmpAlloca, + false, {}, {}, {}); + lexScope.forceCleanup(); + + if (hasCleanupScopeParent) + res = cir::LoadOp::create(builder, loc, cleanupScopeTmpAlloca); cir::YieldOp::create(b, loc, res); - if (res.getParentBlock() != builder.getInsertionBlock()) - cgf.cgm.errorNYI( - e->getSourceRange(), - "ScalarExprEmitter: VisitBinLAnd ternary with cleanup"); }, /*falseBuilder*/ [&](mlir::OpBuilder &b, mlir::Location loc) { @@ -1355,6 +1371,9 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { CIRGenFunction::ConditionalEvaluation eval(cgf); mlir::Value lhsCondV = cgf.evaluateExprAsBool(e->getLHS()); + bool hasCleanupScopeParent = + mlir::isa<cir::CleanupScopeOp>(builder.getBlock()->getParentOp()); + auto resOp = cir::TernaryOp::create( builder, loc, lhsCondV, /*trueBuilder=*/ [&](mlir::OpBuilder &b, mlir::Location loc) { @@ -1369,13 +1388,25 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { CIRGenFunction::LexicalScope lexScope{cgf, loc, b.getInsertionBlock()}; cgf.curLexScope->setAsTernary(); + mlir::Value cleanupScopeTmpAlloca; + if (hasCleanupScopeParent) { + cleanupScopeTmpAlloca = builder.createAlloca( + loc, builder.getPointerTo(builder.getBoolTy()), + builder.getBoolTy(), "cleanup.scope.tmp", + clang::CharUnits::One()); + } + mlir::Value res = cgf.evaluateExprAsBool(e->getRHS()); + + if (hasCleanupScopeParent) + cir::StoreOp::create(builder, loc, res, cleanupScopeTmpAlloca, + false, {}, {}, {}); + lexScope.forceCleanup(); + + if (hasCleanupScopeParent) + res = cir::LoadOp::create(builder, loc, cleanupScopeTmpAlloca); cir::YieldOp::create(b, loc, res); - if (res.getParentBlock() != builder.getInsertionBlock()) - cgf.cgm.errorNYI( - e->getSourceRange(), - "ScalarExprEmitter: VisitBinLOr ternary with cleanup"); }); return maybePromoteBoolResult(resOp.getResult(), resTy); diff --git a/clang/test/CIR/CodeGen/dtors.cpp b/clang/test/CIR/CodeGen/dtors.cpp index 5da94a1c61548..02e329bef768b 100644 --- a/clang/test/CIR/CodeGen/dtors.cpp +++ b/clang/test/CIR/CodeGen/dtors.cpp @@ -1,11 +1,9 @@ // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -fclangir -emit-cir %s -o %t.cir // RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -fclangir -emit-llvm %s -o %t-cir.ll -// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefix=LLVM // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -emit-llvm %s -o %t.ll // RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG -// XFAIL: * +// TODO(cir): Reenable lowering to LLVM after nested EH cleanup scope flattening is not yet implemented. struct A { ~A(); @@ -17,7 +15,12 @@ void test_temporary_dtor() { // CIR: cir.func {{.*}} @_Z19test_temporary_dtorv() // CIR: %[[ALLOCA:.*]] = cir.alloca !rec_A, !cir.ptr<!rec_A>, ["agg.tmp.ensured"] -// CIR: cir.call @_ZN1AD1Ev(%[[ALLOCA]]) nothrow : (!cir.ptr<!rec_A> {{.*}}) -> () +// CIR: cir.cleanup.scope { +// CIR: cir.yield +// CIR: } cleanup normal { +// CIR: cir.call @_ZN1AD1Ev(%[[ALLOCA]]) nothrow : (!cir.ptr<!rec_A> {{.*}}) -> () +// CIR: cir.yield +// CIR: } // LLVM: define dso_local void @_Z19test_temporary_dtorv(){{.*}} // LLVM: %[[ALLOCA:.*]] = alloca %struct.A, i64 1, align 1 @@ -41,20 +44,33 @@ bool test_temp_or() { return make_temp(1) || make_temp(2); } // CIR: %[[REF_TMP0:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp0"] // CIR: %[[ONE:.*]] = cir.const #cir.int<1> // CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP0]], %[[ONE]]) -// CIR: %[[MAKE_TEMP0:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP0]]) -// CIR: %[[TERNARY:.*]] = cir.ternary(%[[MAKE_TEMP0]], true { -// CIR: %[[TRUE:.*]] = cir.const #true -// CIR: cir.yield %[[TRUE]] : !cir.bool -// CIR: }, false { -// CIR: %[[REF_TMP1:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp1"] -// CIR: %[[TWO:.*]] = cir.const #cir.int<2> -// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP1]], %[[TWO]]) -// CIR: %[[MAKE_TEMP1:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP1]]) -// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP1]]) -// CIR: cir.yield %[[MAKE_TEMP1]] : !cir.bool -// CIR: }) -// CIR: cir.store{{.*}} %[[TERNARY]], %[[RETVAL:.*]] -// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP0]]) +// CIR: cir.cleanup.scope { +// CIR: %[[MAKE_TEMP0:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP0]]) +// CIR: %[[TERNARY:.*]] = cir.ternary(%[[MAKE_TEMP0]], true { +// CIR: %[[TRUE:.*]] = cir.const #true +// CIR: cir.yield %[[TRUE]] : !cir.bool +// CIR: }, false { +// CIR: %[[CLEANUP_SCOPE_TMP:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["cleanup.scope.tmp"] +// CIR: %[[REF_TMP1:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp1"] +// CIR: %[[TWO:.*]] = cir.const #cir.int<2> +// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP1]], %[[TWO]]) +// CIR: cir.cleanup.scope { +// CIR: %[[MAKE_TEMP1:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP1]]) +// CIR: cir.store %[[MAKE_TEMP1]], %[[CLEANUP_SCOPE_TMP]] : !cir.bool, !cir.ptr<!cir.bool> loc(#loc33) +// CIR: cir.yield +// CIR: } cleanup normal { +// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP1]]) +// CIR: cir.yield +// CIR: } +// CIR: %[[RESULT:.*]] = cir.load %[[CLEANUP_SCOPE_TMP]] : !cir.ptr<!cir.bool>, !cir.bool +// CIR: cir.yield %[[RESULT]] : !cir.bool +// CIR: }) +// CIR: cir.store{{.*}} %[[TERNARY]], %[[RETVAL:.*]] +// CIR: cir.yield +// CIR: } cleanup normal { +// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP0]]) +// CIR: cir.yield +// CIR: } // CIR: } // LLVM: define{{.*}} i1 @_Z12test_temp_orv(){{.*}} { @@ -111,20 +127,33 @@ bool test_temp_and() { return make_temp(1) && make_temp(2); } // CIR: %[[REF_TMP0:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp0"] // CIR: %[[ONE:.*]] = cir.const #cir.int<1> // CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP0]], %[[ONE]]) -// CIR: %[[MAKE_TEMP0:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP0]]) -// CIR: %[[TERNARY:.*]] = cir.ternary(%[[MAKE_TEMP0]], true { -// CIR: %[[REF_TMP1:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp1"] -// CIR: %[[TWO:.*]] = cir.const #cir.int<2> -// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP1]], %[[TWO]]) -// CIR: %[[MAKE_TEMP1:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP1]]) -// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP1]]) -// CIR: cir.yield %[[MAKE_TEMP1]] : !cir.bool -// CIR: }, false { -// CIR: %[[FALSE:.*]] = cir.const #false -// CIR: cir.yield %[[FALSE]] : !cir.bool -// CIR: }) -// CIR: cir.store{{.*}} %[[TERNARY]], %[[RETVAL:.*]] -// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP0]]) +// CIR: cir.cleanup.scope { +// CIR: %[[MAKE_TEMP0:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP0]]) +// CIR: %[[TERNARY:.*]] = cir.ternary(%[[MAKE_TEMP0]], true { +// CIR: %[[CLEANUP_SCOPE_TMP:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["cleanup.scope.tmp"] +// CIR: %[[REF_TMP1:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp1"] +// CIR: %[[TWO:.*]] = cir.const #cir.int<2> +// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP1]], %[[TWO]]) +// CIR: cir.cleanup.scope { +// CIR: %[[MAKE_TEMP1:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP1]]) +// CIR: cir.store %[[MAKE_TEMP1]], %[[CLEANUP_SCOPE_TMP]] : !cir.bool, !cir.ptr<!cir.bool> +// CIR: cir.yield +// CIR: } cleanup normal { +// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP1]]) +// CIR: cir.yield +// CIR: } +// CIR: %[[RESULT:.*]] = cir.load %[[CLEANUP_SCOPE_TMP]] : !cir.ptr<!cir.bool>, !cir.bool +// CIR: cir.yield %[[RESULT]] : !cir.bool +// CIR: }, false { +// CIR: %[[FALSE:.*]] = cir.const #false +// CIR: cir.yield %[[FALSE]] : !cir.bool +// CIR: }) +// CIR: cir.store{{.*}} %[[TERNARY]], %[[RETVAL:.*]] +// CIR: cir.yield +// CIR: } cleanup normal { +// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP0]]) +// CIR: cir.yield +// CIR: } // CIR: } // LLVM: define{{.*}} i1 @_Z13test_temp_andv(){{.*}} { >From ae6990c6b7fade560f5894eab1a8da7c534e9a58 Mon Sep 17 00:00:00 2001 From: Amr Hesham <[email protected]> Date: Wed, 25 Feb 2026 19:35:17 +0100 Subject: [PATCH 2/2] Address code review comments --- clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp | 58 ++++++++++++---------- clang/lib/CIR/CodeGen/CIRGenFunction.h | 5 ++ 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp index 1d8f1c0443e43..735fe1a85cfad 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp @@ -1306,8 +1306,6 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { CIRGenFunction::ConditionalEvaluation eval(cgf); mlir::Value lhsCondV = cgf.evaluateExprAsBool(e->getLHS()); - bool hasCleanupScopeParent = - mlir::isa<cir::CleanupScopeOp>(builder.getBlock()->getParentOp()); auto resOp = cir::TernaryOp::create( builder, loc, lhsCondV, /*trueBuilder=*/ @@ -1315,24 +1313,29 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { CIRGenFunction::LexicalScope lexScope{cgf, loc, b.getInsertionBlock()}; cgf.curLexScope->setAsTernary(); + mlir::Value res = cgf.evaluateExprAsBool(e->getRHS()); mlir::Value cleanupScopeTmpAlloca; - if (hasCleanupScopeParent) { - cleanupScopeTmpAlloca = builder.createAlloca( - loc, builder.getPointerTo(builder.getBoolTy()), - builder.getBoolTy(), "cleanup.scope.tmp", - clang::CharUnits::One()); - } + bool requiresCleanups = lexScope.requiresCleanups(); + if (requiresCleanups) { + { + mlir::OpBuilder::InsertionGuard guard(builder); + builder.setInsertionPointToStart( + res.getParentBlock()->getParentOp()->getBlock()); + + cleanupScopeTmpAlloca = builder.createAlloca( + loc, builder.getPointerTo(builder.getBoolTy()), + builder.getBoolTy(), "cleanup.scope.tmp", + clang::CharUnits::One()); + } - mlir::Value res = cgf.evaluateExprAsBool(e->getRHS()); - - if (hasCleanupScopeParent) cir::StoreOp::create(builder, loc, res, cleanupScopeTmpAlloca, - false, {}, {}, {}); + /*is_volatile=*/false, {}, {}, {}); + } lexScope.forceCleanup(); - if (hasCleanupScopeParent) + if (requiresCleanups) res = cir::LoadOp::create(builder, loc, cleanupScopeTmpAlloca); cir::YieldOp::create(b, loc, res); }, @@ -1371,8 +1374,6 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { CIRGenFunction::ConditionalEvaluation eval(cgf); mlir::Value lhsCondV = cgf.evaluateExprAsBool(e->getLHS()); - bool hasCleanupScopeParent = - mlir::isa<cir::CleanupScopeOp>(builder.getBlock()->getParentOp()); auto resOp = cir::TernaryOp::create( builder, loc, lhsCondV, /*trueBuilder=*/ @@ -1388,23 +1389,30 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { CIRGenFunction::LexicalScope lexScope{cgf, loc, b.getInsertionBlock()}; cgf.curLexScope->setAsTernary(); - mlir::Value cleanupScopeTmpAlloca; - if (hasCleanupScopeParent) { - cleanupScopeTmpAlloca = builder.createAlloca( - loc, builder.getPointerTo(builder.getBoolTy()), - builder.getBoolTy(), "cleanup.scope.tmp", - clang::CharUnits::One()); - } mlir::Value res = cgf.evaluateExprAsBool(e->getRHS()); - if (hasCleanupScopeParent) + mlir::Value cleanupScopeTmpAlloca; + bool requiresCleanups = lexScope.requiresCleanups(); + if (requiresCleanups) { + { + mlir::OpBuilder::InsertionGuard guard(builder); + builder.setInsertionPointToStart( + res.getParentBlock()->getParentOp()->getBlock()); + + cleanupScopeTmpAlloca = builder.createAlloca( + loc, builder.getPointerTo(builder.getBoolTy()), + builder.getBoolTy(), "cleanup.scope.tmp", + clang::CharUnits::One()); + } + cir::StoreOp::create(builder, loc, res, cleanupScopeTmpAlloca, - false, {}, {}, {}); + /*is_volatile=*/false, {}, {}, {}); + } lexScope.forceCleanup(); - if (hasCleanupScopeParent) + if (requiresCleanups) res = cir::LoadOp::create(builder, loc, cleanupScopeTmpAlloca); cir::YieldOp::create(b, loc, res); }); diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index bb29d47dbb0e5..b96bdff0a7224 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -1059,6 +1059,11 @@ class CIRGenFunction : public CIRGenTypeCache { performCleanup = false; cgf.currentCleanupStackDepth = oldCleanupStackDepth; } + + /// Determine whether this scope requires any cleanups. + bool requiresCleanups() const { + return cgf.ehStack.stable_begin() != cleanupStackDepth; + } }; // Cleanup stack depth of the RunCleanupsScope that was pushed most recently. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
