[CodeGen] Reuse stack space from unused function results (with more accurate unused result detection)

This patch fixes issues with unused result detection which were found in patch http://reviews.llvm.org/D9743.

Differential Revision: http://reviews.llvm.org/D10042

llvm-svn: 239294

clang/lib/CodeGen/CGCall.cpp

@@ -3082,10 +3082,18 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   // 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() && ReturnValue.isUnused()) {
+        uint64_t size =
+            CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(RetTy));
+        if (EmitLifetimeStart(size, SRetPtr))
+          UnusedReturnSize = size;
+      }
+    }
     if (IRFunctionArgs.hasSRetArg()) {
       IRCallArgs[IRFunctionArgs.getSRetArgNo()] = SRetPtr;
     } else {
@@ -3417,6 +3425,10 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   // 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();
 
@@ -3445,8 +3457,13 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   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

clang/lib/CodeGen/CGCall.h

@@ -155,17 +155,25 @@ namespace CodeGen {
   /// ReturnValueSlot - Contains the address where the return value of a 
   /// function can be stored, and whether the address is volatile or not.
   class ReturnValueSlot {
-    llvm::PointerIntPair<llvm::Value *, 1, bool> Value;
+    llvm::PointerIntPair<llvm::Value *, 2, unsigned int> Value;
+
+    // Return value slot flags
+    enum Flags {
+      IS_VOLATILE = 0x1,
+      IS_UNUSED = 0x2,
+    };
 
   public:
     ReturnValueSlot() {}
-    ReturnValueSlot(llvm::Value *Value, bool IsVolatile)
-      : Value(Value, IsVolatile) {}
+    ReturnValueSlot(llvm::Value *Value, bool IsVolatile, bool IsUnused = false)
+      : Value(Value,
+              (IsVolatile ? IS_VOLATILE : 0) | (IsUnused ? IS_UNUSED : 0)) {}
 
     bool isNull() const { return !getValue(); }
-    
-    bool isVolatile() const { return Value.getInt(); }
+
+    bool isVolatile() const { return Value.getInt() & IS_VOLATILE; }
     llvm::Value *getValue() const { return Value.getPointer(); }
+    bool isUnused() const { return Value.getInt() & IS_UNUSED; }
   };
   
 }  // end namespace CodeGen

clang/lib/CodeGen/CGExprAgg.cpp

@@ -34,6 +34,7 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
   CodeGenFunction &CGF;
   CGBuilderTy &Builder;
   AggValueSlot Dest;
+  bool IsResultUnused;
 
   /// We want to use 'dest' as the return slot except under two
   /// conditions:
@@ -48,7 +49,7 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
     if (!shouldUseDestForReturnSlot())
       return ReturnValueSlot();
 
-    return ReturnValueSlot(Dest.getAddr(), Dest.isVolatile());
+    return ReturnValueSlot(Dest.getAddr(), Dest.isVolatile(), IsResultUnused);
   }
 
   AggValueSlot EnsureSlot(QualType T) {
@@ -61,9 +62,9 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
   }
 
 public:
-  AggExprEmitter(CodeGenFunction &cgf, AggValueSlot Dest)
-    : CGF(cgf), Builder(CGF.Builder), Dest(Dest) {
-  }
+  AggExprEmitter(CodeGenFunction &cgf, AggValueSlot Dest, bool IsResultUnused)
+    : CGF(cgf), Builder(CGF.Builder), Dest(Dest),
+    IsResultUnused(IsResultUnused) { }
 
   //===--------------------------------------------------------------------===//
   //                               Utilities
@@ -1394,7 +1395,7 @@ void CodeGenFunction::EmitAggExpr(const Expr *E, AggValueSlot Slot) {
   // Optimize the slot if possible.
   CheckAggExprForMemSetUse(Slot, E, *this);
  
-  AggExprEmitter(*this, Slot).Visit(const_cast<Expr*>(E));
+  AggExprEmitter(*this, Slot, Slot.isIgnored()).Visit(const_cast<Expr*>(E));
 }
 
 LValue CodeGenFunction::EmitAggExprToLValue(const Expr *E) {

clang/test/CodeGenCXX/stack-reuse-miscompile.cpp

@@ -0,0 +1,39 @@
+// RUN: %clang -S -emit-llvm -O1 -mllvm -disable-llvm-optzns -S %s -o - | FileCheck %s
+
+// This test should not to generate llvm.lifetime.start/llvm.lifetime.end for
+// f function because all temporary objects in this function are used for the
+// final result
+
+class S {
+  char *ptr;
+  unsigned int len;
+};
+
+class T {
+  S left;
+  S right;
+
+public:
+  T(const char s[]);
+  T(S);
+
+  T concat(const T &Suffix) const;
+  const char * str() const;
+};
+
+const char * f(S s)
+{
+// CHECK: %1 = alloca %class.T, align 4
+// CHECK: %2 = alloca %class.T, align 4
+// CHECK: %3 = alloca %class.S, align 4
+// CHECK: %4 = alloca %class.T, align 4
+// CHECK: %5 = call x86_thiscallcc %class.T* @"\01??0T@@QAE@QBD@Z"
+// CHECK: %6 = bitcast %class.S* %3 to i8*
+// CHECK: %7 = bitcast %class.S* %s to i8*
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32
+// CHECK: %8 = call x86_thiscallcc %class.T* @"\01??0T@@QAE@VS@@@Z"
+// CHECK: call x86_thiscallcc void @"\01?concat@T@@QBE?AV1@ABV1@@Z"
+// CHECK: %9 = call x86_thiscallcc i8* @"\01?str@T@@QBEPBDXZ"(%class.T* %4)
+
+  return T("[").concat(T(s)).str();
+}

clang/test/CodeGenCXX/stack-reuse.cpp

@@ -0,0 +1,146 @@
+// 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];
+};
+
+// Helper class for lifetime scope absence testing
+struct Combiner {
+  S_large a, b;
+
+  Combiner(S_large);
+  Combiner f();  
+};
+
+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);
+  }
+}
+
+int large_combiner_test(S_large s) {
+// CHECK-LABEL: define i32 @large_combiner_test
+// CHECK: %1 = alloca %struct.Combiner
+// CHECK: %2 = alloca %struct.Combiner
+// CHECK: %3 = call %struct.Combiner* @_ZN8CombinerC1E7S_large(%struct.Combiner* %1, [9 x i32] %s.coerce)
+// CHECK: call void @_ZN8Combiner1fEv(%struct.Combiner* sret %2, %struct.Combiner* %1)
+// CHECK: %4 = getelementptr inbounds %struct.Combiner, %struct.Combiner* %2, i32 0, i32 0, i32 0, i32 0
+// CHECK: %5 = load i32, i32* %4
+// CHECK: ret i32 %5
+
+  return Combiner(s).f().a.a[0];
+}
+
+}