[CodeGen] Eagerly emit lifetime.end markers for calls

In C, we'll wait until the end of the scope to clean up aggregate
temporaries used for returns from calls. This means in cases like:

{
  // Assuming that `Bar` is large enough to warrant indirect returns
  struct Bar b = {};
  b = foo(&b);
  b = foo(&b);
  b = foo(&b);
  b = foo(&b);
}

...We'll allocate space for 5 Bars on the stack (`b`, and 4
temporaries). This becomes painful in things like large switch
statements.

If cleaning up sooner is trivial, we should do it.

llvm-svn: 327229

2 files changed, 129 insertions, 6 deletions

diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 5613f8d9700..371c14cde01 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -23,6 +23,7 @@
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/IntrinsicInst.h"
 using namespace clang;
 using namespace CodeGen;
 
@@ -48,7 +49,7 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
 
   // Calls `Fn` with a valid return value slot, potentially creating a temporary
   // to do so. If a temporary is created, an appropriate copy into `Dest` will
-  // be emitted.
+  // be emitted, as will lifetime markers.
   //
   // The given function should take a ReturnValueSlot, and return an RValue that
   // points to said slot.
@@ -250,16 +251,28 @@ void AggExprEmitter::withReturnValueSlot(
                  (RequiresDestruction && !Dest.getAddress().isValid());
 
   Address RetAddr = Address::invalid();
+
+  EHScopeStack::stable_iterator LifetimeEndBlock;
+  llvm::Value *LifetimeSizePtr = nullptr;
+  llvm::IntrinsicInst *LifetimeStartInst = nullptr;
   if (!UseTemp) {
     RetAddr = Dest.getAddress();
   } else {
     RetAddr = CGF.CreateMemTemp(RetTy);
     uint64_t Size =
         CGF.CGM.getDataLayout().getTypeAllocSize(CGF.ConvertTypeForMem(RetTy));
-    if (llvm::Value *LifetimeSizePtr =
-            CGF.EmitLifetimeStart(Size, RetAddr.getPointer()))
+    LifetimeSizePtr = CGF.EmitLifetimeStart(Size, RetAddr.getPointer());
+    if (LifetimeSizePtr) {
+      LifetimeStartInst =
+          cast<llvm::IntrinsicInst>(std::prev(Builder.GetInsertPoint()));
+      assert(LifetimeStartInst->getIntrinsicID() ==
+                 llvm::Intrinsic::lifetime_start &&
+             "Last insertion wasn't a lifetime.start?");
+
       CGF.pushFullExprCleanup<CodeGenFunction::CallLifetimeEnd>(
           NormalEHLifetimeMarker, RetAddr, LifetimeSizePtr);
+      LifetimeEndBlock = CGF.EHStack.stable_begin();
+    }
   }
 
   RValue Src =
@@ -268,9 +281,18 @@ void AggExprEmitter::withReturnValueSlot(
   if (RequiresDestruction)
     CGF.pushDestroy(RetTy.isDestructedType(), Src.getAggregateAddress(), RetTy);
 
-  if (UseTemp) {
-    assert(Dest.getPointer() != Src.getAggregatePointer());
-    EmitFinalDestCopy(E->getType(), Src);
+  if (!UseTemp)
+    return;
+
+  assert(Dest.getPointer() != Src.getAggregatePointer());
+  EmitFinalDestCopy(E->getType(), Src);
+
+  if (!RequiresDestruction && LifetimeStartInst) {
+    // If there's no dtor to run, the copy was the last use of our temporary.
+    // Since we're not guaranteed to be in an ExprWithCleanups, clean up
+    // eagerly.
+    CGF.DeactivateCleanupBlock(LifetimeEndBlock, LifetimeStartInst);
+    CGF.EmitLifetimeEnd(LifetimeSizePtr, RetAddr.getPointer());
   }
 }
 
diff --git a/clang/test/CodeGen/aggregate-assign-call.c b/clang/test/CodeGen/aggregate-assign-call.c
new file mode 100644
index 00000000000..3539223f214
--- /dev/null
+++ b/clang/test/CodeGen/aggregate-assign-call.c
@@ -0,0 +1,101 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=O1
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O0 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=O0
+//
+// Ensure that we place appropriate lifetime markers around indirectly returned
+// temporaries, and that the lifetime.ends appear in a timely manner.
+//
+// -O1 is used so lifetime markers actually get emitted.
+
+struct S {
+  int ns[40];
+};
+
+struct S foo(void);
+
+// CHECK-LABEL: define dso_local void @bar
+struct S bar() {
+  // O0-NOT: @llvm.lifetime.start
+  // O0-NOT: @llvm.lifetime.end
+
+  struct S r;
+  // O1: call void @llvm.lifetime.start.p0i8({{[^,]*}}, i8* nonnull %[[R_TMP:[^)]+]])
+
+  // O1: call void @llvm.lifetime.start.p0i8({{[^,]*}}, i8* nonnull %[[TMP1:[^)]+]])
+  // O1: call void @foo
+  r = foo();
+  // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* nonnull %[[TMP1]])
+
+  // O1: call void @llvm.lifetime.start.p0i8({{[^,]*}}, i8* nonnull %[[TMP2:[^)]+]])
+  // O1: call void @foo
+  r = foo();
+  // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* nonnull %[[TMP2]])
+
+  // O1: call void @llvm.lifetime.start.p0i8({{[^,]*}}, i8* nonnull %[[TMP3:[^)]+]])
+  // O1: call void @foo
+  r = foo();
+  // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* nonnull %[[TMP3]])
+
+  // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* nonnull %[[R_TMP]])
+  return r;
+}
+
+struct S foo_int(int);
+
+// Be sure that we're placing the lifetime.end so that all paths go through it.
+// Since this function turns out to be large-ish, optnone to hopefully keep it
+// stable.
+// CHECK-LABEL: define dso_local void @baz
+__attribute__((optnone))
+struct S baz(int i, volatile int *j) {
+  // O0-NOT: @llvm.lifetime.start
+  // O0-NOT: @llvm.lifetime.end
+
+  struct S r;
+  // O1: %[[RESULT_ALLOCA:[^ ]+]] = alloca %struct.S
+  // O1: %[[TMP1_ALLOCA:[^ ]+]] = alloca %struct.S
+  // O1: %[[TMP2_ALLOCA:[^ ]+]] = alloca %struct.S
+  // O1: %[[P:[^ ]+]] = bitcast %struct.S* %[[RESULT_ALLOCA]] to i8*
+  // O1: call void @llvm.lifetime.start.p0i8({{[^,]*}}, i8* %[[P]])
+  // O1: br label %[[DO_BODY:.+]]
+
+  do {
+    // O1: [[DO_BODY]]:
+    // O1: %[[P:[^ ]+]] = bitcast %struct.S* %[[TMP1_ALLOCA]] to i8*
+    // O1: call void @llvm.lifetime.start.p0i8({{[^,]*}}, i8* %[[P]])
+    // O1: br i1 {{[^,]+}}, label %[[IF_THEN:[^,]+]], label %[[IF_END:[^,]+]]
+    //
+    // O1: [[IF_THEN]]:
+    // O1: %[[P:[^ ]+]] = bitcast %struct.S* %[[TMP1_ALLOCA]] to i8*
+    // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* %[[P]])
+    // O1: br label %[[DO_END:.*]]
+    //
+    // O1: [[IF_END]]:
+    // O1: call void @foo_int(%struct.S* sret %[[TMP1_ALLOCA]],
+    // O1: call void @llvm.memcpy
+    // O1: %[[P:[^ ]+]] = bitcast %struct.S* %[[TMP1_ALLOCA]] to i8*
+    // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* %[[P]])
+    // O1: %[[P:[^ ]+]] = bitcast %struct.S* %[[TMP2_ALLOCA]] to i8*
+    // O1: call void @llvm.lifetime.start.p0i8({{[^,]*}}, i8* %[[P]])
+    // O1: call void @foo_int(%struct.S* sret %[[TMP2_ALLOCA]],
+    // O1: call void @llvm.memcpy
+    // O1: %[[P:[^ ]+]] = bitcast %struct.S* %[[TMP2_ALLOCA]] to i8*
+    // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* %[[P]])
+    // O1: br label %[[DO_COND:.*]]
+    //
+    // O1: [[DO_COND]]:
+    // O1: br label %[[DO_BODY]]
+    r = foo_int(({
+      if (*j)
+        break;
+      i++;
+    }));
+
+    r = foo_int(i++);
+   } while (1);
+
+  // O1: [[DO_END]]:
+  // O1: call void @llvm.memcpy
+  // O1: %[[P:[^ ]+]] = bitcast %struct.S* %[[RESULT_ALLOCA]] to i8*
+  // O1: call void @llvm.lifetime.end.p0i8({{[^,]*}}, i8* %[[P]])
+  return r;
+}