[CodeGen] Reuse stack space from unused function results

[sdmitrouk]

Summary

Space on stack allocated for unused structures returned by functions was unused
even when it's lifetime didn't intersect with lifetime of any other objects that
could use the same space.

The test added also checks for named and auto objects. It seems to make sense
to have this all in one place.

[rnk]

test/CodeGenCXX/stack-reuse.cpp

1 ↗

(On Diff #25677)

The IR test doesn't need -O3, just -O1 -disable-llvm-optzns. It's better to keep the test as isolated from LLVM changes as possible.

2 ↗

(On Diff #25677)

We should drop the integration test here and assume that LLVM does the right thing when the lifetime markers are right. This test will fail if the ARM backend is compiled out, but the IR test is portable. I'd rather have a test that runs everywhere instead of one that runs sometimes and breaks when LLVM changes a little.

30 ↗

(On Diff #25677)

I'd throw in a check for call void @foo_small between the lifetimes.

[sdmitrouk]

Address inline comments:

• remove ARM asm checks;
• add check for function calls between lifetime markers;
• replace -O3 with -O1 -disable-llvm-optzns.

[rnk]

lgtm, thanks!

cfe/trunk/lib/CodeGen/CGCall.cpp

RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   }
 
   ClangToLLVMArgMapping IRFunctionArgs(CGM.getContext(), CallInfo);
   SmallVector<llvm::Value *, 16> IRCallArgs(IRFunctionArgs.totalIRArgs());
 
   // If the call returns a temporary with struct return, create a temporary
   // alloca to hold the result, unless one is given to us.
   llvm::Value *SRetPtr = nullptr;
+  size_t UnusedReturnSize = 0;
   if (RetAI.isIndirect() || RetAI.isInAlloca()) {
     SRetPtr = ReturnValue.getValue();
-    if (!SRetPtr)
+    if (!SRetPtr) {
       SRetPtr = CreateMemTemp(RetTy);
+      if (HaveInsertPoint()) {
+        uint64_t size =
+            CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(RetTy));
+        if (EmitLifetimeStart(size, SRetPtr))
+          UnusedReturnSize = size;
+      }
+    }
     if (IRFunctionArgs.hasSRetArg()) {
       IRCallArgs[IRFunctionArgs.getSRetArgNo()] = SRetPtr;
     } else {
       llvm::Value *Addr =
           Builder.CreateStructGEP(ArgMemory->getAllocatedType(), ArgMemory,
                                   RetAI.getInAllocaFieldIndex());
       Builder.CreateStore(SRetPtr, Addr);
     }
#endif
   // optimizer it can aggressively ignore unwind edges.
   if (CGM.getLangOpts().ObjCAutoRefCount)
     AddObjCARCExceptionMetadata(CS.getInstruction());
 
   // If the call doesn't return, finish the basic block and clear the
   // insertion point; this allows the rest of IRgen to discard
   // unreachable code.
   if (CS.doesNotReturn()) {
+    if (UnusedReturnSize)
+      EmitLifetimeEnd(llvm::ConstantInt::get(Int64Ty, UnusedReturnSize),
+                      SRetPtr);
+
     Builder.CreateUnreachable();
     Builder.ClearInsertionPoint();
 
     // FIXME: For now, emit a dummy basic block because expr emitters in
     // generally are not ready to handle emitting expressions at unreachable
     // points.
     EnsureInsertPoint();
 
#endif
 
   // The stack cleanup for inalloca arguments has to run out of the normal
   // lexical order, so deactivate it and run it manually here.
   CallArgs.freeArgumentMemory(*this);
 
   RValue Ret = [&] {
     switch (RetAI.getKind()) {
     case ABIArgInfo::InAlloca:
-    case ABIArgInfo::Indirect:
-      return convertTempToRValue(SRetPtr, RetTy, SourceLocation());
+    case ABIArgInfo::Indirect: {
+      RValue ret = convertTempToRValue(SRetPtr, RetTy, SourceLocation());
+      if (UnusedReturnSize)
+        EmitLifetimeEnd(llvm::ConstantInt::get(Int64Ty, UnusedReturnSize),
+                        SRetPtr);
+      return ret;
+    }
 
     case ABIArgInfo::Ignore:
       // If we are ignoring an argument that had a result, make sure to
       // construct the appropriate return value for our caller.
       return GetUndefRValue(RetTy);
 
     case ABIArgInfo::Extend:
     case ABIArgInfo::Direct: {

cfe/trunk/test/CodeGenCXX/stack-reuse.cpp

+// RUN: %clang -target armv7l-unknown-linux-gnueabihf -S %s -o - -emit-llvm -O1 -disable-llvm-optzns | FileCheck %s
+
+// Stack should be reused when possible, no need to allocate two separate slots
+// if they have disjoint lifetime.
+
+// Sizes of objects are related to previously existed threshold of 32.  In case
+// of S_large stack size is rounded to 40 bytes.
+
+// 32B
+struct S_small {
+  int a[8];
+};
+
+// 36B
+struct S_large {
+  int a[9];
+};
+
+extern S_small foo_small();
+extern S_large foo_large();
+extern void bar_small(S_small*);
+extern void bar_large(S_large*);
+
+// Prevent mangling of function names.
+extern "C" {
+
+void small_rvoed_unnamed_temporary_object() {
+// CHECK-LABEL: define void @small_rvoed_unnamed_temporary_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @llvm.lifetime.end
+
+  foo_small();
+  foo_small();
+}
+
+void large_rvoed_unnamed_temporary_object() {
+// CHECK-LABEL: define void @large_rvoed_unnamed_temporary_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @llvm.lifetime.end
+
+  foo_large();
+  foo_large();
+}
+
+void small_rvoed_named_temporary_object() {
+// CHECK-LABEL: define void @small_rvoed_named_temporary_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @llvm.lifetime.end
+
+  {
+    S_small s = foo_small();
+  }
+  {
+    S_small s = foo_small();
+  }
+}
+
+void large_rvoed_named_temporary_object() {
+// CHECK-LABEL: define void @large_rvoed_named_temporary_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @llvm.lifetime.end
+
+  {
+    S_large s = foo_large();
+  }
+  {
+    S_large s = foo_large();
+  }
+}
+
+void small_auto_object() {
+// CHECK-LABEL: define void @small_auto_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9bar_smallP7S_small
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9bar_smallP7S_small
+// CHECK: call void @llvm.lifetime.end
+
+  {
+    S_small s;
+    bar_small(&s);
+  }
+  {
+    S_small s;
+    bar_small(&s);
+  }
+}
+
+void large_auto_object() {
+// CHECK-LABEL: define void @large_auto_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9bar_largeP7S_large
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9bar_largeP7S_large
+// CHECK: call void @llvm.lifetime.end
+
+  {
+    S_large s;
+    bar_large(&s);
+  }
+  {
+    S_large s;
+    bar_large(&s);
+  }
+}
+
+}