Skip to content

Commit 222929a

Browse files
andykaylorandykaylor
authored
[CIR] Backport a fix for array element destruction order. (#1823)
Previously, when an array of destructible object went out of scope, the object destructors were called in the same order that they were constructed. This is incorrect. The objects should be destructed in reverse order. This change fixes that. This fixes #1759
1 parent 4fbec9f commit 222929a

File tree

3 files changed

+48
-26
lines changed

3 files changed

+48
-26
lines changed

‎clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,27 +1333,30 @@ void LoweringPreparePass::lowerDynamicCastOp(DynamicCastOp op) {
13331333

13341334
static void lowerArrayDtorCtorIntoLoop(CIRBaseBuilderTy &builder,
13351335
mlir::Operation *op, mlir::Type eltTy,
1336-
mlir::Value arrayAddr,
1337-
uint64_t arrayLen) {
1336+
mlir::Value arrayAddr, uint64_t arrayLen,
1337+
bool isCtor) {
13381338
// Generate loop to call into ctor/dtor for every element.
13391339
auto loc = op->getLoc();
13401340

13411341
// TODO: instead of fixed integer size, create alias for PtrDiffTy and unify
13421342
// with CIRGen stuff.
13431343
auto ptrDiffTy =
13441344
cir::IntType::get(builder.getContext(), 64, /*signed=*/false);
1345-
auto numArrayElementsConst = builder.create<cir::ConstantOp>(
1346-
loc, ptrDiffTy, cir::IntAttr::get(ptrDiffTy, arrayLen));
1345+
uint64_t endOffset = isCtor ? arrayLen : arrayLen - 1;
1346+
mlir::Value endOffsetVal = builder.create<cir::ConstantOp>(
1347+
loc, ptrDiffTy, cir::IntAttr::get(ptrDiffTy, endOffset));
13471348

13481349
auto begin = builder.create<cir::CastOp>(
13491350
loc, eltTy, cir::CastKind::array_to_ptrdecay, arrayAddr);
1350-
mlir::Value end = builder.create<cir::PtrStrideOp>(loc, eltTy, begin,
1351-
numArrayElementsConst);
1351+
mlir::Value end =
1352+
builder.create<cir::PtrStrideOp>(loc, eltTy, begin, endOffsetVal);
1353+
mlir::Value start = isCtor ? begin : end;
1354+
mlir::Value stop = isCtor ? end : begin;
13521355

13531356
auto tmpAddr = builder.createAlloca(
13541357
loc, /*addr type*/ builder.getPointerTo(eltTy),
13551358
/*var type*/ eltTy, "__array_idx", clang::CharUnits::One());
1356-
builder.createStore(loc, begin, tmpAddr);
1359+
builder.createStore(loc, start, tmpAddr);
13571360

13581361
auto loop = builder.createDoWhile(
13591362
loc,
@@ -1362,7 +1365,7 @@ static void lowerArrayDtorCtorIntoLoop(CIRBaseBuilderTy &builder,
13621365
auto currentElement = b.create<cir::LoadOp>(loc, eltTy, tmpAddr);
13631366
mlir::Type boolTy = cir::BoolType::get(b.getContext());
13641367
auto cmp = builder.create<cir::CmpOp>(loc, boolTy, cir::CmpOpKind::ne,
1365-
currentElement, end);
1368+
currentElement, stop);
13661369
builder.createCondition(cmp);
13671370
},
13681371
/*bodyBuilder=*/
@@ -1373,15 +1376,20 @@ static void lowerArrayDtorCtorIntoLoop(CIRBaseBuilderTy &builder,
13731376
op->walk([&](CallOp c) { ctorCall = c; });
13741377
assert(ctorCall && "expected ctor call");
13751378

1376-
auto one = builder.create<cir::ConstantOp>(
1377-
loc, ptrDiffTy, cir::IntAttr::get(ptrDiffTy, 1));
1379+
cir::ConstantOp stride;
1380+
if (isCtor)
1381+
stride = builder.create<cir::ConstantOp>(
1382+
loc, ptrDiffTy, cir::IntAttr::get(ptrDiffTy, 1));
1383+
else
1384+
stride = builder.create<cir::ConstantOp>(
1385+
loc, ptrDiffTy, cir::IntAttr::get(ptrDiffTy, -1));
13781386

1379-
ctorCall->moveAfter(one);
1387+
ctorCall->moveBefore(stride);
13801388
ctorCall->setOperand(0, currentElement);
13811389

13821390
// Advance pointer and store them to temporary variable
1383-
auto nextElement =
1384-
builder.create<cir::PtrStrideOp>(loc, eltTy, currentElement, one);
1391+
auto nextElement = builder.create<cir::PtrStrideOp>(
1392+
loc, eltTy, currentElement, stride);
13851393
builder.createStore(loc, nextElement, tmpAddr);
13861394
builder.createYield(loc);
13871395
});
@@ -1397,7 +1405,7 @@ void LoweringPreparePass::lowerArrayDtor(ArrayDtor op) {
13971405
auto eltTy = op->getRegion(0).getArgument(0).getType();
13981406
auto arrayLen =
13991407
mlir::cast<cir::ArrayType>(op.getAddr().getType().getPointee()).getSize();
1400-
lowerArrayDtorCtorIntoLoop(builder, op, eltTy, op.getAddr(), arrayLen);
1408+
lowerArrayDtorCtorIntoLoop(builder, op, eltTy, op.getAddr(), arrayLen, false);
14011409
}
14021410

14031411
static std::string getGlobalVarNameForConstString(cir::StoreOp op,
@@ -1460,7 +1468,7 @@ void LoweringPreparePass::lowerArrayCtor(ArrayCtor op) {
14601468
auto eltTy = op->getRegion(0).getArgument(0).getType();
14611469
auto arrayLen =
14621470
mlir::cast<cir::ArrayType>(op.getAddr().getType().getPointee()).getSize();
1463-
lowerArrayDtorCtorIntoLoop(builder, op, eltTy, op.getAddr(), arrayLen);
1471+
lowerArrayDtorCtorIntoLoop(builder, op, eltTy, op.getAddr(), arrayLen, true);
14641472
}
14651473

14661474
void LoweringPreparePass::lowerStdFindOp(StdFindOp op) {

‎clang/test/CIR/CodeGen/array-init-destroy.cpp

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -mmlir --mlir-print-ir-before=cir-lowering-prepare %s -o %t1.cir 2>&1 | FileCheck -check-prefix=BEFORE %s
2-
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -mmlir --mlir-print-ir-after=cir-lowering-prepare %s -o %t2.cir 2>&1 | FileCheck -check-prefix=AFTER %s
3-
1+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -mmlir --mlir-print-ir-before=cir-lowering-prepare %s -o %t.cir &> %t1.cir
2+
// RUN: FileCheck --input-file=%t1.cir -check-prefix=BEFORE %s
3+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -mmlir --mlir-print-ir-after=cir-lowering-prepare %s -o %t.cir &> %t2.cir
4+
// RUN: FileCheck --input-file=%t2.cir -check-prefix=AFTER %s
5+
// Note: The run lines above send the final CIR to %t.cir, but that's ignored.
6+
// The test checks the CIR before and after the cir-lowering-prepare pass.
47
void foo() noexcept;
58

69
class xpto {
@@ -43,8 +46,8 @@ void x() {
4346
// AFTER: cir.store %[[ArrayBegin]], %[[TmpIdx]] : !cir.ptr<!rec_xpto>, !cir.ptr<!cir.ptr<!rec_xpto>>
4447
// AFTER: cir.do {
4548
// AFTER: %[[ArrayElt:.*]] = cir.load %[[TmpIdx]] : !cir.ptr<!cir.ptr<!rec_xpto>>, !cir.ptr<!rec_xpto>
46-
// AFTER: %[[ConstOne:.*]] = cir.const #cir.int<1> : !u64i
4749
// AFTER: cir.call @_ZN4xptoC1Ev(%[[ArrayElt]]) : (!cir.ptr<!rec_xpto>) -> ()
50+
// AFTER: %[[ConstOne:.*]] = cir.const #cir.int<1> : !u64i
4851
// AFTER: %[[NextElt:.*]] = cir.ptr_stride(%[[ArrayElt]] : !cir.ptr<!rec_xpto>, %[[ConstOne]] : !u64i), !cir.ptr<!rec_xpto>
4952
// AFTER: cir.store %[[NextElt]], %[[TmpIdx]] : !cir.ptr<!rec_xpto>, !cir.ptr<!cir.ptr<!rec_xpto>>
5053
// AFTER: cir.yield
@@ -53,10 +56,21 @@ void x() {
5356
// AFTER: %[[ExitCond:.*]] = cir.cmp(ne, %[[ArrayElt]], %[[ArrayPastEnd]]) : !cir.ptr<!rec_xpto>, !cir.bool
5457
// AFTER: cir.condition(%[[ExitCond]])
5558
// AFTER: }
56-
57-
// AFTER: cir.do {
58-
// AFTER: cir.call @_ZN4xptoD1Ev({{.*}}) : (!cir.ptr<!rec_xpto>) -> ()
59-
// AFTER: } while {
60-
// AFTER: }
61-
59+
// AFTER: %[[ConstOne:.*]] = cir.const #cir.int<1> : !u64i
60+
// AFTER: %[[ArrayBegin:.*]] = cir.cast(array_to_ptrdecay, %[[ArrayAddr0]] : !cir.ptr<!cir.array<!rec_xpto x 2>>), !cir.ptr<!rec_xpto>
61+
// AFTER: %[[ArrayEnd:.*]] = cir.ptr_stride(%[[ArrayBegin]] : !cir.ptr<!rec_xpto>, %[[ConstOne]] : !u64i), !cir.ptr<!rec_xpto>
62+
// AFTER: %[[TmpIdx:.*]] = cir.alloca !cir.ptr<!rec_xpto>, !cir.ptr<!cir.ptr<!rec_xpto>>, ["__array_idx"] {alignment = 1 : i64}
63+
// AFTER: cir.store %[[ArrayEnd]], %[[TmpIdx]] : !cir.ptr<!rec_xpto>, !cir.ptr<!cir.ptr<!rec_xpto>>
64+
// AFTER cir.do {
65+
// AFTER %[[ArrElt:.*]] = cir.load{{.*}} %[[TmpIdx]]
66+
// AFTER cir.call @_ZN13array_elementD1Ev(%[[ArrElt]]) : (!cir.ptr<!rec_xpto>) -> ()
67+
// AFTER %[[ConstNegOne:.*]] = cir.const #cir.int<-1> : !s64i
68+
// AFTER %[[NextElt:.*]] = cir.ptr_stride(%[[ARR_CUR]] : !cir.ptr<!rec_xpto>, %[[ConstNegOne]] : !s64i), !cir.ptr<!rec_xpto>
69+
// AFTER cir.store %[[NextElt]], %[[TmpIdx]] : !cir.ptr<!rec_xpto>, !cir.ptr<!cir.ptr<!rec_xpto>>
70+
// AFTER cir.yield
71+
// AFTER } while {
72+
// AFTER %[[ArrElt:.*]] = cir.load %[[TmpIdx]] : !cir.ptr<!cir.ptr<!rec_xpto>>, !cir.ptr<!rec_xpto>
73+
// AFTER: %[[ExitCond:.*]] = cir.cmp(ne, %[[ArrayElt]], %[[ArrayBegin]]) : !cir.ptr<!rec_xpto>, !cir.bool
74+
// AFTER cir.condition(%[[ExitCond]])
75+
// AFTER }
6276
// AFTER: cir.return

‎clang/test/CIR/CodeGen/array-new-init.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ void t_new_constant_size_constructor() {
4848
// AFTER: cir.store{{.*}} %[[ELEM_PTR]], %[[CUR_ELEM_ALLOCA]] : !cir.ptr<!rec_E>, !cir.ptr<!cir.ptr<!rec_E>>
4949
// AFTER: cir.do {
5050
// AFTER: %[[CUR_ELEM_PTR:.*]] = cir.load %[[CUR_ELEM_ALLOCA]] : !cir.ptr<!cir.ptr<!rec_E>>, !cir.ptr<!rec_E>
51-
// AFTER: %[[OFFSET:.*]] = cir.const #cir.int<1> : !u64i
5251
// AFTER: cir.call @_ZN1EC1Ev(%[[CUR_ELEM_PTR]]) : (!cir.ptr<!rec_E>) -> ()
52+
// AFTER: %[[OFFSET:.*]] = cir.const #cir.int<1> : !u64i
5353
// AFTER: %[[NEXT_PTR:.*]] = cir.ptr_stride(%[[CUR_ELEM_PTR]] : !cir.ptr<!rec_E>, %[[OFFSET]] : !u64i), !cir.ptr<!rec_E>
5454
// AFTER: cir.store{{.*}} %[[NEXT_PTR]], %[[CUR_ELEM_ALLOCA]] : !cir.ptr<!rec_E>, !cir.ptr<!cir.ptr<!rec_E>>
5555
// AFTER: cir.yield

0 commit comments

Comments
 (0)