Reid, first of all thank you very much for your comments. I've updated the
implementation. Now I do decision about result usage inside
CodeGenFunction::EmitAggExpr method. For an unused result I have incoming
expression with type CallExpr. But for the used result I have two calls of
EmitAggExpr method second of which has incoming expression with type
MaterializeTemporaryExpr. So I decided to mark result as used in case of
incoming MaterializeTemporaryExpr expression and to mark result as unused in
other cases.
In the current implementation I don't use automatically cleanup mechanism for
CallLifetimeEnd. I want to prepare it in separate patch.
REPOSITORY
rL LLVM
http://reviews.llvm.org/D10042
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCall.h
lib/CodeGen/CGExprAgg.cpp
test/CodeGenCXX/stack-reuse-miscompile.cpp
test/CodeGenCXX/stack-reuse.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -3081,10 +3081,18 @@
// 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 {
@@ -3416,6 +3424,10 @@
// 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();
@@ -3444,8 +3456,13 @@
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
Index: lib/CodeGen/CGCall.h
===================================================================
--- lib/CodeGen/CGCall.h
+++ lib/CodeGen/CGCall.h
@@ -156,16 +156,18 @@
/// function can be stored, and whether the address is volatile or not.
class ReturnValueSlot {
llvm::PointerIntPair<llvm::Value *, 1, bool> Value;
+ bool IsUnused = false;
public:
ReturnValueSlot() {}
- ReturnValueSlot(llvm::Value *Value, bool IsVolatile)
- : Value(Value, IsVolatile) {}
+ ReturnValueSlot(llvm::Value *Value, bool IsVolatile, bool isUnused = false)
+ : Value(Value, IsVolatile), IsUnused(isUnused) {}
bool isNull() const { return !getValue(); }
bool isVolatile() const { return Value.getInt(); }
llvm::Value *getValue() const { return Value.getPointer(); }
+ bool isUnused() const { return IsUnused; }
};
} // end namespace CodeGen
Index: lib/CodeGen/CGExprAgg.cpp
===================================================================
--- lib/CodeGen/CGExprAgg.cpp
+++ lib/CodeGen/CGExprAgg.cpp
@@ -34,6 +34,7 @@
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 @@
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 @@
}
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,8 @@
// Optimize the slot if possible.
CheckAggExprForMemSetUse(Slot, E, *this);
- AggExprEmitter(*this, Slot).Visit(const_cast<Expr*>(E));
+ bool isResultUnused = !isa<const MaterializeTemporaryExpr>(E);
+ AggExprEmitter(*this, Slot, isResultUnused).Visit(const_cast<Expr*>(E));
}
LValue CodeGenFunction::EmitAggExprToLValue(const Expr *E) {
Index: test/CodeGenCXX/stack-reuse-miscompile.cpp
===================================================================
--- test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ 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();
+}
Index: test/CodeGenCXX/stack-reuse.cpp
===================================================================
--- test/CodeGenCXX/stack-reuse.cpp
+++ test/CodeGenCXX/stack-reuse.cpp
@@ -0,0 +1,125 @@
+// 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);
+ }
+}
+
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
