Coro destruct temporary ehcleanup

clang/lib/CodeGen/Address.h

@@ -17,6 +17,7 @@
 #include "clang/AST/CharUnits.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/Support/MathExtras.h"
 
 namespace clang {
@@ -53,6 +54,10 @@ class Address {
     return PointerAndKnownNonNull.getPointer();
   }
 
+  llvm::AllocaInst *getAllocaInst() const {
+    return llvm::dyn_cast<llvm::AllocaInst>(getPointer());
+  }
+
   /// Return the type of the pointer value.
   llvm::PointerType *getType() const {
     return llvm::cast<llvm::PointerType>(getPointer()->getType());

clang/lib/CodeGen/CGClass.cpp

@@ -671,7 +671,7 @@ static void EmitMemberInitializer(CodeGenFunction &CGF,
       // Ensure that we destroy the objects if an exception is thrown later in
       // the constructor.
       QualType::DestructionKind dtorKind = FieldType.isDestructedType();
-      if (CGF.needsEHCleanup(dtorKind))
+      if (dtorKind)
         CGF.pushEHDestroy(dtorKind, LHS.getAddress(CGF), FieldType);
       return;
     }
@@ -710,7 +710,7 @@ void CodeGenFunction::EmitInitializerForField(FieldDecl *Field, LValue LHS,
   // Ensure that we destroy this object if an exception is thrown
   // later in the constructor.
   QualType::DestructionKind dtorKind = FieldType.isDestructedType();
-  if (needsEHCleanup(dtorKind))
+  if (dtorKind)
     pushEHDestroy(dtorKind, LHS.getAddress(*this), FieldType);
 }
 

clang/lib/CodeGen/CGCleanup.cpp

@@ -18,6 +18,8 @@
 
 #include "CGCleanup.h"
 #include "CodeGenFunction.h"
+#include "EHScopeStack.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
@@ -294,18 +296,22 @@ void EHScopeStack::popNullFixups() {
     BranchFixups.pop_back();
 }
 
-Address CodeGenFunction::createCleanupActiveFlag() {
+Address CodeGenFunction::createCleanupActiveFlag(EHCleanupScope *TopCleanup) {
   // Create a variable to decide whether the cleanup needs to be run.
   Address active = CreateTempAllocaWithoutCast(
       Builder.getInt1Ty(), CharUnits::One(), "cleanup.cond");
-
+  if (TopCleanup)
+    TopCleanup->AddAuxInst(active.getAllocaInst());
   // Initialize it to false at a site that's guaranteed to be run
   // before each evaluation.
-  setBeforeOutermostConditional(Builder.getFalse(), active);
+  auto *store1 = setBeforeOutermostConditional(Builder.getFalse(), active);
+  if (TopCleanup)
+    TopCleanup->AddAuxInst(store1);
 
   // Initialize it to true at the current location.
-  Builder.CreateStore(Builder.getTrue(), active);
-
+  auto *store = Builder.CreateStore(Builder.getTrue(), active);
+  if (TopCleanup)
+    TopCleanup->AddAuxInst(store);
   return active;
 }
 
@@ -488,37 +494,37 @@ void CodeGenFunction::PopCleanupBlocks(
   }
 }
 
-/// Pops cleanup blocks until the given savepoint is reached, then add the
-/// cleanups from the given savepoint in the lifetime-extended cleanups stack.
-void CodeGenFunction::PopCleanupBlocks(
-    EHScopeStack::stable_iterator Old, size_t OldLifetimeExtendedSize,
-    std::initializer_list<llvm::Value **> ValuesToReload) {
-  PopCleanupBlocks(Old, ValuesToReload);
-
-  // Move our deferred cleanups onto the EH stack.
-  for (size_t I = OldLifetimeExtendedSize,
-              E = LifetimeExtendedCleanupStack.size(); I != E; /**/) {
+void CodeGenFunction::AddDeferredCleanups(DeferredCleanupStack &Cleanups,
+                                          size_t OldSize) {
+  for (size_t I = OldSize, E = Cleanups.size(); I != E;) {
     // Alignment should be guaranteed by the vptrs in the individual cleanups.
-    assert((I % alignof(LifetimeExtendedCleanupHeader) == 0) &&
+    assert((I % alignof(DeferredCleanupHeader) == 0) &&
            "misaligned cleanup stack entry");
 
-    LifetimeExtendedCleanupHeader &Header =
-        reinterpret_cast<LifetimeExtendedCleanupHeader&>(
-            LifetimeExtendedCleanupStack[I]);
+    DeferredCleanupHeader &Header =
+        reinterpret_cast<DeferredCleanupHeader &>(Cleanups[I]);
     I += sizeof(Header);
 
-    EHStack.pushCopyOfCleanup(Header.getKind(),
-                              &LifetimeExtendedCleanupStack[I],
-                              Header.getSize());
+    EHStack.pushCopyOfCleanup(Header.getKind(), &Cleanups[I], Header.getSize());
     I += Header.getSize();
 
     if (Header.isConditional()) {
-      Address ActiveFlag =
-          reinterpret_cast<Address &>(LifetimeExtendedCleanupStack[I]);
+      Address ActiveFlag = reinterpret_cast<Address &>(Cleanups[I]);
       initFullExprCleanupWithFlag(ActiveFlag);
       I += sizeof(ActiveFlag);
     }
   }
+}
+
+/// Pops cleanup blocks until the given savepoint is reached, then add the
+/// cleanups from the given savepoint in the lifetime-extended cleanups stack.
+void CodeGenFunction::PopCleanupBlocks(
+    EHScopeStack::stable_iterator Old, size_t OldLifetimeExtendedSize,
+    std::initializer_list<llvm::Value **> ValuesToReload) {
+  PopCleanupBlocks(Old, ValuesToReload);
+
+  // Move our deferred lifetime-extended cleanups onto the EH stack.
+  AddDeferredCleanups(LifetimeExtendedCleanupStack, OldLifetimeExtendedSize);
   LifetimeExtendedCleanupStack.resize(OldLifetimeExtendedSize);
 }
 
@@ -672,15 +678,16 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
   Address NormalActiveFlag =
     Scope.shouldTestFlagInNormalCleanup() ? Scope.getActiveFlag()
                                           : Address::invalid();
-  Address EHActiveFlag =
-    Scope.shouldTestFlagInEHCleanup() ? Scope.getActiveFlag()
-                                      : Address::invalid();
-
+  Address EHActiveFlag = Scope.shouldTestFlagInEHCleanup()
+                             ? Scope.getActiveFlag()
+                             : Address::invalid();
   // Check whether we need an EH cleanup.  This is only true if we've
   // generated a lazy EH cleanup block.
   llvm::BasicBlock *EHEntry = Scope.getCachedEHDispatchBlock();
   assert(Scope.hasEHBranches() == (EHEntry != nullptr));
   bool RequiresEHCleanup = (EHEntry != nullptr);
+  bool EmitEHCleanup =
+      RequiresEHCleanup && (EHActiveFlag.isValid() || IsActive);
   EHScopeStack::stable_iterator EHParent = Scope.getEnclosingEHScope();
 
   // Check the three conditions which might require a normal cleanup:
@@ -792,6 +799,8 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
         EmitSehCppScopeEnd();
     }
     destroyOptimisticNormalEntry(*this, Scope);
+    if (EmitEHCleanup)
+      Scope.MarkEmitted();
     EHStack.popCleanup();
   } else {
     // If we have a fallthrough and no other need for the cleanup,
@@ -808,6 +817,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
       }
 
       destroyOptimisticNormalEntry(*this, Scope);
+      Scope.MarkEmitted();
       EHStack.popCleanup();
 
       EmitCleanup(*this, Fn, cleanupFlags, NormalActiveFlag);
@@ -944,6 +954,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
       }
 
       // IV.  Pop the cleanup and emit it.
+      Scope.MarkEmitted();
       EHStack.popCleanup();
       assert(EHStack.hasNormalCleanups() == HasEnclosingCleanups);
 
@@ -1047,7 +1058,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
 
     // We only actually emit the cleanup code if the cleanup is either
     // active or was used before it was deactivated.
-    if (EHActiveFlag.isValid() || IsActive) {
+    if (EmitEHCleanup) {
       cleanupFlags.setIsForEHCleanup();
       EmitCleanup(*this, Fn, cleanupFlags, EHActiveFlag);
     }
@@ -1089,6 +1100,30 @@ bool CodeGenFunction::isObviouslyBranchWithoutCleanups(JumpDest Dest) const {
   return false;
 }
 
+void CodeGenFunction::EmitEHCleanupsForBranch(JumpDest Dest) {
+  if (!Dest.isValid())
+    return;
+  EHScopeStack::stable_iterator TopCleanup = EHStack.getInnermostEHScope();
+  std::vector<EHScopeStack::stable_iterator> Cleanups;
+  while (TopCleanup != EHStack.stable_end() &&
+         Dest.getEHDepth().strictlyEncloses(TopCleanup)) {
+    if (EHStack.find(TopCleanup)->getKind() != EHScope::Cleanup)
+      break;
+    EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.find(TopCleanup));
+    if (!Scope.isNormalCleanup() && !Scope.shouldSkipBranchInExpr())
+      Cleanups.push_back(TopCleanup);
+    TopCleanup = Scope.getEnclosingEHScope();
+  }
+  for (size_t I = Cleanups.size(); I > 0; I--) {
+    EHCleanupScope &Scope =
+        cast<EHCleanupScope>(*EHStack.find(Cleanups[I - 1]));
+    assert(Scope.isEHCleanup());
+    EHStack.pushCopyOfCleanup(NormalCleanup, Scope.getCleanup(),
+                              Scope.getCleanupSize());
+    cast<EHCleanupScope>(*EHStack.begin())
+        .SetExternalAuxInsts(&Scope.getAuxillaryInstructions());
+  }
+}
 
 /// Terminate the current block by emitting a branch which might leave
 /// the current cleanup-protected scope.  The target scope may not yet
@@ -1102,6 +1137,12 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
   if (!HaveInsertPoint())
     return;
 
+  // If we are emitting a branch in a partial expression, add deferred cleanups
+  // to EHStack, which would otherwise have only been emitted after the full
+  // expression.
+  RunCleanupsScope BranchInExprCleanups(*this);
+  if (Dest.isValid())
+    EmitEHCleanupsForBranch(Dest);
   // Create the branch.
   llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock());
 

clang/lib/CodeGen/CGCleanup.h

@@ -16,8 +16,10 @@
 #include "EHScopeStack.h"
 
 #include "Address.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/Instruction.h"
 
 namespace llvm {
 class BasicBlock;
@@ -81,6 +83,8 @@ class EHScope {
     /// Whether the EH cleanup should test the activation flag.
     unsigned TestFlagInEHCleanup : 1;
 
+    unsigned SkipBranchInExpr : 1;
+
     /// The amount of extra storage needed by the Cleanup.
     /// Always a multiple of the scope-stack alignment.
     unsigned CleanupSize : 12;
@@ -256,6 +260,30 @@ class alignas(8) EHCleanupScope : public EHScope {
       BranchAfters;
   };
   mutable struct ExtInfo *ExtInfo;
+  // Erases unused auxillary instructions for the cleanup.
+  // Cleanups should mark these instructions 'used' if the cleanup is
+  // emitted, otherwise these instructions would be erased.
+  struct AuxillaryInsts {
+    SmallVector<llvm::Instruction *, 1> AuxInsts;
+    bool used = false;
+
+    // Records a potentially unused instruction to be erased later.
+    void Add(llvm::Instruction *Inst) { AuxInsts.push_back(Inst); }
+
+    // Mark all recorded instructions as used. These will not be erased later.
+    void MarkUsed() {
+      used = true;
+      AuxInsts.clear();
+    }
+    ~AuxillaryInsts() {
+      if (used)
+        return;
+      for (auto *Inst : llvm::reverse(AuxInsts))
+        Inst->eraseFromParent();
+    }
+  };
+  mutable struct AuxillaryInsts *AuxInsts;
+  bool AuxInstsIsOwned = true;
 
   /// The number of fixups required by enclosing scopes (not including
   /// this one).  If this is the top cleanup scope, all the fixups
@@ -279,6 +307,19 @@ class alignas(8) EHCleanupScope : public EHScope {
     return sizeof(EHCleanupScope) + Size;
   }
 
+  AuxillaryInsts &getAuxillaryInstructions() {
+    if (!AuxInsts) {
+      assert(AuxInstsIsOwned);
+      AuxInsts = new struct AuxillaryInsts();
+    }
+    return *AuxInsts;
+  }
+
+  void SetExternalAuxInsts(AuxillaryInsts *NewAuxInsts) {
+    AuxInsts = NewAuxInsts;
+    AuxInstsIsOwned = false;
+  }
+
   size_t getAllocatedSize() const {
     return sizeof(EHCleanupScope) + CleanupBits.CleanupSize;
   }
@@ -289,20 +330,24 @@ class alignas(8) EHCleanupScope : public EHScope {
                  EHScopeStack::stable_iterator enclosingEH)
       : EHScope(EHScope::Cleanup, enclosingEH),
         EnclosingNormal(enclosingNormal), NormalBlock(nullptr),
-        ActiveFlag(Address::invalid()), ExtInfo(nullptr),
+        ActiveFlag(Address::invalid()), ExtInfo(nullptr), AuxInsts(nullptr),
         FixupDepth(fixupDepth) {
     CleanupBits.IsNormalCleanup = isNormal;
     CleanupBits.IsEHCleanup = isEH;
     CleanupBits.IsActive = true;
     CleanupBits.IsLifetimeMarker = false;
     CleanupBits.TestFlagInNormalCleanup = false;
     CleanupBits.TestFlagInEHCleanup = false;
+    CleanupBits.SkipBranchInExpr = false;
     CleanupBits.CleanupSize = cleanupSize;
+    AuxInstsIsOwned = true;
 
     assert(CleanupBits.CleanupSize == cleanupSize && "cleanup size overflow");
   }
 
   void Destroy() {
+    if (AuxInstsIsOwned)
+      delete AuxInsts;
     delete ExtInfo;
   }
   // Objects of EHCleanupScope are not destructed. Use Destroy().
@@ -320,6 +365,9 @@ class alignas(8) EHCleanupScope : public EHScope {
   bool isLifetimeMarker() const { return CleanupBits.IsLifetimeMarker; }
   void setLifetimeMarker() { CleanupBits.IsLifetimeMarker = true; }
 
+  void setShouldSkipBranchInExpr() { CleanupBits.SkipBranchInExpr = true; }
+  bool shouldSkipBranchInExpr() const { return CleanupBits.SkipBranchInExpr; }
+
   bool hasActiveFlag() const { return ActiveFlag.isValid(); }
   Address getActiveFlag() const {
     return ActiveFlag;
@@ -348,6 +396,12 @@ class alignas(8) EHCleanupScope : public EHScope {
     return EnclosingNormal;
   }
 
+  void AddAuxInst(llvm::Instruction *Inst) {
+    getAuxillaryInstructions().Add(Inst);
+  }
+
+  void MarkEmitted() { getAuxillaryInstructions().MarkUsed(); }
+
   size_t getCleanupSize() const { return CleanupBits.CleanupSize; }
   void *getCleanupBuffer() { return this + 1; }
 

clang/lib/CodeGen/CGCoroutine.cpp

@@ -12,9 +12,10 @@
 
 #include "CGCleanup.h"
 #include "CodeGenFunction.h"
-#include "llvm/ADT/ScopeExit.h"
+#include "EHScopeStack.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -743,6 +744,11 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
     GroManager.EmitGroInit();
 
     EHStack.pushCleanup<CallCoroEnd>(EHCleanup);
+    {
+      EHCleanupScope &Scope =
+          cast<EHCleanupScope>(*EHStack.find(EHStack.stable_begin()));
+      Scope.setShouldSkipBranchInExpr();
+    }
 
     CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
     CurCoro.Data->ExceptionHandler = S.getExceptionHandler();