Skip to content

Commit bb5df6d

Browse files
epilkrazmser
authored andcommittedOct 11, 2023
Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas
This reverts commit e26c24b. These temporaries are only used in the callee, and their memory can be reused after the call is complete. rdar://58552124 Link: llvm#38157 Link: llvm#41896 Link: llvm#43598 Link: ClangBuiltLinux/linux#39 Link: https://reviews.llvm.org/rGfafc6e4fdf3673dcf557d6c8ae0c0a4bb3184402 Reviewed By: rjmccall Differential Revision: https://reviews.llvm.org/D74094
1 parent e18050f commit bb5df6d

File tree

6 files changed

+149
-1
lines changed

6 files changed

+149
-1
lines changed
 

‎clang/lib/CodeGen/CGCall.cpp

+22-1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "llvm/IR/IntrinsicInst.h"
4141
#include "llvm/IR/Intrinsics.h"
4242
#include "llvm/IR/Type.h"
43+
#include "llvm/Support/TypeSize.h"
4344
#include "llvm/Transforms/Utils/Local.h"
4445
#include <optional>
4546
using namespace clang;
@@ -4644,7 +4645,24 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
46444645
return;
46454646
}
46464647

4647-
args.add(EmitAnyExprToTemp(E), type);
4648+
AggValueSlot ArgSlot = AggValueSlot::ignored();
4649+
if (hasAggregateEvaluationKind(E->getType())) {
4650+
Address ArgSlotAlloca = Address::invalid();
4651+
ArgSlot = CreateAggTemp(E->getType(), "agg.tmp", &ArgSlotAlloca);
4652+
4653+
// Emit a lifetime start/end for this temporary. If the type has a
4654+
// destructor, then we need to keep it alive. FIXME: We should still be able
4655+
// to end the lifetime after the destructor returns.
4656+
if (!E->getType().isDestructedType()) {
4657+
llvm::TypeSize size =
4658+
CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(E->getType()));
4659+
if (llvm::Value *lifetimeSize =
4660+
EmitLifetimeStart(size, ArgSlotAlloca.getPointer()))
4661+
args.addLifetimeCleanup({ArgSlotAlloca.getPointer(), lifetimeSize});
4662+
}
4663+
}
4664+
4665+
args.add(EmitAnyExpr(E, ArgSlot), type);
46484666
}
46494667

46504668
QualType CodeGenFunction::getVarArgType(const Expr *Arg) {
@@ -5813,6 +5831,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
58135831
for (CallLifetimeEnd &LifetimeEnd : CallLifetimeEndAfterCall)
58145832
LifetimeEnd.Emit(*this, /*Flags=*/{});
58155833

5834+
for (const CallArgList::EndLifetimeInfo &LT : CallArgs.getLifetimeCleanups())
5835+
EmitLifetimeEnd(LT.Size, LT.Addr);
5836+
58165837
if (!ReturnValue.isExternallyDestructed() &&
58175838
RetTy.isDestructedType() == QualType::DK_nontrivial_c_struct)
58185839
pushDestroy(QualType::DK_nontrivial_c_struct, Ret.getAggregateAddress(),

‎clang/lib/CodeGen/CGCall.h

+20
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,11 @@ class CallArgList : public SmallVector<CallArg, 8> {
278278
llvm::Instruction *IsActiveIP;
279279
};
280280

281+
struct EndLifetimeInfo {
282+
llvm::Value *Addr;
283+
llvm::Value *Size;
284+
};
285+
281286
void add(RValue rvalue, QualType type) { push_back(CallArg(rvalue, type)); }
282287

283288
void addUncopiedAggregate(LValue LV, QualType type) {
@@ -294,6 +299,9 @@ class CallArgList : public SmallVector<CallArg, 8> {
294299
CleanupsToDeactivate.insert(CleanupsToDeactivate.end(),
295300
other.CleanupsToDeactivate.begin(),
296301
other.CleanupsToDeactivate.end());
302+
LifetimeCleanups.insert(LifetimeCleanups.end(),
303+
other.LifetimeCleanups.begin(),
304+
other.LifetimeCleanups.end());
297305
assert(!(StackBase && other.StackBase) && "can't merge stackbases");
298306
if (!StackBase)
299307
StackBase = other.StackBase;
@@ -333,6 +341,14 @@ class CallArgList : public SmallVector<CallArg, 8> {
333341
/// memory.
334342
bool isUsingInAlloca() const { return StackBase; }
335343

344+
void addLifetimeCleanup(EndLifetimeInfo Info) {
345+
LifetimeCleanups.push_back(Info);
346+
}
347+
348+
ArrayRef<EndLifetimeInfo> getLifetimeCleanups() const {
349+
return LifetimeCleanups;
350+
}
351+
336352
private:
337353
SmallVector<Writeback, 1> Writebacks;
338354

@@ -341,6 +357,10 @@ class CallArgList : public SmallVector<CallArg, 8> {
341357
/// occurs.
342358
SmallVector<CallArgCleanup, 1> CleanupsToDeactivate;
343359

360+
/// Lifetime information needed to call llvm.lifetime.end for any temporary
361+
/// argument allocas.
362+
SmallVector<EndLifetimeInfo, 2> LifetimeCleanups;
363+
344364
/// The stacksave call. It dominates all of the argument evaluation.
345365
llvm::CallInst *StackBase;
346366
};
+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// RUN: %clang -cc1 -triple x86_64-apple-macos -O1 -disable-llvm-passes %s -S -emit-llvm -o - | FileCheck %s --implicit-check-not=llvm.lifetime
2+
// RUN: %clang -cc1 -xc++ -std=c++17 -triple x86_64-apple-macos -O1 -disable-llvm-passes %s -S -emit-llvm -o - -Wno-return-type-c-linkage | FileCheck %s --implicit-check-not=llvm.lifetime --check-prefix=CHECK --check-prefix=CXX
3+
// RUN: %clang -cc1 -xobjective-c -triple x86_64-apple-macos -O1 -disable-llvm-passes %s -S -emit-llvm -o - | FileCheck %s --implicit-check-not=llvm.lifetime --check-prefix=CHECK --check-prefix=OBJC
4+
5+
typedef struct { int x[100]; } aggregate;
6+
7+
#ifdef __cplusplus
8+
extern "C" {
9+
#endif
10+
11+
void takes_aggregate(aggregate);
12+
aggregate gives_aggregate();
13+
14+
// CHECK-LABEL: define void @t1
15+
void t1() {
16+
takes_aggregate(gives_aggregate());
17+
18+
// CHECK: [[AGGTMP:%.*]] = alloca %struct.aggregate, align 8
19+
// CHECK: call void @llvm.lifetime.start.p0(i64 400, ptr [[AGGTMP]])
20+
// CHECK: call void{{.*}} @gives_aggregate(ptr sret(%struct.aggregate) align 4 [[AGGTMP]])
21+
// CHECK: call void @takes_aggregate(ptr noundef byval(%struct.aggregate) align 8 [[AGGTMP]])
22+
// CHECK: call void @llvm.lifetime.end.p0(i64 400, ptr [[AGGTMP]])
23+
}
24+
25+
// CHECK: declare {{.*}}llvm.lifetime.start
26+
// CHECK: declare {{.*}}llvm.lifetime.end
27+
28+
#ifdef __cplusplus
29+
// CXX: define void @t2
30+
void t2() {
31+
struct S {
32+
S(aggregate) {}
33+
};
34+
S{gives_aggregate()};
35+
36+
// CXX: [[AGG:%.*]] = alloca %struct.aggregate
37+
// CXX: call void @llvm.lifetime.start.p0(i64 400, ptr
38+
// CXX: call void @gives_aggregate(ptr sret(%struct.aggregate) align 4 [[AGG]])
39+
// CXX: call void @_ZZ2t2EN1SC1E9aggregate(ptr {{.*}}, ptr {{.*}} byval(%struct.aggregate) align 8 [[AGG]])
40+
// CXX: call void @llvm.lifetime.end.p0(i64 400, ptr
41+
}
42+
43+
struct Dtor {
44+
~Dtor();
45+
};
46+
47+
void takes_dtor(Dtor);
48+
Dtor gives_dtor();
49+
50+
// CXX: define void @t3
51+
void t3() {
52+
takes_dtor(gives_dtor());
53+
54+
// CXX-NOT @llvm.lifetime
55+
// CXX: ret void
56+
}
57+
58+
#endif
59+
60+
#ifdef __OBJC__
61+
62+
@interface X
63+
-m:(aggregate)x;
64+
@end
65+
66+
// OBJC: define void @t4
67+
void t4(X *x) {
68+
[x m: gives_aggregate()];
69+
70+
// OBJC: [[AGG:%.*]] = alloca %struct.aggregate
71+
// OBJC: call void @llvm.lifetime.start.p0(i64 400, ptr
72+
// OBJC: call void{{.*}} @gives_aggregate(ptr sret(%struct.aggregate) align 4 [[AGGTMP]])
73+
// OBJC: call {{.*}}@objc_msgSend
74+
// OBJC: call void @llvm.lifetime.end.p0(i64 400, ptr
75+
}
76+
77+
#endif
78+
79+
#ifdef __cplusplus
80+
}
81+
#endif
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -O3 -disable-llvm-passes -o - %s | FileCheck %s
2+
3+
struct A {
4+
float x, y, z, w;
5+
};
6+
7+
void foo(A a);
8+
9+
// CHECK-LABEL: @_Z4testv
10+
// CHECK: [[A:%.*]] = alloca [[STRUCT_A:%.*]], align 4, addrspace(5)
11+
// CHECK-NEXT: [[AGG_TMP:%.*]] = alloca [[STRUCT_A]], align 4, addrspace(5)
12+
// CHECK-NEXT: [[A_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[A]] to ptr
13+
// CHECK-NEXT: [[AGG_TMP_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[AGG_TMP]] to ptr
14+
// CHECK-NEXT: call void @llvm.lifetime.start.p5(i64 16, ptr addrspace(5) [[A]]) #[[ATTR4:[0-9]+]]
15+
// CHECK-NEXT: call void @llvm.lifetime.start.p5(i64 16, ptr addrspace(5) [[AGG_TMP]]) #[[ATTR4]]
16+
void test() {
17+
A a;
18+
foo(a);
19+
}

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

+4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ const char * f(S s)
2626
// CHECK: [[T2:%.*]] = alloca %class.T, align 4
2727
// CHECK: [[T3:%.*]] = alloca %class.T, align 4
2828
//
29+
// CHECK: [[AGG:%.*]] = alloca %class.S, align 4
30+
//
2931
// FIXME: We could defer starting the lifetime of the return object of concat
3032
// until the call.
3133
// CHECK: call void @llvm.lifetime.start.p0(i64 16, ptr [[T1]])
@@ -34,7 +36,9 @@ const char * f(S s)
3436
// CHECK: [[T4:%.*]] = call noundef ptr @_ZN1TC1EPKc(ptr {{[^,]*}} [[T2]], ptr noundef @.str)
3537
//
3638
// CHECK: call void @llvm.lifetime.start.p0(i64 16, ptr [[T3]])
39+
// CHECK: call void @llvm.lifetime.start.p0(i64 8, ptr [[AGG]])
3740
// CHECK: [[T5:%.*]] = call noundef ptr @_ZN1TC1E1S(ptr {{[^,]*}} [[T3]], [2 x i32] %{{.*}})
41+
// CHECK: call void @llvm.lifetime.end.p0(i64 8, ptr [[AGG]]
3842
//
3943
// CHECK: call void @_ZNK1T6concatERKS_(ptr sret(%class.T) align 4 [[T1]], ptr {{[^,]*}} [[T2]], ptr noundef nonnull align 4 dereferenceable(16) [[T3]])
4044
// CHECK: [[T6:%.*]] = call noundef ptr @_ZNK1T3strEv(ptr {{[^,]*}} [[T1]])

‎clang/test/CodeGenCoroutines/pr59181.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,15 @@ void foo() {
4949
}
5050

5151
// CHECK: cleanup.cont:{{.*}}
52+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 4, ptr [[AGG:%agg.tmp[0-9]+]])
5253
// CHECK-NEXT: load i8
5354
// CHECK-NEXT: trunc
5455
// CHECK-NEXT: store i1 false
5556
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF:%ref.tmp[0-9]+]])
5657

5758
// CHECK: await.suspend:{{.*}}
5859
// CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF]])
60+
// CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[AGG]])
5961
// CHECK: call void @_ZZN4Task12promise_type15await_transformES_EN10Suspension13await_suspendESt16coroutine_handleIvE
62+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[AGG2:%agg.tmp[0-9]+]])
6063
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[REF]])

0 commit comments

Comments
 (0)
Please sign in to comment.