-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[CIR] Add support for array cleanups #150499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This adds initial support for array cleanups, including the ArrayDtor op.
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis adds support for array cleanups, including the ArrayDtor op. Patch is 20.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/150499.diff 8 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index 5c04d59475b6a..b6dd4eecaef67 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -75,6 +75,12 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
return getConstant(loc, cir::IntAttr::get(ty, value));
}
+ mlir::Value getSignedInt(mlir::Location loc, int64_t val, unsigned numBits) {
+ auto type = cir::IntType::get(getContext(), numBits, /*isSigned=*/true);
+ return getConstAPInt(loc, type,
+ llvm::APInt(numBits, val, /*isSigned=*/true));
+ }
+
mlir::Value getUnsignedInt(mlir::Location loc, uint64_t val,
unsigned numBits) {
auto type = cir::IntType::get(getContext(), numBits, /*isSigned=*/false);
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 32bb9009aeec9..14147f8c80133 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -607,8 +607,8 @@ def CIR_ConditionOp : CIR_Op<"condition", [
//===----------------------------------------------------------------------===//
defvar CIR_YieldableScopes = [
- "ArrayCtor", "CaseOp", "DoWhileOp", "ForOp", "IfOp", "ScopeOp", "SwitchOp",
- "TernaryOp", "WhileOp"
+ "ArrayCtor", "ArrayDtor", "CaseOp", "DoWhileOp", "ForOp", "IfOp", "ScopeOp",
+ "SwitchOp", "TernaryOp", "WhileOp"
];
def CIR_YieldOp : CIR_Op<"yield", [
@@ -2229,7 +2229,7 @@ def CIR_TrapOp : CIR_Op<"trap", [Terminator]> {
}
//===----------------------------------------------------------------------===//
-// ArrayCtor
+// ArrayCtor & ArrayDtor
//===----------------------------------------------------------------------===//
class CIR_ArrayInitDestroy<string mnemonic> : CIR_Op<mnemonic> {
@@ -2272,6 +2272,23 @@ def CIR_ArrayCtor : CIR_ArrayInitDestroy<"array.ctor"> {
}];
}
+def CIR_ArrayDtor : CIR_ArrayInitDestroy<"array.dtor"> {
+ let summary = "Destroy array elements with C++ dtors";
+ let description = [{
+ Destroy each array element using the same C++ destructor. This
+ operation has one region, with one single block. The block has an
+ incoming argument for the current array index to initialize.
+
+ ```mlir
+ cir.array.dtor(%0 : !cir.ptr<!cir.array<!rec_S x 42>>) {
+ ^bb0(%arg0: !cir.ptr<!rec_S>):
+ cir.call @some_dtor(%arg0) : (!cir.ptr<!rec_S>) -> ()
+ cir.yield
+ }
+ ```
+ }];
+}
+
//===----------------------------------------------------------------------===//
// VecCreate
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index a28ac3c16ce59..f8dd2c6097b1c 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -649,6 +649,41 @@ void CIRGenFunction::emitNullabilityCheck(LValue lhs, mlir::Value rhs,
assert(!cir::MissingFeatures::sanitizers());
}
+/// Destroys all the elements of the given array, beginning from last to first.
+/// The array cannot be zero-length.
+///
+/// \param begin - a type* denoting the first element of the array
+/// \param end - a type* denoting one past the end of the array
+/// \param elementType - the element type of the array
+/// \param destroyer - the function to call to destroy elements
+void CIRGenFunction::emitArrayDestroy(mlir::Value begin, mlir::Value end,
+ QualType elementType,
+ CharUnits elementAlign,
+ Destroyer *destroyer,
+ bool checkZeroLength) {
+ assert(!elementType->isArrayType());
+ if (checkZeroLength)
+ cgm.errorNYI("emitArrayDestroy: check for zero length");
+
+ // Differently from LLVM traditional codegen, use a higher level
+ // representation instead of lowering directly to a loop.
+ mlir::Type cirElementType = convertTypeForMem(elementType);
+ cir::PointerType ptrToElmType = builder.getPointerTo(cirElementType);
+
+ // Emit the dtor call that will execute for every array element.
+ builder.create<cir::ArrayDtor>(
+ *currSrcLoc, begin, [&](mlir::OpBuilder &b, mlir::Location loc) {
+ auto arg = b.getInsertionBlock()->addArgument(ptrToElmType, loc);
+ Address curAddr = Address(arg, cirElementType, elementAlign);
+ assert(!cir::MissingFeatures::dtorCleanups());
+
+ // Perform the actual destruction there.
+ destroyer(*this, curAddr, elementType);
+
+ builder.create<cir::YieldOp>(loc);
+ });
+}
+
/// Immediately perform the destruction of the given object.
///
/// \param addr - the address of the object; a type*
@@ -658,10 +693,38 @@ void CIRGenFunction::emitNullabilityCheck(LValue lhs, mlir::Value rhs,
/// elements
void CIRGenFunction::emitDestroy(Address addr, QualType type,
Destroyer *destroyer) {
- if (getContext().getAsArrayType(type))
- cgm.errorNYI("emitDestroy: array type");
+ const ArrayType *arrayType = getContext().getAsArrayType(type);
+ if (!arrayType)
+ return destroyer(*this, addr, type);
+
+ mlir::Value length = emitArrayLength(arrayType, type, addr);
+
+ CharUnits elementAlign = addr.getAlignment().alignmentOfArrayElement(
+ getContext().getTypeSizeInChars(type));
+
+ // Normally we have to check whether the array is zero-length.
+ bool checkZeroLength = true;
+
+ // But if the array length is constant, we can suppress that.
+ auto constantCount = dyn_cast<cir::ConstantOp>(length.getDefiningOp());
+ if (constantCount) {
+ auto constIntAttr = mlir::dyn_cast<cir::IntAttr>(constantCount.getValue());
+ // ...and if it's constant zero, we can just skip the entire thing.
+ if (constIntAttr && constIntAttr.getUInt() == 0)
+ return;
+ checkZeroLength = false;
+ } else {
+ cgm.errorNYI("emitDestroy: variable length array");
+ return;
+ }
+
+ mlir::Value begin = addr.getPointer();
+ mlir::Value end; // This will be used for future non-constant counts.
+ emitArrayDestroy(begin, end, type, elementAlign, destroyer, checkZeroLength);
- return destroyer(*this, addr, type);
+ // If the array destroy didn't use the length op, we can erase it.
+ if (constantCount.use_empty())
+ constantCount.erase();
}
CIRGenFunction::Destroyer *
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 4891c7496588f..48e5fd079efec 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -848,6 +848,10 @@ class CIRGenFunction : public CIRGenTypeCache {
/// even if no aggregate location is provided.
RValue emitAnyExprToTemp(const clang::Expr *e);
+ void emitArrayDestroy(mlir::Value begin, mlir::Value end,
+ QualType elementType, CharUnits elementAlign,
+ Destroyer *destroyer, bool checkZeroLength);
+
mlir::Value emitArrayLength(const clang::ArrayType *arrayType,
QualType &baseType, Address &addr);
LValue emitArraySubscriptExpr(const clang::ArraySubscriptExpr *e);
diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
index cef83eae2ef50..0625ffd55b3ae 100644
--- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
@@ -29,6 +29,7 @@ struct LoweringPreparePass : public LoweringPrepareBase<LoweringPreparePass> {
void runOnOp(mlir::Operation *op);
void lowerCastOp(cir::CastOp op);
void lowerUnaryOp(cir::UnaryOp op);
+ void lowerArrayDtor(ArrayDtor op);
void lowerArrayCtor(ArrayCtor op);
///
@@ -172,28 +173,29 @@ void LoweringPreparePass::lowerUnaryOp(cir::UnaryOp op) {
static void lowerArrayDtorCtorIntoLoop(cir::CIRBaseBuilderTy &builder,
clang::ASTContext *astCtx,
mlir::Operation *op, mlir::Type eltTy,
- mlir::Value arrayAddr,
- uint64_t arrayLen) {
+ mlir::Value arrayAddr, uint64_t arrayLen,
+ bool isCtor) {
// Generate loop to call into ctor/dtor for every element.
mlir::Location loc = op->getLoc();
- // TODO: instead of fixed integer size, create alias for PtrDiffTy and unify
- // with CIRGen stuff.
+ // TODO: instead of getting the size from the AST context, create alias for
+ // PtrDiffTy and unify with CIRGen stuff.
const unsigned sizeTypeSize =
astCtx->getTypeSize(astCtx->getSignedSizeType());
- auto ptrDiffTy =
- cir::IntType::get(builder.getContext(), sizeTypeSize, /*isSigned=*/false);
- mlir::Value numArrayElementsConst = builder.getUnsignedInt(loc, arrayLen, 64);
+ mlir::Value numArrayElementsConst =
+ builder.getUnsignedInt(loc, arrayLen, sizeTypeSize);
auto begin = builder.create<cir::CastOp>(
loc, eltTy, cir::CastKind::array_to_ptrdecay, arrayAddr);
mlir::Value end = builder.create<cir::PtrStrideOp>(loc, eltTy, begin,
numArrayElementsConst);
+ mlir::Value start = isCtor ? begin : end;
+ mlir::Value stop = isCtor ? end : begin;
mlir::Value tmpAddr = builder.createAlloca(
loc, /*addr type*/ builder.getPointerTo(eltTy),
/*var type*/ eltTy, "__array_idx", builder.getAlignmentAttr(1));
- builder.createStore(loc, begin, tmpAddr);
+ builder.createStore(loc, start, tmpAddr);
cir::DoWhileOp loop = builder.createDoWhile(
loc,
@@ -202,7 +204,7 @@ static void lowerArrayDtorCtorIntoLoop(cir::CIRBaseBuilderTy &builder,
auto currentElement = b.create<cir::LoadOp>(loc, eltTy, tmpAddr);
mlir::Type boolTy = cir::BoolType::get(b.getContext());
auto cmp = builder.create<cir::CmpOp>(loc, boolTy, cir::CmpOpKind::ne,
- currentElement, end);
+ currentElement, stop);
builder.createCondition(cmp);
},
/*bodyBuilder=*/
@@ -213,15 +215,23 @@ static void lowerArrayDtorCtorIntoLoop(cir::CIRBaseBuilderTy &builder,
op->walk([&](cir::CallOp c) { ctorCall = c; });
assert(ctorCall && "expected ctor call");
- auto one = builder.create<cir::ConstantOp>(
- loc, ptrDiffTy, cir::IntAttr::get(ptrDiffTy, 1));
-
- ctorCall->moveAfter(one);
- ctorCall->setOperand(0, currentElement);
-
- // Advance pointer and store them to temporary variable
- auto nextElement =
- builder.create<cir::PtrStrideOp>(loc, eltTy, currentElement, one);
+ // Array elements get constructed in order but destructed in reverse.
+ cir::PtrStrideOp nextElement;
+ if (isCtor) {
+ mlir::Value stride = builder.getUnsignedInt(loc, 1, sizeTypeSize);
+ ctorCall->moveBefore(stride.getDefiningOp());
+ ctorCall->setOperand(0, currentElement);
+ nextElement = builder.create<cir::PtrStrideOp>(
+ loc, eltTy, currentElement, stride);
+ } else {
+ mlir::Value stride = builder.getSignedInt(loc, -1, sizeTypeSize);
+ nextElement = builder.create<cir::PtrStrideOp>(
+ loc, eltTy, currentElement, stride);
+ ctorCall->moveAfter(nextElement);
+ ctorCall->setOperand(0, nextElement);
+ }
+
+ // Store the element pointer to the temporary variable
builder.createStore(loc, nextElement, tmpAddr);
builder.createYield(loc);
});
@@ -230,6 +240,17 @@ static void lowerArrayDtorCtorIntoLoop(cir::CIRBaseBuilderTy &builder,
op->erase();
}
+void LoweringPreparePass::lowerArrayDtor(ArrayDtor op) {
+ CIRBaseBuilderTy builder(getContext());
+ builder.setInsertionPointAfter(op.getOperation());
+
+ mlir::Type eltTy = op->getRegion(0).getArgument(0).getType();
+ auto arrayLen =
+ mlir::cast<cir::ArrayType>(op.getAddr().getType().getPointee()).getSize();
+ lowerArrayDtorCtorIntoLoop(builder, astCtx, op, eltTy, op.getAddr(), arrayLen,
+ false);
+}
+
void LoweringPreparePass::lowerArrayCtor(cir::ArrayCtor op) {
cir::CIRBaseBuilderTy builder(getContext());
builder.setInsertionPointAfter(op.getOperation());
@@ -238,8 +259,8 @@ void LoweringPreparePass::lowerArrayCtor(cir::ArrayCtor op) {
assert(!cir::MissingFeatures::vlas());
auto arrayLen =
mlir::cast<cir::ArrayType>(op.getAddr().getType().getPointee()).getSize();
- lowerArrayDtorCtorIntoLoop(builder, astCtx, op, eltTy, op.getAddr(),
- arrayLen);
+ lowerArrayDtorCtorIntoLoop(builder, astCtx, op, eltTy, op.getAddr(), arrayLen,
+ true);
}
void LoweringPreparePass::runOnOp(mlir::Operation *op) {
@@ -249,6 +270,8 @@ void LoweringPreparePass::runOnOp(mlir::Operation *op) {
lowerCastOp(cast);
else if (auto unary = mlir::dyn_cast<cir::UnaryOp>(op))
lowerUnaryOp(unary);
+ else if (auto arrayDtor = dyn_cast<ArrayDtor>(op))
+ lowerArrayDtor(arrayDtor);
}
void LoweringPreparePass::runOnOperation() {
@@ -257,7 +280,8 @@ void LoweringPreparePass::runOnOperation() {
llvm::SmallVector<mlir::Operation *> opsToTransform;
op->walk([&](mlir::Operation *op) {
- if (mlir::isa<cir::ArrayCtor, cir::CastOp, cir::UnaryOp>(op))
+ if (mlir::isa<cir::ArrayCtor, cir::ArrayDtor, cir::CastOp, cir::UnaryOp>(
+ op))
opsToTransform.push_back(op);
});
diff --git a/clang/test/CIR/CodeGen/array-ctor.cpp b/clang/test/CIR/CodeGen/array-ctor.cpp
index b3d81a8ab82a4..c373acf0bff8c 100644
--- a/clang/test/CIR/CodeGen/array-ctor.cpp
+++ b/clang/test/CIR/CodeGen/array-ctor.cpp
@@ -33,8 +33,8 @@ void foo() {
// CIR: cir.store %[[DECAY]], %[[ITER]] : !cir.ptr<!rec_S>, !cir.ptr<!cir.ptr<!rec_S>>
// CIR: cir.do {
// CIR: %[[CURRENT:.*]] = cir.load %[[ITER]] : !cir.ptr<!cir.ptr<!rec_S>>, !cir.ptr<!rec_S>
-// CIR: %[[CONST1:.*]] = cir.const #cir.int<1> : !u64i
// CIR: cir.call @_ZN1SC1Ev(%[[CURRENT]]) : (!cir.ptr<!rec_S>) -> ()
+// CIR: %[[CONST1:.*]] = cir.const #cir.int<1> : !u64i
// CIR: %[[NEXT:.*]] = cir.ptr_stride(%[[CURRENT]] : !cir.ptr<!rec_S>, %[[CONST1]] : !u64i), !cir.ptr<!rec_S>
// CIR: cir.store %[[NEXT]], %[[ITER]] : !cir.ptr<!rec_S>, !cir.ptr<!cir.ptr<!rec_S>>
// CIR: cir.yield
diff --git a/clang/test/CIR/CodeGen/array-dtor.cpp b/clang/test/CIR/CodeGen/array-dtor.cpp
new file mode 100644
index 0000000000000..f0d43f0d49519
--- /dev/null
+++ b/clang/test/CIR/CodeGen/array-dtor.cpp
@@ -0,0 +1,104 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-cir -mmlir --mlir-print-ir-before=cir-lowering-prepare %s -o %t.cir 2> %t-before-lp.cir
+// RUN: FileCheck --input-file=%t-before-lp.cir %s -check-prefix=CIR-BEFORE-LPP
+// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -Wno-unused-value -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 -Wno-unused-value -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG
+
+struct S {
+ ~S();
+};
+
+void test_cleanup_array() {
+ S s[42];
+}
+
+// CIR-BEFORE-LPP: cir.func{{.*}} @_Z18test_cleanup_arrayv()
+// CIR-BEFORE-LPP: %[[S:.*]] = cir.alloca !cir.array<!rec_S x 42>, !cir.ptr<!cir.array<!rec_S x 42>>, ["s"]
+// CIR-BEFORE-LPP: cir.array.dtor %[[S]] : !cir.ptr<!cir.array<!rec_S x 42>> {
+// CIR-BEFORE-LPP: ^bb0(%arg0: !cir.ptr<!rec_S>
+// CIR-BEFORE-LPP: cir.call @_ZN1SD1Ev(%arg0) nothrow : (!cir.ptr<!rec_S>) -> ()
+// CIR-BEFORE-LPP: cir.yield
+// CIR-BEFORE-LPP: }
+// CIR-BEFORE-LPP: cir.return
+
+// CIR: cir.func{{.*}} @_Z18test_cleanup_arrayv()
+// CIR: %[[S:.*]] = cir.alloca !cir.array<!rec_S x 42>, !cir.ptr<!cir.array<!rec_S x 42>>, ["s"]
+// CIR: %[[CONST42:.*]] = cir.const #cir.int<42> : !u64i
+// CIR: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[S]] : !cir.ptr<!cir.array<!rec_S x 42>>), !cir.ptr<!rec_S>
+// CIR: %[[END_PTR:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr<!rec_S>, %[[CONST42]] : !u64i), !cir.ptr<!rec_S>
+// CIR: %[[ITER:.*]] = cir.alloca !cir.ptr<!rec_S>, !cir.ptr<!cir.ptr<!rec_S>>, ["__array_idx"]
+// CIR: cir.store %[[END_PTR]], %[[ITER]] : !cir.ptr<!rec_S>, !cir.ptr<!cir.ptr<!rec_S>>
+// CIR: cir.do {
+// CIR: %[[CURRENT:.*]] = cir.load %[[ITER]] : !cir.ptr<!cir.ptr<!rec_S>>, !cir.ptr<!rec_S>
+// CIR: %[[CONST_MINUS1:.*]] = cir.const #cir.int<-1> : !s64i
+// CIR: %[[NEXT:.*]] = cir.ptr_stride(%[[CURRENT]] : !cir.ptr<!rec_S>, %[[CONST_MINUS1]] : !s64i), !cir.ptr<!rec_S>
+// CIR: cir.call @_ZN1SD1Ev(%[[NEXT]]) nothrow : (!cir.ptr<!rec_S>) -> ()
+// CIR: cir.store %[[NEXT]], %[[ITER]] : !cir.ptr<!rec_S>, !cir.ptr<!cir.ptr<!rec_S>>
+// CIR: cir.yield
+// CIR: } while {
+// CIR: %[[CURRENT2:.*]] = cir.load %[[ITER]] : !cir.ptr<!cir.ptr<!rec_S>>, !cir.ptr<!rec_S>
+// CIR: %[[CMP:.*]] = cir.cmp(ne, %[[CURRENT2]], %[[DECAY]])
+// CIR: cir.condition(%[[CMP]])
+// CIR: }
+// CIR: cir.return
+
+// LLVM: define{{.*}} void @_Z18test_cleanup_arrayv()
+// LLVM: %[[ARRAY:.*]] = alloca [42 x %struct.S]
+// LLVM: %[[START:.*]] = getelementptr %struct.S, ptr %[[ARRAY]], i32 0
+// LLVM: %[[END:.*]] = getelementptr %struct.S, ptr %[[START]], i64 42
+// LLVM: %[[ITER:.*]] = alloca ptr
+// LLVM: store ptr %[[END]], ptr %[[ITER]]
+// LLVM: br label %[[LOOP:.*]]
+// LLVM: [[COND:.*]]:
+// LLVM: %[[CURRENT_CHECK:.*]] = load ptr, ptr %[[ITER]]
+// LLVM: %[[DONE:.*]] = icmp ne ptr %[[CURRENT_CHECK]], %[[START]]
+// LLVM: br i1 %[[DONE]], label %[[LOOP]], label %[[EXIT:.*]]
+// LLVM: [[LOOP]]:
+// LLVM: %[[CURRENT:.*]] = load ptr, ptr %[[ITER]]
+// LLVM: %[[LAST:.*]] = getelementptr %struct.S, ptr %[[CURRENT]], i64 -1
+// LLVM: call void @_ZN1SD1Ev(ptr %[[LAST]])
+// LLVM: store ptr %[[LAST]], ptr %[[ITER]]
+// LLVM: br label %[[COND]]
+// LLVM: [[EXIT]]:
+// LLVM: ret void
+
+// OGCG: define{{.*}} void @_Z18test_cleanup_arrayv()
+// OGCG: %[[ARRAY:.*]] = alloca [42 x %struct.S]
+// OGCG: %[[START:.*]] = getelementptr{{.*}} %struct.S{{.*}}
+// OGCG: %[[END:.*]] = getelementptr{{.*}} %struct.S{{.*}} i64 42
+// OGCG: br label %[[LOOP:.*]]
+// OGCG: [[LOOP]]:
+// OGCG: %[[NEXT:.*]] = phi ptr [ %[[END]], %{{.*}} ], [ %[[LAST:.*]], %[[LOOP]] ]
+// OGCG: %[[LAST]] = getelementptr{{.*}} %struct.S{{.*}}, ptr %[[NEXT]], i64 -1
+// OGCG: call void @_ZN1SD1Ev(ptr{{.*}} %[[LAST]])
+// OGCG: %[[DONE:.*]] = icmp eq ptr %[[LAST]], %[[START]]
+// OGCG: br i1 %[[DONE]], label %[[EXIT:.*]], label %[[LOOP]]
+// OGCG: [[EXIT]]:
+// OGCG: ret void
+
+void test_cleanup_zero_length_array() {
+ S s[0];
+}
+
+// CIR-BEFORE-LPP: cir.func{{.*}} @_Z30test_cleanup_zero_length_arrayv()
+// CIR-BEFORE-LPP: %[[S:.*]] = cir.alloca !cir.array<!rec_S x 0>, !cir.ptr<!cir.array<!rec_S x 0>>, ["s"]
+// CIR-BEFORE-LPP-NOT: cir.array.dtor
+// CIR-BEFORE-LPP: cir.return
+
+// CIR: cir.func{{.*}} @_Z30test_cleanup_zero_length_arrayv()
+// CIR: %[[S:.*]] = cir.alloca !cir.array<!rec_S x 0>, !cir.ptr<!cir.array<!rec_S x 0>>, ["s"]
+// CIR-NOT: cir.do
+// CIR-NOT: cir.call @_ZN1SD1Ev
+// CIR: cir.return
+
+// LLVM: define{{.*}} void @_Z30test_cleanup_zero_length_arrayv()
+// LLVM: alloca [0 x %struct.S]
+// LLVM-NOT: call void @_ZN1SD1Ev
+// LLVM: ret void
+
+// OGCG: define{{.*}} void @_Z30test_cleanup_zero_length_arrayv()
+// OGCG: alloca [0 x %struct.S]
+// OGCG-NOT: call void @_ZN1SD1Ev
+// OGCG: ret void
diff --git a/clang/test/CIR/IR/array-dtor.cir b/clang/test/CIR/IR/array-dtor.cir
new file mode 100644
index 0000000000000..6d08d1639f0db
--- /dev/null
+++ b/clang/test/CIR/IR/array-dtor.cir
@@ -0,0 +1,28 @@
+// RUN: cir-opt %s | FileCheck %s
+
+!u8i = !cir.int<u, 8>
+!rec_S = !cir.record<struct "S" padded {!u8i}>
+
+module {
+ cir.func private @_ZN1SD1Ev(!cir.ptr<!rec_S>)
+ cir.func dso_local @_Z3foov() {
...
[truncated]
|
incoming argument for the current array index to initialize. | ||
|
||
```mlir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
incoming argument for the current array index to initialize. | |
```mlir | |
incoming argument for the current array index to initialize. | |
Example: | |
```mlir |
clang/lib/CIR/CodeGen/CIRGenDecl.cpp
Outdated
cir::PointerType ptrToElmType = builder.getPointerTo(cirElementType); | ||
|
||
// Emit the dtor call that will execute for every array element. | ||
builder.create<cir::ArrayDtor>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably align with mlir work of transforming from builder.create
to Op::create
At some point I will give passthrough what is upstreamed and fix it, but new stuff can be fixed on the fly directly. So this should be cir::ArrayDtor::create(builder, ...
See more info here #147168
clang/lib/CIR/CodeGen/CIRGenDecl.cpp
Outdated
// Perform the actual destruction there. | ||
destroyer(*this, curAddr, elementType); | ||
|
||
builder.create<cir::YieldOp>(loc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
builder.create<cir::YieldOp>(loc); | |
cir::YieldOp::create(builder, loc); |
clang/lib/CIR/CodeGen/CIRGenDecl.cpp
Outdated
bool checkZeroLength = true; | ||
|
||
// But if the array length is constant, we can suppress that. | ||
auto constantCount = dyn_cast<cir::ConstantOp>(length.getDefiningOp()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto constantCount = dyn_cast<cir::ConstantOp>(length.getDefiningOp()); | |
auto constantCount = length.getDefiningOp<cir::ConstantOp>(); |
void lowerArrayDtor(ArrayDtor op); | ||
void lowerArrayCtor(ArrayCtor op); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void lowerArrayDtor(ArrayDtor op); | |
void lowerArrayCtor(ArrayCtor op); | |
void lowerArrayDtor(cir::ArrayDtor op); | |
void lowerArrayCtor(cir::ArrayCtor op); |
nextElement = builder.create<cir::PtrStrideOp>( | ||
loc, eltTy, currentElement, stride); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nextElement = builder.create<cir::PtrStrideOp>( | |
loc, eltTy, currentElement, stride); | |
nextElement = cir::PtrStrideOp::create( | |
builder, loc, eltTy, currentElement, stride); |
nextElement = builder.create<cir::PtrStrideOp>( | ||
loc, eltTy, currentElement, stride); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nextElement = builder.create<cir::PtrStrideOp>( | |
loc, eltTy, currentElement, stride); | |
nextElement = cir::PtrStrideOp::create( | |
builder, loc, eltTy, currentElement, stride); |
CIRBaseBuilderTy builder(getContext()); | ||
builder.setInsertionPointAfter(op.getOperation()); | ||
|
||
mlir::Type eltTy = op->getRegion(0).getArgument(0).getType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArrayCtor asserts here !cir::MissingFeatures::vlas()
, shouldn't it be asserted also here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of nits
clang/lib/CIR/CodeGen/CIRGenDecl.cpp
Outdated
if (checkZeroLength) | ||
cgm.errorNYI("emitArrayDestroy: check for zero length"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We turn ArrayDtor
into a loop in LoweringPrepare. I don't think we ever need to insert a branch here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, though that means we will need to check for that condition in LowerPrepare when we implement support for VLAs.
return; | ||
checkZeroLength = false; | ||
} else { | ||
cgm.errorNYI("emitDestroy: variable length array"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a cir::MissingFeature::vla()
here?
|
||
return destroyer(*this, addr, type); | ||
// If the array destroy didn't use the length op, we can erase it. | ||
if (constantCount.use_empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simplify the control flow a little if you invert the condition up there. Something like:
if(!constantCount) {
assert(!cir::MissingFeature::vlas());
cgm.errorNYI("emitDestroy: variable length array");
return;
}
auto constIntAttr = mlir::dyn_cast<cir::IntAttr>(constantCount.getValue());
// ...and if it's constant zero, we can just skip the entire thing.
if (constIntAttr && constIntAttr.getUInt() == 0)
return;
checkZeroLength = false;
mlir::Value begin = addr.getPointer();
mlir::Value end; // This will be used for future non-constant counts.
emitArrayDestroy(begin, end, type, elementAlign, destroyer, checkZeroLength);
if (constantCount.use_empty())
constantCount.erase();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will probably need to switch back to the if-else structure when we support VLAs, but for now your suggestion looks better. I'll make the change.
@@ -249,6 +270,8 @@ void LoweringPreparePass::runOnOp(mlir::Operation *op) { | |||
lowerCastOp(cast); | |||
else if (auto unary = mlir::dyn_cast<cir::UnaryOp>(op)) | |||
lowerUnaryOp(unary); | |||
else if (auto arrayDtor = dyn_cast<ArrayDtor>(op)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should try to keep this if/else cascade in lexicographical order.
mlir::Value stride = builder.getSignedInt(loc, -1, sizeTypeSize); | ||
nextElement = builder.create<cir::PtrStrideOp>( | ||
loc, eltTy, currentElement, stride); | ||
ctorCall->moveAfter(nextElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your loop is correct. But I also think if you initialize stop with end - 1
you don't have to use moveAfter
here and can share more code between the ctor and dtor case (the zero case is handled elsewhere so that would be safe to do). But that's more of a comment and less of a request to change your patch. I don't care either way :)
let description = [{ | ||
Destroy each array element using the same C++ destructor. This | ||
operation has one region, with one single block. The block has an | ||
incoming argument for the current array index to initialize. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
incoming argument for the current array index to initialize. | |
incoming argument for the current array element to destruct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Straightforward change, nothing to add here after existing comments, LGTM.
This adds support for array cleanups, including the ArrayDtor op.