itessier created this revision.
itessier added a reviewer: rjmccall.
Herald added a subscriber: cfe-commits.
Clang is not emitting lifetime markers for temporary function parameters, so
more stack space is potentially being allocated than necessary.
A new parameter is added to the CreateAggTemp and EmitAnyExprToTemp functions
to indicate whether to emit lifetime markers for aggregates, so that the
EmitCallArg function can request them when evaluating an arg. The parameter is
defaulted to false so that the behaviour of other callers is not affected.
Repository:
rC Clang
https://reviews.llvm.org/D52440
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
test/CodeGenCXX/stack-reuse-miscompile.cpp
test/CodeGenCXX/stack-reuse.cpp
Index: test/CodeGenCXX/stack-reuse.cpp
===================================================================
--- test/CodeGenCXX/stack-reuse.cpp
+++ test/CodeGenCXX/stack-reuse.cpp
@@ -27,7 +27,9 @@
extern S_small foo_small();
extern S_large foo_large();
extern void bar_small(S_small*);
+extern void bar_small(S_small);
extern void bar_large(S_large*);
+extern void bar_large(S_large);
// Prevent mangling of function names.
extern "C" {
@@ -130,6 +132,36 @@
}
}
+void small_temp_object() {
+// CHECK-LABEL: define void @small_temp_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @_Z9bar_small7S_small
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @_Z9bar_small7S_small
+// CHECK: call void @llvm.lifetime.end
+
+ bar_small(foo_small());
+ bar_small(foo_small());
+}
+
+void large_temp_object() {
+// CHECK-LABEL: define void @large_temp_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @_Z9bar_large7S_large
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @_Z9bar_large7S_large
+// CHECK: call void @llvm.lifetime.end
+
+ bar_large(foo_large());
+ bar_large(foo_large());
+}
+
int large_combiner_test(S_large s) {
// CHECK-LABEL: define i32 @large_combiner_test
// CHECK: [[T2:%.*]] = alloca %struct.Combiner
Index: test/CodeGenCXX/stack-reuse-miscompile.cpp
===================================================================
--- test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -19,33 +19,39 @@
const char * f(S s)
{
-// It's essential that the lifetimes of all three T temporaries here are
+// It's essential that the lifetimes of the first three T temporaries here are
// overlapping. They must all remain alive through the call to str().
//
// CHECK: [[T1:%.*]] = alloca %class.T, align 4
// CHECK: [[T2:%.*]] = alloca %class.T, align 4
// CHECK: [[T3:%.*]] = alloca %class.T, align 4
+// CHECK: [[T4:%.*]] = alloca %class.S, align 4
//
// FIXME: We could defer starting the lifetime of the return object of concat
// until the call.
// CHECK: [[T1i8:%.*]] = bitcast %class.T* [[T1]] to i8*
// CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T1i8]])
//
// CHECK: [[T2i8:%.*]] = bitcast %class.T* [[T2]] to i8*
// CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T2i8]])
-// CHECK: [[T4:%.*]] = call %class.T* @_ZN1TC1EPKc(%class.T* [[T2]], i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str, i32 0, i32 0))
+// CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1EPKc(%class.T* [[T2]], i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str, i32 0, i32 0))
//
// CHECK: [[T3i8:%.*]] = bitcast %class.T* [[T3]] to i8*
// CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T3i8]])
-// CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
+//
+// CHECK: [[T4i8:%.*]] = bitcast %class.S* [[T4]] to i8*
+// CHECK: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[T4i8]])
+//
+// CHECK: [[T6:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
//
// CHECK: call void @_ZNK1T6concatERKS_(%class.T* sret [[T1]], %class.T* [[T2]], %class.T* dereferenceable(16) [[T3]])
-// CHECK: [[T6:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
+// CHECK: [[T7:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
//
// CHECK: call void @llvm.lifetime.end.p0i8(
// CHECK: call void @llvm.lifetime.end.p0i8(
// CHECK: call void @llvm.lifetime.end.p0i8(
-// CHECK: ret i8* [[T6]]
+// CHECK: call void @llvm.lifetime.end.p0i8(
+// CHECK: ret i8* [[T7]]
return T("[").concat(T(s)).str();
}
Index: test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
+++ test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
@@ -322,6 +322,10 @@
// WIN32: call void @llvm.memset.p0i8.i32(
// WIN32: call i32 @"?getS@PR37146@@YA?AUS@1@XZ"(
// WIN32: call void @"?func@PR37146@@YAXUS@1@0@Z"(
+// WIN32-NEXT: bitcast
+// WIN32-NEXT: call void @llvm.lifetime.end.p0i8(
+// WIN32-NEXT: bitcast
+// WIN32-NEXT: call void @llvm.lifetime.end.p0i8(
// WIN32-NEXT: ret void
// WIN32-NEXT: {{^}$}}
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2227,14 +2227,8 @@
/// CreateAggTemp - Create a temporary memory object for the given
/// aggregate type.
- AggValueSlot CreateAggTemp(QualType T, const Twine &Name = "tmp") {
- return AggValueSlot::forAddr(CreateMemTemp(T, Name),
- T.getQualifiers(),
- AggValueSlot::IsNotDestructed,
- AggValueSlot::DoesNotNeedGCBarriers,
- AggValueSlot::IsNotAliased,
- AggValueSlot::DoesNotOverlap);
- }
+ AggValueSlot CreateAggTemp(QualType T, const Twine &Name = "tmp",
+ bool EmitLifetimeMarkers = false);
/// Emit a cast to void* in the appropriate address space.
llvm::Value *EmitCastToVoidPtr(llvm::Value *value);
@@ -2267,7 +2261,10 @@
/// EmitAnyExprToTemp - Similarly to EmitAnyExpr(), however, the result will
/// always be accessible even if no aggregate location is provided.
- RValue EmitAnyExprToTemp(const Expr *E);
+ ///
+ /// \param EmitLifetimeMarkers True if lifetime markers should be emitted when
+ /// creating an aggregrate location.
+ RValue EmitAnyExprToTemp(const Expr *E, bool EmitLifetimeMarkers = false);
/// EmitAnyExprToMem - Emits the code necessary to evaluate an
/// arbitrary expression into the given memory location.
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -159,6 +159,26 @@
Name);
}
+AggValueSlot CodeGenFunction::CreateAggTemp(QualType T, const Twine &Name,
+ bool EmitLifetimeMarkers) {
+ AggValueSlot Slot = AggValueSlot::forAddr(CreateMemTemp(T, Name),
+ T.getQualifiers(),
+ AggValueSlot::IsNotDestructed,
+ AggValueSlot::DoesNotNeedGCBarriers,
+ AggValueSlot::IsNotAliased,
+ AggValueSlot::DoesNotOverlap);
+ if (ShouldEmitLifetimeMarkers && EmitLifetimeMarkers) {
+ uint64_t AllocSize =
+ CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(T));
+ auto *Size = EmitLifetimeStart(AllocSize, Slot.getPointer());
+ if (Size) {
+ pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker,
+ Slot.getAddress(), Size);
+ }
+ }
+ return Slot;
+}
+
/// EvaluateExprAsBool - Perform the usual unary conversions on the specified
/// expression and compare the result against zero, returning an Int1Ty value.
llvm::Value *CodeGenFunction::EvaluateExprAsBool(const Expr *E) {
@@ -210,11 +230,12 @@
/// EmitAnyExprToTemp - Similar to EmitAnyExpr(), however, the result will
/// always be accessible even if no aggregate location is provided.
-RValue CodeGenFunction::EmitAnyExprToTemp(const Expr *E) {
+RValue CodeGenFunction::EmitAnyExprToTemp(const Expr *E,
+ bool EmitLifetimeMarkers) {
AggValueSlot AggSlot = AggValueSlot::ignored();
if (hasAggregateEvaluationKind(E->getType()))
- AggSlot = CreateAggTemp(E->getType(), "agg.tmp");
+ AggSlot = CreateAggTemp(E->getType(), "agg.tmp", EmitLifetimeMarkers);
return EmitAnyExpr(E, AggSlot);
}
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -3587,7 +3587,7 @@
if (args.isUsingInAlloca())
Slot = createPlaceholderSlot(*this, type);
else
- Slot = CreateAggTemp(type, "agg.tmp");
+ Slot = CreateAggTemp(type, "agg.tmp", /*EmitLifetimeMarkers=*/true);
bool DestroyedInCallee = true, NeedsEHCleanup = true;
if (const auto *RD = type->getAsCXXRecordDecl())
@@ -3623,7 +3623,7 @@
return;
}
- args.add(EmitAnyExprToTemp(E), type);
+ args.add(EmitAnyExprToTemp(E, /*EmitLifetimeMarkers=*/true), type);
}
QualType CodeGenFunction::getVarArgType(const Expr *Arg) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
