From c8cc5c3e5b634c9ce6fbc5f6d9ee7955f182587f Mon Sep 17 00:00:00 2001 From: Erik Pilkington Date: Wed, 16 Aug 2023 15:21:05 -0700 Subject: [PATCH] Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas This reverts commit e26c24b849211f35a988d001753e0cd15e4a9d7b. These temporaries are only used in the callee, and their memory can be reused after the call is complete. rdar://58552124 Link: https://github.com/llvm/llvm-project/issues/38157 Link: https://github.com/llvm/llvm-project/issues/41896 Link: https://github.com/llvm/llvm-project/issues/43598 Link: https://github.com/ClangBuiltLinux/linux/issues/39 Link: https://reviews.llvm.org/rGfafc6e4fdf3673dcf557d6c8ae0c0a4bb3184402 Reviewed By: rjmccall Differential Revision: https://reviews.llvm.org/D74094 --- clang/lib/CodeGen/CGCall.cpp | 23 +++++- clang/lib/CodeGen/CGCall.h | 20 +++++ clang/test/CodeGen/lifetime-call-temp.c | 81 +++++++++++++++++++ .../CodeGenCXX/amdgcn-call-with-aggarg.cpp | 19 +++++ .../CodeGenCXX/stack-reuse-miscompile.cpp | 4 + clang/test/CodeGenCoroutines/pr59181.cpp | 3 + 6 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 clang/test/CodeGen/lifetime-call-temp.c create mode 100644 clang/test/CodeGenCXX/amdgcn-call-with-aggarg.cpp diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 920a219205a21..204858e80770b 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -40,6 +40,7 @@ #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/Type.h" +#include "llvm/Support/TypeSize.h" #include "llvm/Transforms/Utils/Local.h" #include using namespace clang; @@ -4644,7 +4645,24 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E, return; } - args.add(EmitAnyExprToTemp(E), type); + AggValueSlot ArgSlot = AggValueSlot::ignored(); + if (hasAggregateEvaluationKind(E->getType())) { + Address ArgSlotAlloca = Address::invalid(); + ArgSlot = CreateAggTemp(E->getType(), "agg.tmp", &ArgSlotAlloca); + + // Emit a lifetime start/end for this temporary. If the type has a + // destructor, then we need to keep it alive. FIXME: We should still be able + // to end the lifetime after the destructor returns. + if (!E->getType().isDestructedType()) { + llvm::TypeSize size = + CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(E->getType())); + if (llvm::Value *lifetimeSize = + EmitLifetimeStart(size, ArgSlotAlloca.getPointer())) + args.addLifetimeCleanup({ArgSlotAlloca.getPointer(), lifetimeSize}); + } + } + + args.add(EmitAnyExpr(E, ArgSlot), type); } QualType CodeGenFunction::getVarArgType(const Expr *Arg) { @@ -5813,6 +5831,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, for (CallLifetimeEnd &LifetimeEnd : CallLifetimeEndAfterCall) LifetimeEnd.Emit(*this, /*Flags=*/{}); + for (const CallArgList::EndLifetimeInfo < : CallArgs.getLifetimeCleanups()) + EmitLifetimeEnd(LT.Size, LT.Addr); + if (!ReturnValue.isExternallyDestructed() && RetTy.isDestructedType() == QualType::DK_nontrivial_c_struct) pushDestroy(QualType::DK_nontrivial_c_struct, Ret.getAggregateAddress(), diff --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h index 65a7d8e832884..aab4a678e68e6 100644 --- a/clang/lib/CodeGen/CGCall.h +++ b/clang/lib/CodeGen/CGCall.h @@ -278,6 +278,11 @@ class CallArgList : public SmallVector { llvm::Instruction *IsActiveIP; }; + struct EndLifetimeInfo { + llvm::Value *Addr; + llvm::Value *Size; + }; + void add(RValue rvalue, QualType type) { push_back(CallArg(rvalue, type)); } void addUncopiedAggregate(LValue LV, QualType type) { @@ -294,6 +299,9 @@ class CallArgList : public SmallVector { CleanupsToDeactivate.insert(CleanupsToDeactivate.end(), other.CleanupsToDeactivate.begin(), other.CleanupsToDeactivate.end()); + LifetimeCleanups.insert(LifetimeCleanups.end(), + other.LifetimeCleanups.begin(), + other.LifetimeCleanups.end()); assert(!(StackBase && other.StackBase) && "can't merge stackbases"); if (!StackBase) StackBase = other.StackBase; @@ -333,6 +341,14 @@ class CallArgList : public SmallVector { /// memory. bool isUsingInAlloca() const { return StackBase; } + void addLifetimeCleanup(EndLifetimeInfo Info) { + LifetimeCleanups.push_back(Info); + } + + ArrayRef getLifetimeCleanups() const { + return LifetimeCleanups; + } + private: SmallVector Writebacks; @@ -341,6 +357,10 @@ class CallArgList : public SmallVector { /// occurs. SmallVector CleanupsToDeactivate; + /// Lifetime information needed to call llvm.lifetime.end for any temporary + /// argument allocas. + SmallVector LifetimeCleanups; + /// The stacksave call. It dominates all of the argument evaluation. llvm::CallInst *StackBase; }; diff --git a/clang/test/CodeGen/lifetime-call-temp.c b/clang/test/CodeGen/lifetime-call-temp.c new file mode 100644 index 0000000000000..fcc225aeb07a3 --- /dev/null +++ b/clang/test/CodeGen/lifetime-call-temp.c @@ -0,0 +1,81 @@ +// RUN: %clang -cc1 -triple x86_64-apple-macos -O1 -disable-llvm-passes %s -S -emit-llvm -o - | FileCheck %s --implicit-check-not=llvm.lifetime +// 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 +// 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 + +typedef struct { int x[100]; } aggregate; + +#ifdef __cplusplus +extern "C" { +#endif + +void takes_aggregate(aggregate); +aggregate gives_aggregate(); + +// CHECK-LABEL: define void @t1 +void t1() { + takes_aggregate(gives_aggregate()); + + // CHECK: [[AGGTMP:%.*]] = alloca %struct.aggregate, align 8 + // CHECK: call void @llvm.lifetime.start.p0(i64 400, ptr [[AGGTMP]]) + // CHECK: call void{{.*}} @gives_aggregate(ptr sret(%struct.aggregate) align 4 [[AGGTMP]]) + // CHECK: call void @takes_aggregate(ptr noundef byval(%struct.aggregate) align 8 [[AGGTMP]]) + // CHECK: call void @llvm.lifetime.end.p0(i64 400, ptr [[AGGTMP]]) +} + +// CHECK: declare {{.*}}llvm.lifetime.start +// CHECK: declare {{.*}}llvm.lifetime.end + +#ifdef __cplusplus +// CXX: define void @t2 +void t2() { + struct S { + S(aggregate) {} + }; + S{gives_aggregate()}; + + // CXX: [[AGG:%.*]] = alloca %struct.aggregate + // CXX: call void @llvm.lifetime.start.p0(i64 400, ptr + // CXX: call void @gives_aggregate(ptr sret(%struct.aggregate) align 4 [[AGG]]) + // CXX: call void @_ZZ2t2EN1SC1E9aggregate(ptr {{.*}}, ptr {{.*}} byval(%struct.aggregate) align 8 [[AGG]]) + // CXX: call void @llvm.lifetime.end.p0(i64 400, ptr +} + +struct Dtor { + ~Dtor(); +}; + +void takes_dtor(Dtor); +Dtor gives_dtor(); + +// CXX: define void @t3 +void t3() { + takes_dtor(gives_dtor()); + + // CXX-NOT @llvm.lifetime + // CXX: ret void +} + +#endif + +#ifdef __OBJC__ + +@interface X +-m:(aggregate)x; +@end + +// OBJC: define void @t4 +void t4(X *x) { + [x m: gives_aggregate()]; + + // OBJC: [[AGG:%.*]] = alloca %struct.aggregate + // OBJC: call void @llvm.lifetime.start.p0(i64 400, ptr + // OBJC: call void{{.*}} @gives_aggregate(ptr sret(%struct.aggregate) align 4 [[AGGTMP]]) + // OBJC: call {{.*}}@objc_msgSend + // OBJC: call void @llvm.lifetime.end.p0(i64 400, ptr +} + +#endif + +#ifdef __cplusplus +} +#endif diff --git a/clang/test/CodeGenCXX/amdgcn-call-with-aggarg.cpp b/clang/test/CodeGenCXX/amdgcn-call-with-aggarg.cpp new file mode 100644 index 0000000000000..9b598a48f6436 --- /dev/null +++ b/clang/test/CodeGenCXX/amdgcn-call-with-aggarg.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -O3 -disable-llvm-passes -o - %s | FileCheck %s + +struct A { + float x, y, z, w; +}; + +void foo(A a); + +// CHECK-LABEL: @_Z4testv +// CHECK: [[A:%.*]] = alloca [[STRUCT_A:%.*]], align 4, addrspace(5) +// CHECK-NEXT: [[AGG_TMP:%.*]] = alloca [[STRUCT_A]], align 4, addrspace(5) +// CHECK-NEXT: [[A_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[A]] to ptr +// CHECK-NEXT: [[AGG_TMP_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[AGG_TMP]] to ptr +// CHECK-NEXT: call void @llvm.lifetime.start.p5(i64 16, ptr addrspace(5) [[A]]) #[[ATTR4:[0-9]+]] +// CHECK-NEXT: call void @llvm.lifetime.start.p5(i64 16, ptr addrspace(5) [[AGG_TMP]]) #[[ATTR4]] +void test() { + A a; + foo(a); +} diff --git a/clang/test/CodeGenCXX/stack-reuse-miscompile.cpp b/clang/test/CodeGenCXX/stack-reuse-miscompile.cpp index dbeea5e32cfb8..45a1d3ba42c5a 100644 --- a/clang/test/CodeGenCXX/stack-reuse-miscompile.cpp +++ b/clang/test/CodeGenCXX/stack-reuse-miscompile.cpp @@ -26,6 +26,8 @@ const char * f(S s) // CHECK: [[T2:%.*]] = alloca %class.T, align 4 // CHECK: [[T3:%.*]] = alloca %class.T, align 4 // +// CHECK: [[AGG:%.*]] = alloca %class.S, align 4 +// // FIXME: We could defer starting the lifetime of the return object of concat // until the call. // CHECK: call void @llvm.lifetime.start.p0(i64 16, ptr [[T1]]) @@ -34,7 +36,9 @@ const char * f(S s) // CHECK: [[T4:%.*]] = call noundef ptr @_ZN1TC1EPKc(ptr {{[^,]*}} [[T2]], ptr noundef @.str) // // CHECK: call void @llvm.lifetime.start.p0(i64 16, ptr [[T3]]) +// CHECK: call void @llvm.lifetime.start.p0(i64 8, ptr [[AGG]]) // CHECK: [[T5:%.*]] = call noundef ptr @_ZN1TC1E1S(ptr {{[^,]*}} [[T3]], [2 x i32] %{{.*}}) +// CHECK: call void @llvm.lifetime.end.p0(i64 8, ptr [[AGG]] // // CHECK: call void @_ZNK1T6concatERKS_(ptr sret(%class.T) align 4 [[T1]], ptr {{[^,]*}} [[T2]], ptr noundef nonnull align 4 dereferenceable(16) [[T3]]) // CHECK: [[T6:%.*]] = call noundef ptr @_ZNK1T3strEv(ptr {{[^,]*}} [[T1]]) diff --git a/clang/test/CodeGenCoroutines/pr59181.cpp b/clang/test/CodeGenCoroutines/pr59181.cpp index 80f4634db2521..2527176d0992b 100644 --- a/clang/test/CodeGenCoroutines/pr59181.cpp +++ b/clang/test/CodeGenCoroutines/pr59181.cpp @@ -49,6 +49,7 @@ void foo() { } // CHECK: cleanup.cont:{{.*}} +// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 4, ptr [[AGG:%agg.tmp[0-9]+]]) // CHECK-NEXT: load i8 // CHECK-NEXT: trunc // CHECK-NEXT: store i1 false @@ -56,5 +57,7 @@ void foo() { // CHECK: await.suspend:{{.*}} // CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF]]) +// CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[AGG]]) // CHECK: call void @_ZZN4Task12promise_type15await_transformES_EN10Suspension13await_suspendESt16coroutine_handleIvE +// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[AGG2:%agg.tmp[0-9]+]]) // CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[REF]])