Revert "[codegen] Emit missing cleanups for stmt-expr and coro suspensions" and related commits

clang/lib/CodeGen/CGCall.cpp

@@ -4694,11 +4694,11 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
     AggValueSlot Slot = args.isUsingInAlloca()
         ? createPlaceholderSlot(*this, type) : CreateAggTemp(type, "agg.tmp");
 
-    bool DestroyedInCallee = true, NeedsCleanup = true;
+    bool DestroyedInCallee = true, NeedsEHCleanup = true;
     if (const auto *RD = type->getAsCXXRecordDecl())
       DestroyedInCallee = RD->hasNonTrivialDestructor();
     else
-      NeedsCleanup = type.isDestructedType();
+      NeedsEHCleanup = needsEHCleanup(type.isDestructedType());
 
     if (DestroyedInCallee)
       Slot.setExternallyDestructed();
@@ -4707,15 +4707,14 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
     RValue RV = Slot.asRValue();
     args.add(RV, type);
 
-    if (DestroyedInCallee && NeedsCleanup) {
+    if (DestroyedInCallee && NeedsEHCleanup) {
       // Create a no-op GEP between the placeholder and the cleanup so we can
       // RAUW it successfully.  It also serves as a marker of the first
       // instruction where the cleanup is active.
-      pushFullExprCleanup<DestroyUnpassedArg>(NormalAndEHCleanup,
-                                              Slot.getAddress(), type);
+      pushFullExprCleanup<DestroyUnpassedArg>(EHCleanup, Slot.getAddress(),
+                                              type);
       // This unreachable is a temporary marker which will be removed later.
-      llvm::Instruction *IsActive =
-          Builder.CreateFlagLoad(llvm::Constant::getNullValue(Int8PtrTy));
+      llvm::Instruction *IsActive = Builder.CreateUnreachable();
       args.addArgCleanupDeactivation(EHStack.stable_begin(), IsActive);
     }
     return;

clang/lib/CodeGen/CGCleanup.cpp

@@ -634,19 +634,12 @@ static void destroyOptimisticNormalEntry(CodeGenFunction &CGF,
 /// Pops a cleanup block.  If the block includes a normal cleanup, the
 /// current insertion point is threaded through the cleanup, as are
 /// any branch fixups on the cleanup.
-void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
-                                      bool ForDeactivation) {
+void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
   assert(!EHStack.empty() && "cleanup stack is empty!");
   assert(isa<EHCleanupScope>(*EHStack.begin()) && "top not a cleanup!");
   EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.begin());
   assert(Scope.getFixupDepth() <= EHStack.getNumBranchFixups());
 
-  // If we are deactivating a normal cleanup, we need to pretend that the
-  // fallthrough is unreachable. We restore this IP before returning.
-  CGBuilderTy::InsertPoint NormalDeactivateOrigIP;
-  if (ForDeactivation && (Scope.isNormalCleanup() || !getLangOpts().EHAsynch)) {
-    NormalDeactivateOrigIP = Builder.saveAndClearIP();
-  }
   // Remember activation information.
   bool IsActive = Scope.isActive();
   Address NormalActiveFlag =
@@ -674,8 +667,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
 
   // - whether there's a fallthrough
   llvm::BasicBlock *FallthroughSource = Builder.GetInsertBlock();
-  bool HasFallthrough =
-      FallthroughSource != nullptr && (IsActive || HasExistingBranches);
+  bool HasFallthrough = (FallthroughSource != nullptr && IsActive);
 
   // Branch-through fall-throughs leave the insertion point set to the
   // end of the last cleanup, which points to the current scope.  The
@@ -700,11 +692,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
 
   // If we have a prebranched fallthrough into an inactive normal
   // cleanup, rewrite it so that it leads to the appropriate place.
-  if (Scope.isNormalCleanup() && HasPrebranchedFallthrough &&
-      !RequiresNormalCleanup) {
-    // FIXME: Come up with a program which would need forwarding prebranched
-    // fallthrough and add tests. Otherwise delete this and assert against it.
-    assert(!IsActive);
+  if (Scope.isNormalCleanup() && HasPrebranchedFallthrough && !IsActive) {
     llvm::BasicBlock *prebranchDest;
 
     // If the prebranch is semantically branching through the next
@@ -736,8 +724,6 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
     EHStack.popCleanup(); // safe because there are no fixups
     assert(EHStack.getNumBranchFixups() == 0 ||
            EHStack.hasNormalCleanups());
-    if (NormalDeactivateOrigIP.isSet())
-      Builder.restoreIP(NormalDeactivateOrigIP);
     return;
   }
 
@@ -774,19 +760,11 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
   if (!RequiresNormalCleanup) {
     // Mark CPP scope end for passed-by-value Arg temp
     //   per Windows ABI which is "normally" Cleanup in callee
-    if (IsEHa && getInvokeDest()) {
-      // If we are deactivating a normal cleanup then we don't have a
-      // fallthrough. Restore original IP to emit CPP scope ends in the correct
-      // block.
-      if (NormalDeactivateOrigIP.isSet())
-        Builder.restoreIP(NormalDeactivateOrigIP);
-      if (Personality.isMSVCXXPersonality() && Builder.GetInsertBlock())
+    if (IsEHa && getInvokeDest() && Builder.GetInsertBlock()) {
+      if (Personality.isMSVCXXPersonality())
         EmitSehCppScopeEnd();
-      if (NormalDeactivateOrigIP.isSet())
-        NormalDeactivateOrigIP = Builder.saveAndClearIP();
     }
     destroyOptimisticNormalEntry(*this, Scope);
-    Scope.MarkEmitted();
     EHStack.popCleanup();
   } else {
     // If we have a fallthrough and no other need for the cleanup,
@@ -803,7 +781,6 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
       }
 
       destroyOptimisticNormalEntry(*this, Scope);
-      Scope.MarkEmitted();
       EHStack.popCleanup();
 
       EmitCleanup(*this, Fn, cleanupFlags, NormalActiveFlag);
@@ -939,7 +916,6 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
       }
 
       // IV.  Pop the cleanup and emit it.
-      Scope.MarkEmitted();
       EHStack.popCleanup();
       assert(EHStack.hasNormalCleanups() == HasEnclosingCleanups);
 
@@ -1008,8 +984,6 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
     }
   }
 
-  if (NormalDeactivateOrigIP.isSet())
-    Builder.restoreIP(NormalDeactivateOrigIP);
   assert(EHStack.hasNormalCleanups() || EHStack.getNumBranchFixups() == 0);
 
   // Emit the EH cleanup if required.
@@ -1299,8 +1273,17 @@ void CodeGenFunction::DeactivateCleanupBlock(EHScopeStack::stable_iterator C,
   // to the current RunCleanupsScope.
   if (C == EHStack.stable_begin() &&
       CurrentCleanupScopeDepth.strictlyEncloses(C)) {
-    PopCleanupBlock(/*FallthroughIsBranchThrough=*/false,
-                    /*ForDeactivation=*/true);
+    // Per comment below, checking EHAsynch is not really necessary
+    // it's there to assure zero-impact w/o EHAsynch option
+    if (!Scope.isNormalCleanup() && getLangOpts().EHAsynch) {
+      PopCleanupBlock();
+    } else {
+      // If it's a normal cleanup, we need to pretend that the
+      // fallthrough is unreachable.
+      CGBuilderTy::InsertPoint SavedIP = Builder.saveAndClearIP();
+      PopCleanupBlock();
+      Builder.restoreIP(SavedIP);
+    }
     return;
   }
 

clang/lib/CodeGen/CGCleanup.h

@@ -16,11 +16,8 @@
 #include "EHScopeStack.h"
 
 #include "Address.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/IR/Instruction.h"
 
 namespace llvm {
 class BasicBlock;
@@ -269,51 +266,6 @@ class alignas(8) EHCleanupScope : public EHScope {
   };
   mutable struct ExtInfo *ExtInfo;
 
-  /// Erases auxillary allocas and their usages for an unused cleanup.
-  /// Cleanups should mark these allocas as 'used' if the cleanup is
-  /// emitted, otherwise these instructions would be erased.
-  struct AuxillaryAllocas {
-    SmallVector<llvm::Instruction *, 1> AuxAllocas;
-    bool used = false;
-
-    // Records a potentially unused instruction to be erased later.
-    void Add(llvm::AllocaInst *Alloca) { AuxAllocas.push_back(Alloca); }
-
-    // Mark all recorded instructions as used. These will not be erased later.
-    void MarkUsed() {
-      used = true;
-      AuxAllocas.clear();
-    }
-
-    ~AuxillaryAllocas() {
-      if (used)
-        return;
-      llvm::SetVector<llvm::Instruction *> Uses;
-      for (auto *Inst : llvm::reverse(AuxAllocas))
-        CollectUses(Inst, Uses);
-      // Delete uses in the reverse order of insertion.
-      for (auto *I : llvm::reverse(Uses))
-        I->eraseFromParent();
-    }
-
-  private:
-    void CollectUses(llvm::Instruction *I,
-                     llvm::SetVector<llvm::Instruction *> &Uses) {
-      if (!I || !Uses.insert(I))
-        return;
-      for (auto *User : I->users())
-        CollectUses(cast<llvm::Instruction>(User), Uses);
-    }
-  };
-  mutable struct AuxillaryAllocas *AuxAllocas;
-
-  AuxillaryAllocas &getAuxillaryAllocas() {
-    if (!AuxAllocas) {
-      AuxAllocas = new struct AuxillaryAllocas();
-    }
-    return *AuxAllocas;
-  }
-
   /// The number of fixups required by enclosing scopes (not including
   /// this one).  If this is the top cleanup scope, all the fixups
   /// from this index onwards belong to this scope.
@@ -346,7 +298,7 @@ class alignas(8) EHCleanupScope : public EHScope {
                  EHScopeStack::stable_iterator enclosingEH)
       : EHScope(EHScope::Cleanup, enclosingEH),
         EnclosingNormal(enclosingNormal), NormalBlock(nullptr),
-        ActiveFlag(Address::invalid()), ExtInfo(nullptr), AuxAllocas(nullptr),
+        ActiveFlag(Address::invalid()), ExtInfo(nullptr),
         FixupDepth(fixupDepth) {
     CleanupBits.IsNormalCleanup = isNormal;
     CleanupBits.IsEHCleanup = isEH;
@@ -360,15 +312,8 @@ class alignas(8) EHCleanupScope : public EHScope {
   }
 
   void Destroy() {
-    if (AuxAllocas)
-      delete AuxAllocas;
     delete ExtInfo;
   }
-  void AddAuxAllocas(llvm::SmallVector<llvm::AllocaInst *> Allocas) {
-    for (auto *Alloca : Allocas)
-      getAuxillaryAllocas().Add(Alloca);
-  }
-  void MarkEmitted() { getAuxillaryAllocas().MarkUsed(); }
   // Objects of EHCleanupScope are not destructed. Use Destroy().
   ~EHCleanupScope() = delete;
 

clang/lib/CodeGen/CGDecl.cpp

@@ -19,7 +19,6 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
-#include "EHScopeStack.h"
 #include "PatternInit.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
@@ -2202,27 +2201,6 @@ void CodeGenFunction::pushDestroy(CleanupKind cleanupKind, Address addr,
                                      destroyer, useEHCleanupForArray);
 }
 
-// Pushes a destroy and defers its deactivation until its
-// CleanupDeactivationScope is exited.
-void CodeGenFunction::pushDestroyAndDeferDeactivation(
-    QualType::DestructionKind dtorKind, Address addr, QualType type) {
-  assert(dtorKind && "cannot push destructor for trivial type");
-
-  CleanupKind cleanupKind = getCleanupKind(dtorKind);
-  pushDestroyAndDeferDeactivation(
-      cleanupKind, addr, type, getDestroyer(dtorKind), cleanupKind & EHCleanup);
-}
-
-void CodeGenFunction::pushDestroyAndDeferDeactivation(
-    CleanupKind cleanupKind, Address addr, QualType type, Destroyer *destroyer,
-    bool useEHCleanupForArray) {
-  llvm::Instruction *DominatingIP =
-      Builder.CreateFlagLoad(llvm::Constant::getNullValue(Int8PtrTy));
-  pushDestroy(cleanupKind, addr, type, destroyer, useEHCleanupForArray);
-  DeferredDeactivationCleanupStack.push_back(
-      {EHStack.stable_begin(), DominatingIP});
-}
-
 void CodeGenFunction::pushStackRestore(CleanupKind Kind, Address SPMem) {
   EHStack.pushCleanup<CallStackRestore>(Kind, SPMem);
 }
@@ -2239,19 +2217,16 @@ void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
   // If we're not in a conditional branch, we don't need to bother generating a
   // conditional cleanup.
   if (!isInConditionalBranch()) {
+    // Push an EH-only cleanup for the object now.
     // FIXME: When popping normal cleanups, we need to keep this EH cleanup
     // around in case a temporary's destructor throws an exception.
+    if (cleanupKind & EHCleanup)
+      EHStack.pushCleanup<DestroyObject>(
+          static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), addr, type,
+          destroyer, useEHCleanupForArray);
 
-    // Add the cleanup to the EHStack. After the full-expr, this would be
-    // deactivated before being popped from the stack.
-    pushDestroyAndDeferDeactivation(cleanupKind, addr, type, destroyer,
-                                    useEHCleanupForArray);
-
-    // Since this is lifetime-extended, push it once again to the EHStack after
-    // the full expression.
     return pushCleanupAfterFullExprWithActiveFlag<DestroyObject>(
-        cleanupKind, Address::invalid(), addr, type, destroyer,
-        useEHCleanupForArray);
+        cleanupKind, Address::invalid(), addr, type, destroyer, useEHCleanupForArray);
   }
 
   // Otherwise, we should only destroy the object if it's been initialized.
@@ -2266,12 +2241,13 @@ void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
   Address ActiveFlag = createCleanupActiveFlag();
   SavedType SavedAddr = saveValueInCond(addr);
 
-  pushCleanupAndDeferDeactivation<ConditionalCleanupType>(
-      cleanupKind, SavedAddr, type, destroyer, useEHCleanupForArray);
-  initFullExprCleanupWithFlag(ActiveFlag);
+  if (cleanupKind & EHCleanup) {
+    EHStack.pushCleanup<ConditionalCleanupType>(
+        static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), SavedAddr, type,
+        destroyer, useEHCleanupForArray);
+    initFullExprCleanupWithFlag(ActiveFlag);
+  }
 
-  // Since this is lifetime-extended, push it once again to the EHStack after
-  // the full expression.
   pushCleanupAfterFullExprWithActiveFlag<ConditionalCleanupType>(
       cleanupKind, ActiveFlag, SavedAddr, type, destroyer,
       useEHCleanupForArray);
@@ -2466,9 +2442,9 @@ namespace {
   };
 } // end anonymous namespace
 
-/// pushIrregularPartialArrayCleanup - Push a NormalAndEHCleanup to
-/// destroy already-constructed elements of the given array.  The cleanup may be
-/// popped with DeactivateCleanupBlock or PopCleanupBlock.
+/// pushIrregularPartialArrayCleanup - Push an EH cleanup to destroy
+/// already-constructed elements of the given array.  The cleanup
+/// may be popped with DeactivateCleanupBlock or PopCleanupBlock.
 ///
 /// \param elementType - the immediate element type of the array;
 ///   possibly still an array type
@@ -2477,9 +2453,10 @@ void CodeGenFunction::pushIrregularPartialArrayCleanup(llvm::Value *arrayBegin,
                                                        QualType elementType,
                                                        CharUnits elementAlign,
                                                        Destroyer *destroyer) {
-  pushFullExprCleanup<IrregularPartialArrayDestroy>(
-      NormalAndEHCleanup, arrayBegin, arrayEndPointer, elementType,
-      elementAlign, destroyer);
+  pushFullExprCleanup<IrregularPartialArrayDestroy>(EHCleanup,
+                                                    arrayBegin, arrayEndPointer,
+                                                    elementType, elementAlign,
+                                                    destroyer);
 }
 
 /// pushRegularPartialArrayCleanup - Push an EH cleanup to destroy

clang/lib/CodeGen/CGExpr.cpp

@@ -115,16 +115,10 @@ RawAddress CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
 llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
                                                     const Twine &Name,
                                                     llvm::Value *ArraySize) {
-  llvm::AllocaInst *Alloca;
   if (ArraySize)
-    Alloca = Builder.CreateAlloca(Ty, ArraySize, Name);
-  else
-    Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
-                                  ArraySize, Name, AllocaInsertPt);
-  if (Allocas) {
-    Allocas->Add(Alloca);
-  }
-  return Alloca;
+    return Builder.CreateAlloca(Ty, ArraySize, Name);
+  return new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
+                              ArraySize, Name, AllocaInsertPt);
 }
 
 /// CreateDefaultAlignTempAlloca - This creates an alloca with the