[clang] [clang-tools-extra] [codegen] Emit missing cleanups when an expression contains a branch (PR #80698)

Utkarsh Saxena via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 13 03:23:53 PST 2024


================
@@ -592,10 +590,14 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
       // observed to be unnecessary.
       if (endOfInit.isValid()) Builder.CreateStore(element, endOfInit);
     }
-
-    LValue elementLV = CGF.MakeAddrLValue(
-        Address(element, llvmElementType, elementAlign), elementType);
+    Address address = Address(element, llvmElementType, elementAlign);
+    LValue elementLV = CGF.MakeAddrLValue(address, elementType);
     EmitInitializationToLValue(Args[i], elementLV);
+    // Schedule to emit element cleanup if we see a branch in the array
+    // initialisation expression.
+    if (CGF.needsBranchCleanup(dtorKind))
+      CGF.pushDestroy(BranchInExprCleanup, address, elementType,
----------------
usx95 wrote:

> Element wise cleanup Vs iteration-style cleanup.

I had considered using `IrregularPartialArrayDestroy` instead of element wise destroy. The catch is that this cleanup expects allocated `array.init.end` to store the current end pointer. This is updated as we iterate over and create the elements.
I do not think we should add these "stores" overhead, especially with `-fno-exceptions` with branches in expr.

What we could be doing instead is just considering the current array index and using that for the array end address.
But that seems to warned against in the comments: 
```
      // In principle we could tell the Cleanup where we are more
      // directly, but the control flow can get so varied here that it
      // would actually be quite complex.  Therefore we go through an
      // alloca.
```
I do not understand the "complex" flows where this could go wrong.


https://github.com/llvm/llvm-project/pull/80698


More information about the cfe-commits mailing list