Skip to content

Commit 6154a31

Browse files
bruteforceboylanza
authored andcommitted
[CIR][CodeGen] Emit dtor properly for objects in TernaryOp (llvm#1614)
Currently, the following code snippet fails with a crash: ``` #include <stdio.h> struct X { int a; X(int a) : a(a) {} ~X() {} }; bool foo(const X &) { return false; } bool bar() { return foo(1) || foo(2); } ``` it fails with ``` error: 'cir.yield' op must be the last operation in the parent block ``` The crash can be traced to the [construction of the TernaryOp](https://github.com/llvm/clangir/blob/5df50096be1a783c26b48ea437523347df8a3110/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L2728). A [yield](https://github.com/llvm/clangir/blob/5df50096be1a783c26b48ea437523347df8a3110/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L2776) is created and when the LexicalScope is destroyed the destructors for the object follow. So, the CIR looks something like: ``` %11 = "cir.ternary"(%10) ({ %13 = "cir.const"() <{value = #cir.bool<true> : !cir.bool}> : () -> !cir.bool "cir.yield"(%13) : (!cir.bool) -> () }, { %12 = "cir.const"() <{value = #cir.bool<false> : !cir.bool}> : () -> !cir.bool "cir.yield"(%12) : (!cir.bool) -> () }) : (!cir.bool) -> !cir.bool "cir.yield"(%11) : (!cir.bool) -> () "cir.call"(%7) <{callee = @_ZN1XD2Ev, calling_conv = 1 : i32, extra_attrs = #cir<extra({nothrow = #cir.nothrow})>, side_effect = 1 : i32}> ({ }) ``` which is wrong, since the yield should come last. The fix here is forcing a cleanup and then the yield follows. A similar rule also applies [here](https://github.com/llvm/clangir/blob/5df50096be1a783c26b48ea437523347df8a3110/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L2657) for the "&&" case, e.g., foo(1) && foo(2) instead in the code snippet. One more modification I made is adding a check for when the current lexScope is ternary, when creating dispatch blocks for cleanups. I believe in this case, we do not want to create a dispatch block, please correct me if I am wrong. Finally, I add two tests to verify that everything works as intended.
1 parent 9c57aaa commit 6154a31

File tree

3 files changed

+59
-0
lines changed

3 files changed

+59
-0
lines changed

clang/lib/CIR/CodeGen/CIRGenException.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,8 @@ CIRGenFunction::getEHDispatchBlock(EHScopeStack::stable_iterator si,
806806
"one per call: expected empty region at this point");
807807
dispatchBlock = builder.createBlock(&callWithExceptionCtx.getCleanup());
808808
builder.createYield(callWithExceptionCtx.getLoc());
809+
} else if (currLexScope && currLexScope->isTernary()) {
810+
break;
809811
} else {
810812
// Usually coming from general cir.scope cleanups that aren't
811813
// tried to a specific throwing call.

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2677,6 +2677,7 @@ mlir::Value ScalarExprEmitter::VisitBinLAnd(const clang::BinaryOperator *E) {
26772677
auto res = b.create<cir::ConstantOp>(Loc, Builder.getFalseAttr());
26782678
b.create<cir::YieldOp>(Loc, res.getRes());
26792679
});
2680+
LexScope.ForceCleanup();
26802681
B.create<cir::YieldOp>(Loc, res.getResult());
26812682
},
26822683
/*falseBuilder*/
@@ -2772,6 +2773,7 @@ mlir::Value ScalarExprEmitter::VisitBinLOr(const clang::BinaryOperator *E) {
27722773
auto res = b.create<cir::ConstantOp>(Loc, Builder.getFalseAttr());
27732774
b.create<cir::YieldOp>(Loc, res.getRes());
27742775
});
2776+
LexScope.ForceCleanup();
27752777
B.create<cir::YieldOp>(Loc, res.getResult());
27762778
});
27772779

clang/test/CIR/CodeGen/dtors.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,61 @@ class B : public A
5050
// CHECK: cir.return
5151
// CHECK: }
5252

53+
struct X {
54+
int a;
55+
X(int a) : a(a) {}
56+
~X() {}
57+
};
58+
59+
bool foo(const X &) { return false; }
60+
bool bar() { return foo(1) || foo(2); }
61+
62+
// CHECK: cir.func @_Z3barv()
63+
// CHECK: %[[V0:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["__retval"] {alignment = 1 : i64}
64+
// CHECK: cir.scope {
65+
// CHECK: %[[V2:.*]] = cir.alloca !rec_X, !cir.ptr<!rec_X>, ["ref.tmp0"] {alignment = 4 : i64}
66+
// CHECK: %[[V3:.*]] = cir.const #cir.int<1> : !s32i
67+
// CHECK: cir.call @_ZN1XC2Ei(%[[V2]], %[[V3]]) : (!cir.ptr<!rec_X>, !s32i) -> ()
68+
// CHECK: %[[V4:.*]] = cir.call @_Z3fooRK1X(%[[V2]]) : (!cir.ptr<!rec_X>) -> !cir.bool
69+
// CHECK: %[[V5:.*]] = cir.ternary(%[[V4]], true {
70+
// CHECK: %[[V6:.*]] = cir.const #true
71+
// CHECK: cir.yield %[[V6]] : !cir.bool
72+
// CHECK: }, false {
73+
// CHECK: %[[V6:.*]] = cir.alloca !rec_X, !cir.ptr<!rec_X>, ["ref.tmp1"] {alignment = 4 : i64}
74+
// CHECK: %[[V7:.*]] = cir.const #cir.int<2> : !s32i
75+
// CHECK: cir.call @_ZN1XC2Ei(%[[V6]], %[[V7]]) : (!cir.ptr<!rec_X>, !s32i) -> ()
76+
// CHECK: %[[V8:.*]] = cir.call @_Z3fooRK1X(%[[V6]]) : (!cir.ptr<!rec_X>) -> !cir.bool
77+
// CHECK: %[[V9:.*]] = cir.ternary(%[[V8]], true {
78+
// CHECK: %[[V10:.*]] = cir.const #true
79+
// CHECK: cir.yield %[[V10]] : !cir.bool
80+
// CHECK: }, false {
81+
// CHECK: %[[V10:.*]] = cir.const #false
82+
// CHECK: cir.yield %[[V10]] : !cir.bool
83+
// CHECK: }) : (!cir.bool) -> !cir.bool
84+
// CHECK: cir.call @_ZN1XD2Ev(%[[V6]]) : (!cir.ptr<!rec_X>) -> ()
85+
// CHECK: cir.yield %[[V9]] : !cir.bool
86+
// CHECK: }) : (!cir.bool) -> !cir.bool
87+
// CHECK: cir.store %[[V5]], %[[V0]] : !cir.bool, !cir.ptr<!cir.bool>
88+
// CHECK: cir.call @_ZN1XD2Ev(%[[V2]]) : (!cir.ptr<!rec_X>) -> ()
89+
// CHECK: }
90+
// CHECK: %[[V1:.*]] = cir.load %[[V0]] : !cir.ptr<!cir.bool>, !cir.bool
91+
// CHECK: cir.return %[[V1]] : !cir.bool
92+
// CHECK: }
93+
94+
bool bar2() { return foo(1) && foo(2); }
95+
96+
// CHECK: cir.func @_Z4bar2v()
97+
// CHECK: cir.alloca !rec_X, !cir.ptr<!rec_X>
98+
// CHECK: {{.*}} = cir.ternary({{.*}}, true {
99+
// CHECK: cir.alloca !rec_X, !cir.ptr<!rec_X>
100+
// CHECK: cir.call @_ZN1XD2Ev
101+
// CHECK: cir.yield
102+
// CHECK: }, false {
103+
// CHECK: {{.*}} = cir.const #false
104+
// CHECK: cir.yield
105+
// CHECK: }) : (!cir.bool) -> !cir.bool
106+
// CHECK: cir.call @_ZN1XD2Ev
107+
53108
// @B::~B() #1 definition call into base @A::~A()
54109
// CHECK: cir.func linkonce_odr @_ZN1BD2Ev{{.*}}{
55110
// CHECK: cir.call @_ZN1AD2Ev(

0 commit comments

Comments
 (0)