Skip to content

[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

Merged
merged 2 commits into from
Jul 29, 2025
Merged

Conversation

andykaylor
Copy link
Contributor

This adds support for array cleanups, including the ArrayDtor op.

This adds initial support for array cleanups, including the ArrayDtor op.
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Jul 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 24, 2025

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This 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:

  • (modified) clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h (+6)
  • (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+20-3)
  • (modified) clang/lib/CIR/CodeGen/CIRGenDecl.cpp (+66-3)
  • (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+4)
  • (modified) clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp (+45-21)
  • (modified) clang/test/CIR/CodeGen/array-ctor.cpp (+1-1)
  • (added) clang/test/CIR/CodeGen/array-dtor.cpp (+104)
  • (added) clang/test/CIR/IR/array-dtor.cir (+28)
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]

Comment on lines 2280 to 2282
incoming argument for the current array index to initialize.

```mlir
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
incoming argument for the current array index to initialize.
```mlir
incoming argument for the current array index to initialize.
Example:
```mlir

cir::PointerType ptrToElmType = builder.getPointerTo(cirElementType);

// Emit the dtor call that will execute for every array element.
builder.create<cir::ArrayDtor>(
Copy link
Contributor

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

// Perform the actual destruction there.
destroyer(*this, curAddr, elementType);

builder.create<cir::YieldOp>(loc);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
builder.create<cir::YieldOp>(loc);
cir::YieldOp::create(builder, loc);

bool checkZeroLength = true;

// But if the array length is constant, we can suppress that.
auto constantCount = dyn_cast<cir::ConstantOp>(length.getDefiningOp());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto constantCount = dyn_cast<cir::ConstantOp>(length.getDefiningOp());
auto constantCount = length.getDefiningOp<cir::ConstantOp>();

Comment on lines 32 to 33
void lowerArrayDtor(ArrayDtor op);
void lowerArrayCtor(ArrayCtor op);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void lowerArrayDtor(ArrayDtor op);
void lowerArrayCtor(ArrayCtor op);
void lowerArrayDtor(cir::ArrayDtor op);
void lowerArrayCtor(cir::ArrayCtor op);

Comment on lines 224 to 225
nextElement = builder.create<cir::PtrStrideOp>(
loc, eltTy, currentElement, stride);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
nextElement = builder.create<cir::PtrStrideOp>(
loc, eltTy, currentElement, stride);
nextElement = cir::PtrStrideOp::create(
builder, loc, eltTy, currentElement, stride);

Comment on lines 228 to 229
nextElement = builder.create<cir::PtrStrideOp>(
loc, eltTy, currentElement, stride);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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();
Copy link
Contributor

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?

Copy link
Contributor

@mmha mmha left a 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

Comment on lines 665 to 666
if (checkZeroLength)
cgm.errorNYI("emitArrayDestroy: check for zero length");
Copy link
Contributor

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.

Copy link
Contributor Author

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");
Copy link
Contributor

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())
Copy link
Contributor

@mmha mmha Jul 25, 2025

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();

Copy link
Contributor Author

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))
Copy link
Contributor

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);
Copy link
Contributor

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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
incoming argument for the current array index to initialize.
incoming argument for the current array element to destruct.

Copy link
Member

@bcardosolopes bcardosolopes left a 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.

@andykaylor andykaylor merged commit 88620ae into llvm:main Jul 29, 2025
9 checks passed
@andykaylor andykaylor deleted the cir-array-cleanup branch July 29, 2025 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants