Skip to content

Commit b4a95fe

Browse files
andykaylorandykaylor
authored
[CIR] Fix destructor calls with temporary objects (#161922)
This fixes a few problems where destructors were not called for temporary objects and, after calling was enabled, they were placed incorrectly relative to cir.yield operations.
1 parent 3b9ec95 commit b4a95fe

File tree

7 files changed

+224
-13
lines changed

7 files changed

+224
-13
lines changed

‎clang/lib/CIR/CodeGen/CIRGenClass.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ void CIRGenFunction::emitCXXAggrConstructorCall(
690690
// every temporary created in a default argument expression is sequenced
691691
// before the construction of the next array element, if any.
692692
{
693-
assert(!cir::MissingFeatures::runCleanupsScope());
693+
RunCleanupsScope scope(*this);
694694

695695
// Evaluate the constructor and its arguments in a regular
696696
// partial-destroy cleanup.

‎clang/lib/CIR/CodeGen/CIRGenCleanup.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ using namespace clang::CIRGen;
2828
// CIRGenFunction cleanup related
2929
//===----------------------------------------------------------------------===//
3030

31+
/// Emits all the code to cause the given temporary to be cleaned up.
32+
void CIRGenFunction::emitCXXTemporary(const CXXTemporary *temporary,
33+
QualType tempType, Address ptr) {
34+
pushDestroy(NormalAndEHCleanup, ptr, tempType, destroyCXXObject);
35+
}
36+
3137
//===----------------------------------------------------------------------===//
3238
// EHScopeStack
3339
//===----------------------------------------------------------------------===//

‎clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
4646
return dest;
4747
}
4848

49+
void ensureDest(mlir::Location loc, QualType ty) {
50+
if (!dest.isIgnored())
51+
return;
52+
dest = cgf.createAggTemp(ty, loc, "agg.tmp.ensured");
53+
}
54+
4955
public:
5056
AggExprEmitter(CIRGenFunction &cgf, AggValueSlot dest)
5157
: cgf(cgf), dest(dest) {}
@@ -96,10 +102,22 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
96102
Visit(die->getExpr());
97103
}
98104
void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *e) {
99-
assert(!cir::MissingFeatures::aggValueSlotDestructedFlag());
105+
// Ensure that we have a slot, but if we already do, remember
106+
// whether it was externally destructed.
107+
bool wasExternallyDestructed = dest.isExternallyDestructed();
108+
ensureDest(cgf.getLoc(e->getSourceRange()), e->getType());
109+
110+
// We're going to push a destructor if there isn't already one.
111+
dest.setExternallyDestructed();
112+
100113
Visit(e->getSubExpr());
114+
115+
// Push that destructor we promised.
116+
if (!wasExternallyDestructed)
117+
cgf.emitCXXTemporary(e->getTemporary(), e->getType(), dest.getAddress());
101118
}
102119
void VisitLambdaExpr(LambdaExpr *e);
120+
void VisitExprWithCleanups(ExprWithCleanups *e);
103121

104122
// Stubs -- These should be moved up when they are implemented.
105123
void VisitCastExpr(CastExpr *e) {
@@ -241,11 +259,6 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
241259
cgf.cgm.errorNYI(e->getSourceRange(),
242260
"AggExprEmitter: VisitCXXStdInitializerListExpr");
243261
}
244-
245-
void VisitExprWithCleanups(ExprWithCleanups *e) {
246-
cgf.cgm.errorNYI(e->getSourceRange(),
247-
"AggExprEmitter: VisitExprWithCleanups");
248-
}
249262
void VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *e) {
250263
cgf.cgm.errorNYI(e->getSourceRange(),
251264
"AggExprEmitter: VisitCXXScalarValueInitExpr");
@@ -588,6 +601,11 @@ void AggExprEmitter::VisitLambdaExpr(LambdaExpr *e) {
588601
}
589602
}
590603

604+
void AggExprEmitter::VisitExprWithCleanups(ExprWithCleanups *e) {
605+
CIRGenFunction::RunCleanupsScope cleanups(cgf);
606+
Visit(e->getSubExpr());
607+
}
608+
591609
void AggExprEmitter::VisitCallExpr(const CallExpr *e) {
592610
if (e->getCallReturnType(cgf.getContext())->isReferenceType()) {
593611
cgf.cgm.errorNYI(e->getSourceRange(), "reference return type");

‎clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,15 +1099,17 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
10991099
CIRGenFunction::LexicalScope lexScope{cgf, loc,
11001100
b.getInsertionBlock()};
11011101
cgf.curLexScope->setAsTernary();
1102-
b.create<cir::YieldOp>(loc, cgf.evaluateExprAsBool(e->getRHS()));
1102+
mlir::Value res = cgf.evaluateExprAsBool(e->getRHS());
1103+
lexScope.forceCleanup();
1104+
cir::YieldOp::create(b, loc, res);
11031105
},
11041106
/*falseBuilder*/
11051107
[&](mlir::OpBuilder &b, mlir::Location loc) {
11061108
CIRGenFunction::LexicalScope lexScope{cgf, loc,
11071109
b.getInsertionBlock()};
11081110
cgf.curLexScope->setAsTernary();
1109-
auto res = b.create<cir::ConstantOp>(loc, builder.getFalseAttr());
1110-
b.create<cir::YieldOp>(loc, res.getRes());
1111+
auto res = cir::ConstantOp::create(b, loc, builder.getFalseAttr());
1112+
cir::YieldOp::create(b, loc, res.getRes());
11111113
});
11121114
return maybePromoteBoolResult(resOp.getResult(), resTy);
11131115
}
@@ -1143,15 +1145,17 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
11431145
CIRGenFunction::LexicalScope lexScope{cgf, loc,
11441146
b.getInsertionBlock()};
11451147
cgf.curLexScope->setAsTernary();
1146-
auto res = b.create<cir::ConstantOp>(loc, builder.getTrueAttr());
1147-
b.create<cir::YieldOp>(loc, res.getRes());
1148+
auto res = cir::ConstantOp::create(b, loc, builder.getTrueAttr());
1149+
cir::YieldOp::create(b, loc, res.getRes());
11481150
},
11491151
/*falseBuilder*/
11501152
[&](mlir::OpBuilder &b, mlir::Location loc) {
11511153
CIRGenFunction::LexicalScope lexScope{cgf, loc,
11521154
b.getInsertionBlock()};
11531155
cgf.curLexScope->setAsTernary();
1154-
b.create<cir::YieldOp>(loc, cgf.evaluateExprAsBool(e->getRHS()));
1156+
mlir::Value res = cgf.evaluateExprAsBool(e->getRHS());
1157+
lexScope.forceCleanup();
1158+
cir::YieldOp::create(b, loc, res);
11551159
});
11561160

11571161
return maybePromoteBoolResult(resOp.getResult(), resTy);

‎clang/lib/CIR/CodeGen/CIRGenFunction.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,6 +1258,9 @@ class CIRGenFunction : public CIRGenTypeCache {
12581258

12591259
RValue emitCXXPseudoDestructorExpr(const CXXPseudoDestructorExpr *expr);
12601260

1261+
void emitCXXTemporary(const CXXTemporary *temporary, QualType tempType,
1262+
Address ptr);
1263+
12611264
void emitCXXThrowExpr(const CXXThrowExpr *e);
12621265

12631266
void emitCtorPrologue(const clang::CXXConstructorDecl *ctor,

‎clang/lib/CIR/CodeGen/CIRGenValue.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,13 @@ class AggValueSlot {
371371
mayOverlap, isZeroed);
372372
}
373373

374+
IsDestructed_t isExternallyDestructed() const {
375+
return IsDestructed_t(destructedFlag);
376+
}
377+
void setExternallyDestructed(bool destructed = true) {
378+
destructedFlag = destructed;
379+
}
380+
374381
clang::Qualifiers getQualifiers() const { return quals; }
375382

376383
Address getAddress() const { return addr; }

‎clang/test/CIR/CodeGen/dtors.cpp‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -fclangir -emit-cir %s -o %t.cir
2+
// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
3+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -fclangir -emit-llvm %s -o %t-cir.ll
4+
// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefix=LLVM
5+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -emit-llvm %s -o %t.ll
6+
// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
7+
8+
struct A {
9+
~A();
10+
};
11+
12+
void test_temporary_dtor() {
13+
A();
14+
}
15+
16+
// CIR: cir.func dso_local @_Z19test_temporary_dtorv()
17+
// CIR: %[[ALLOCA:.*]] = cir.alloca !rec_A, !cir.ptr<!rec_A>, ["agg.tmp0"]
18+
// CIR: cir.call @_ZN1AD1Ev(%[[ALLOCA]]) nothrow : (!cir.ptr<!rec_A>) -> ()
19+
20+
// LLVM: define dso_local void @_Z19test_temporary_dtorv()
21+
// LLVM: %[[ALLOCA:.*]] = alloca %struct.A, i64 1, align 1
22+
// LLVM: call void @_ZN1AD1Ev(ptr %[[ALLOCA]])
23+
24+
// OGCG: define dso_local void @_Z19test_temporary_dtorv()
25+
// OGCG: %[[ALLOCA:.*]] = alloca %struct.A, align 1
26+
// OGCG: call void @_ZN1AD1Ev(ptr {{.*}} %[[ALLOCA]])
27+
28+
struct B {
29+
int n;
30+
B(int n) : n(n) {}
31+
~B() {}
32+
};
33+
34+
bool make_temp(const B &) { return false; }
35+
bool test_temp_or() { return make_temp(1) || make_temp(2); }
36+
37+
// CIR: cir.func{{.*}} @_Z12test_temp_orv()
38+
// CIR: %[[SCOPE:.*]] = cir.scope {
39+
// CIR: %[[REF_TMP0:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp0"]
40+
// CIR: %[[ONE:.*]] = cir.const #cir.int<1>
41+
// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP0]], %[[ONE]])
42+
// CIR: %[[MAKE_TEMP0:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP0]])
43+
// CIR: %[[TERNARY:.*]] = cir.ternary(%[[MAKE_TEMP0]], true {
44+
// CIR: %[[TRUE:.*]] = cir.const #true
45+
// CIR: cir.yield %[[TRUE]] : !cir.bool
46+
// CIR: }, false {
47+
// CIR: %[[REF_TMP1:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp1"]
48+
// CIR: %[[TWO:.*]] = cir.const #cir.int<2>
49+
// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP1]], %[[TWO]])
50+
// CIR: %[[MAKE_TEMP1:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP1]])
51+
// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP1]])
52+
// CIR: cir.yield %[[MAKE_TEMP1]] : !cir.bool
53+
// CIR: })
54+
// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP0]])
55+
// CIR: cir.yield %[[TERNARY]] : !cir.bool
56+
// CIR: } : !cir.bool
57+
58+
// LLVM: define{{.*}} i1 @_Z12test_temp_orv() {
59+
// LLVM: %[[REF_TMP0:.*]] = alloca %struct.B
60+
// LLVM: %[[REF_TMP1:.*]] = alloca %struct.B
61+
// LLVM: br label %[[LOR_BEGIN:.*]]
62+
// LLVM: [[LOR_BEGIN]]:
63+
// LLVM: call void @_ZN1BC2Ei(ptr %[[REF_TMP0]], i32 1)
64+
// LLVM: %[[MAKE_TEMP0:.*]] = call i1 @_Z9make_tempRK1B(ptr %[[REF_TMP0]])
65+
// LLVM: br i1 %[[MAKE_TEMP0]], label %[[LHS_TRUE_BLOCK:.*]], label %[[LHS_FALSE_BLOCK:.*]]
66+
// LLVM: [[LHS_TRUE_BLOCK]]:
67+
// LLVM: br label %[[RESULT_BLOCK:.*]]
68+
// LLVM: [[LHS_FALSE_BLOCK]]:
69+
// LLVM: call void @_ZN1BC2Ei(ptr %[[REF_TMP1]], i32 2)
70+
// LLVM: %[[MAKE_TEMP1:.*]] = call i1 @_Z9make_tempRK1B(ptr %[[REF_TMP1]])
71+
// LLVM: call void @_ZN1BD2Ev(ptr %[[REF_TMP1]])
72+
// LLVM: br label %[[RESULT_BLOCK]]
73+
// LLVM: [[RESULT_BLOCK]]:
74+
// LLVM: %[[RESULT:.*]] = phi i1 [ %[[MAKE_TEMP1]], %[[LHS_FALSE_BLOCK]] ], [ true, %[[LHS_TRUE_BLOCK]] ]
75+
// LLVM: br label %[[LOR_END:.*]]
76+
// LLVM: [[LOR_END]]:
77+
// LLVM: call void @_ZN1BD2Ev(ptr %[[REF_TMP0]])
78+
79+
// OGCG: define {{.*}} i1 @_Z12test_temp_orv()
80+
// OGCG: [[ENTRY:.*]]:
81+
// OGCG: %[[RETVAL:.*]] = alloca i1
82+
// OGCG: %[[REF_TMP0:.*]] = alloca %struct.B
83+
// OGCG: %[[REF_TMP1:.*]] = alloca %struct.B
84+
// OGCG: %[[CLEANUP_COND:.*]] = alloca i1
85+
// OGCG: call void @_ZN1BC2Ei(ptr {{.*}} %[[REF_TMP0]], i32 {{.*}} 1)
86+
// OGCG: %[[MAKE_TEMP0:.*]] = call {{.*}} i1 @_Z9make_tempRK1B(ptr {{.*}} %[[REF_TMP0]])
87+
// OGCG: store i1 false, ptr %cleanup.cond
88+
// OGCG: br i1 %[[MAKE_TEMP0]], label %[[LOR_END:.*]], label %[[LOR_RHS:.*]]
89+
// OGCG: [[LOR_RHS]]:
90+
// OGCG: call void @_ZN1BC2Ei(ptr {{.*}} %[[REF_TMP1]], i32 {{.*}} 2)
91+
// OGCG: store i1 true, ptr %[[CLEANUP_COND]]
92+
// OGCG: %[[MAKE_TEMP1:.*]] = call {{.*}} i1 @_Z9make_tempRK1B(ptr {{.*}} %[[REF_TMP1]])
93+
// OGCG: br label %[[LOR_END]]
94+
// OGCG: [[LOR_END]]:
95+
// OGCG: %[[PHI:.*]] = phi i1 [ true, %[[ENTRY]] ], [ %[[MAKE_TEMP1]], %[[LOR_RHS]] ]
96+
// OGCG: store i1 %[[PHI]], ptr %[[RETVAL]]
97+
// OGCG: %[[CLEANUP_IS_ACTIVE:.*]] = load i1, ptr %[[CLEANUP_COND]]
98+
// OGCG: br i1 %[[CLEANUP_IS_ACTIVE]], label %[[CLEANUP_ACTION:.*]], label %[[CLEANUP_DONE:.*]]
99+
// OGCG: [[CLEANUP_ACTION]]:
100+
// OGCG: call void @_ZN1BD2Ev(ptr {{.*}} %[[REF_TMP1]])
101+
// OGCG: br label %[[CLEANUP_DONE]]
102+
// OGCG: [[CLEANUP_DONE]]:
103+
// OGCG: call void @_ZN1BD2Ev(ptr {{.*}} %[[REF_TMP0]])
104+
105+
bool test_temp_and() { return make_temp(1) && make_temp(2); }
106+
107+
// CIR: cir.func{{.*}} @_Z13test_temp_andv()
108+
// CIR: %[[SCOPE:.*]] = cir.scope {
109+
// CIR: %[[REF_TMP0:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp0"]
110+
// CIR: %[[ONE:.*]] = cir.const #cir.int<1>
111+
// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP0]], %[[ONE]])
112+
// CIR: %[[MAKE_TEMP0:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP0]])
113+
// CIR: %[[TERNARY:.*]] = cir.ternary(%[[MAKE_TEMP0]], true {
114+
// CIR: %[[REF_TMP1:.*]] = cir.alloca !rec_B, !cir.ptr<!rec_B>, ["ref.tmp1"]
115+
// CIR: %[[TWO:.*]] = cir.const #cir.int<2>
116+
// CIR: cir.call @_ZN1BC2Ei(%[[REF_TMP1]], %[[TWO]])
117+
// CIR: %[[MAKE_TEMP1:.*]] = cir.call @_Z9make_tempRK1B(%[[REF_TMP1]])
118+
// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP1]])
119+
// CIR: cir.yield %[[MAKE_TEMP1]] : !cir.bool
120+
// CIR: }, false {
121+
// CIR: %[[FALSE:.*]] = cir.const #false
122+
// CIR: cir.yield %[[FALSE]] : !cir.bool
123+
// CIR: })
124+
// CIR: cir.call @_ZN1BD2Ev(%[[REF_TMP0]])
125+
// CIR: cir.yield %[[TERNARY]] : !cir.bool
126+
// CIR: } : !cir.bool
127+
128+
// LLVM: define{{.*}} i1 @_Z13test_temp_andv() {
129+
// LLVM: %[[REF_TMP0:.*]] = alloca %struct.B
130+
// LLVM: %[[REF_TMP1:.*]] = alloca %struct.B
131+
// LLVM: br label %[[LAND_BEGIN:.*]]
132+
// LLVM: [[LAND_BEGIN]]:
133+
// LLVM: call void @_ZN1BC2Ei(ptr %[[REF_TMP0]], i32 1)
134+
// LLVM: %[[MAKE_TEMP0:.*]] = call i1 @_Z9make_tempRK1B(ptr %[[REF_TMP0]])
135+
// LLVM: br i1 %[[MAKE_TEMP0]], label %[[LHS_TRUE_BLOCK:.*]], label %[[LHS_FALSE_BLOCK:.*]]
136+
// LLVM: [[LHS_TRUE_BLOCK]]:
137+
// LLVM: call void @_ZN1BC2Ei(ptr %[[REF_TMP1]], i32 2)
138+
// LLVM: %[[MAKE_TEMP1:.*]] = call i1 @_Z9make_tempRK1B(ptr %[[REF_TMP1]])
139+
// LLVM: call void @_ZN1BD2Ev(ptr %[[REF_TMP1]])
140+
// LLVM: br label %[[RESULT_BLOCK:.*]]
141+
// LLVM: [[LHS_FALSE_BLOCK]]:
142+
// LLVM: br label %[[RESULT_BLOCK]]
143+
// LLVM: [[RESULT_BLOCK]]:
144+
// LLVM: %[[RESULT:.*]] = phi i1 [ false, %[[LHS_FALSE_BLOCK]] ], [ %[[MAKE_TEMP1]], %[[LHS_TRUE_BLOCK]] ]
145+
// LLVM: br label %[[LAND_END:.*]]
146+
// LLVM: [[LAND_END]]:
147+
// LLVM: call void @_ZN1BD2Ev(ptr %[[REF_TMP0]])
148+
149+
// OGCG: define {{.*}} i1 @_Z13test_temp_andv()
150+
// OGCG: [[ENTRY:.*]]:
151+
// OGCG: %[[RETVAL:.*]] = alloca i1
152+
// OGCG: %[[REF_TMP0:.*]] = alloca %struct.B
153+
// OGCG: %[[REF_TMP1:.*]] = alloca %struct.B
154+
// OGCG: %[[CLEANUP_COND:.*]] = alloca i1
155+
// OGCG: call void @_ZN1BC2Ei(ptr {{.*}} %[[REF_TMP0]], i32 {{.*}} 1)
156+
// OGCG: %[[MAKE_TEMP0:.*]] = call {{.*}} i1 @_Z9make_tempRK1B(ptr {{.*}} %[[REF_TMP0]])
157+
// OGCG: store i1 false, ptr %cleanup.cond
158+
// OGCG: br i1 %[[MAKE_TEMP0]], label %[[LAND_RHS:.*]], label %[[LAND_END:.*]]
159+
// OGCG: [[LAND_RHS]]:
160+
// OGCG: call void @_ZN1BC2Ei(ptr {{.*}} %[[REF_TMP1]], i32 {{.*}} 2)
161+
// OGCG: store i1 true, ptr %[[CLEANUP_COND]]
162+
// OGCG: %[[MAKE_TEMP1:.*]] = call {{.*}} i1 @_Z9make_tempRK1B(ptr {{.*}} %[[REF_TMP1]])
163+
// OGCG: br label %[[LAND_END]]
164+
// OGCG: [[LAND_END]]:
165+
// OGCG: %[[PHI:.*]] = phi i1 [ false, %[[ENTRY]] ], [ %[[MAKE_TEMP1]], %[[LAND_RHS]] ]
166+
// OGCG: store i1 %[[PHI]], ptr %[[RETVAL]]
167+
// OGCG: %[[CLEANUP_IS_ACTIVE:.*]] = load i1, ptr %[[CLEANUP_COND]]
168+
// OGCG: br i1 %[[CLEANUP_IS_ACTIVE]], label %[[CLEANUP_ACTION:.*]], label %[[CLEANUP_DONE:.*]]
169+
// OGCG: [[CLEANUP_ACTION]]:
170+
// OGCG: call void @_ZN1BD2Ev(ptr {{.*}} %[[REF_TMP1]])
171+
// OGCG: br label %[[CLEANUP_DONE]]
172+
// OGCG: [[CLEANUP_DONE]]:
173+
// OGCG: call void @_ZN1BD2Ev(ptr {{.*}} %[[REF_TMP0]])

0 commit comments

Comments
 (0)