[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

(I am working on rebasing this; tests need updating for opaque ptr)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added a subscriber: lei.
jroelofs added a comment.

In D74094#4559037 , @dexonsmith wrote:

> In D74094#4554327 , @foad wrote:
>
>> Hi @erik.pilkington, I see this got reverted:
>
> I'm not sure if @erik.pilkington is still watching Phabricator, but in any 
> case I think (like me) he no longer has rdar access. Since this was a Linux 
> PPC bot, it's possible Erik didn't get far investigating the failure. Three 
> years later, the sands may have shifted substantially... maybe the best 
> option to is rebase and investigate the new failures (if any).
>
> @akyrtzi @arphaman @jroelofs, is one of you available to check 
> rdar://58552124, in case it has extra context from the PPC bot failure?

I found rdar://59400672&60009515, which track the revert and reminder to 
investigate.  Neither go into any more detail on how/why it broke that bot, 
unfortunately.  I also found 
https://reviews.llvm.org/rGfafc6e4fdf3673dcf557d6c8ae0c0a4bb3184402 has a tiny 
bit more context than this review.  Maybe @lei can share the reproducer 
mentioned in that thread?  Otherwise, as Duncan said, I think your best bet is 
to try to re-land and see what breaks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added subscribers: arphaman, akyrtzi, jroelofs.
dexonsmith added a comment.

In D74094#4554327 , @foad wrote:

> Hi @erik.pilkington, I see this got reverted:

I'm not sure if @erik.pilkington is still watching Phabricator, but in any case 
I think (like me) he no longer has rdar access. Since this was a Linux PPC bot, 
it's possible Erik didn't get far investigating the failure. Three years later, 
the sands may have shifted substantially... maybe the best option to is rebase 
and investigate the new failures (if any).

@akyrtzi @arphaman @jroelofs, is one of you available to check rdar://58552124, 
in case it has extra context from the PPC bot failure?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D74094#4554327 , @foad wrote:

> Hi @erik.pilkington, I see this got reverted:
>
>   commit e26c24b849211f35a988d001753e0cd15e4a9d7b
>   Author: Erik Pilkington 
>   Date:   Wed Feb 12 12:02:58 2020 -0800
>   
>   Revert "[IRGen] Emit lifetime intrinsics around temporary aggregate 
> argument allocas"
>   
>   This reverts commit fafc6e4fdf3673dcf557d6c8ae0c0a4bb3184402.
>   
>   Should fix ppc stage2 failure: 
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/23546
>
> Do you have any more info on the "ppc stage2 failure"? I'd like to pursue 
> something like this patch to get more accurate lifetime markers for 
> temporaries, so that LLVM stack slot coloring can do a better job, and we get 
> smaller stack usage. This is prompted by 
> https://github.com/llvm/llvm-project/issues/41896

I too am very interested, as this was flagged as a potential help for 
https://github.com/llvm/llvm-project/issues/41896.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.
Herald added a project: All.

Hi @erik.pilkington, I see this got reverted:

  commit e26c24b849211f35a988d001753e0cd15e4a9d7b
  Author: Erik Pilkington 
  Date:   Wed Feb 12 12:02:58 2020 -0800
  
  Revert "[IRGen] Emit lifetime intrinsics around temporary aggregate 
argument allocas"
  
  This reverts commit fafc6e4fdf3673dcf557d6c8ae0c0a4bb3184402.
  
  Should fix ppc stage2 failure: 
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/23546

Do you have any more info on the "ppc stage2 failure"? I'd like to pursue 
something like this patch to get more accurate lifetime markers for 
temporaries, so that LLVM stack slot coloring can do a better job, and we get 
smaller stack usage. This is prompted by 
https://github.com/llvm/llvm-project/issues/41896


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Erik Pilkington via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfafc6e4fdf36: [IRGen] Emit lifetime intrinsics around 
temporary aggregate argument allocas (authored by erik.pilkington).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h
  clang/test/CodeGen/lifetime-call-temp.c
  clang/test/CodeGenCXX/stack-reuse-miscompile.cpp

Index: clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
===
--- clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -26,6 +26,8 @@
 // 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: [[T1i8:%.*]] = bitcast %class.T* [[T1]] to i8*
@@ -37,8 +39,15 @@
 //
 // CHECK: [[T3i8:%.*]] = bitcast %class.T* [[T3]] to i8*
 // CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T3i8]])
+//
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* [[AGG]] to i8*
+// CHECK: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
 //
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* {{.*}} to i8*
+// CHECK: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: call void @_ZNK1T6concatERKS_(%class.T* sret [[T1]], %class.T* [[T2]], %class.T* dereferenceable(16) [[T3]])
 // CHECK: [[T6:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
 //
Index: clang/test/CodeGen/lifetime-call-temp.c
===
--- /dev/null
+++ clang/test/CodeGen/lifetime-call-temp.c
@@ -0,0 +1,83 @@
+// 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 - | 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: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.start.p0i8(i64 400, i8* [[CAST]])
+  // CHECK: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // CHECK: call void @takes_aggregate(%struct.aggregate* byval(%struct.aggregate) align 8 [[AGGTMP]])
+  // CHECK: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.end.p0i8(i64 400, i8* [[CAST]])
+}
+
+// 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.p0i8(i64 400, i8*
+  // CXX: call void @gives_aggregate(%struct.aggregate* sret [[AGG]])
+  // CXX: call void @_ZZ2t2EN1SC1E9aggregate(%struct.S* {{.*}}, %struct.aggregate* byval(%struct.aggregate) align 8 [[AGG]])
+  // CXX: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+
+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.p0i8(i64 400, i8*
+  // OBJC: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // OBJC: call {{.*}}@objc_msgSend
+  // OBJC: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+
+#endif
+
+#ifdef __cplusplus
+}
+#endif
Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -283,6 +283,11 @@
 llvm::Instruction *IsActiveIP;
   };
 
+  struct EndLifetimeInfo {
+llvm::Value *Addr;
+llvm::Value *Size;
+  };
+
   void add(RValue rvalue, QualType type) { push_back(CallArg(rvalue, 

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3687
 
-  args.add(EmitAnyExprToTemp(E), type);
 }

thegameg wrote:
> Is there any other use of `EmitAnyExprToTemp` that can benefit from this?
Right now, the only other user of that function is 
`CodeGenFunction::EmitStmtExprLValue`, which doesn't seem all that interesting. 
There are a lot of places that do create temporary allocas that would be worth 
looking into in the future though. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3687
 
-  args.add(EmitAnyExprToTemp(E), type);
 }

Is there any other use of `EmitAnyExprToTemp` that can benefit from this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 243243.
erik.pilkington marked 2 inline comments as done.
erik.pilkington added a comment.

Disable the optimization for non-trivially destructible types.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h
  clang/test/CodeGen/lifetime-call-temp.c
  clang/test/CodeGenCXX/stack-reuse-miscompile.cpp

Index: clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
===
--- clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -26,6 +26,8 @@
 // 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: [[T1i8:%.*]] = bitcast %class.T* [[T1]] to i8*
@@ -37,8 +39,15 @@
 //
 // CHECK: [[T3i8:%.*]] = bitcast %class.T* [[T3]] to i8*
 // CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T3i8]])
+//
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* [[AGG]] to i8*
+// CHECK: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
 //
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* {{.*}} to i8*
+// CHECK: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: call void @_ZNK1T6concatERKS_(%class.T* sret [[T1]], %class.T* [[T2]], %class.T* dereferenceable(16) [[T3]])
 // CHECK: [[T6:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
 //
Index: clang/test/CodeGen/lifetime-call-temp.c
===
--- /dev/null
+++ clang/test/CodeGen/lifetime-call-temp.c
@@ -0,0 +1,83 @@
+// 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 - | 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: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.start.p0i8(i64 400, i8* [[CAST]])
+  // CHECK: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // CHECK: call void @takes_aggregate(%struct.aggregate* byval(%struct.aggregate) align 8 [[AGGTMP]])
+  // CHECK: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.end.p0i8(i64 400, i8* [[CAST]])
+}
+
+// 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.p0i8(i64 400, i8*
+  // CXX: call void @gives_aggregate(%struct.aggregate* sret [[AGG]])
+  // CXX: call void @_ZZ2t2EN1SC1E9aggregate(%struct.S* {{.*}}, %struct.aggregate* byval(%struct.aggregate) align 8 [[AGG]])
+  // CXX: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+
+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.p0i8(i64 400, i8*
+  // OBJC: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // OBJC: call {{.*}}@objc_msgSend
+  // OBJC: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+
+#endif
+
+#ifdef __cplusplus
+}
+#endif
Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -283,6 +283,11 @@
 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) {
@@ -299,6 

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3697
+
+  args.add(EmitAnyExpr(E, ArgSlot), type);
 }

erik.pilkington wrote:
> rjmccall wrote:
> > If the argument type has a C++ destructor, will we end its lifetime before 
> > we call destructors at the end of the full-expression?
> Yeah, this is broken :/
Well, we could at least do it for trivially-destructible types.  That probably 
needs to include all kinds of non-trivial destructibility, not just  C++, 
though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington planned changes to this revision.
erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3697
+
+  args.add(EmitAnyExpr(E, ArgSlot), type);
 }

rjmccall wrote:
> If the argument type has a C++ destructor, will we end its lifetime before we 
> call destructors at the end of the full-expression?
Yeah, this is broken :/


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3697
+
+  args.add(EmitAnyExpr(E, ArgSlot), type);
 }

If the argument type has a C++ destructor, will we end its lifetime before we 
call destructors at the end of the full-expression?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, thegameg, rsmith.
Herald added subscribers: ributzka, dexonsmith, jkorous.

These temporaries are only used in the callee, and their memory can be reused 
after the call is complete.

rdar://58552124


https://reviews.llvm.org/D74094

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h
  clang/test/CodeGen/lifetime-call-temp.c
  clang/test/CodeGenCXX/stack-reuse-miscompile.cpp

Index: clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
===
--- clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -26,6 +26,8 @@
 // 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: [[T1i8:%.*]] = bitcast %class.T* [[T1]] to i8*
@@ -37,8 +39,15 @@
 //
 // CHECK: [[T3i8:%.*]] = bitcast %class.T* [[T3]] to i8*
 // CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T3i8]])
+//
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* [[AGG]] to i8*
+// CHECK: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
 //
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* {{.*}} to i8*
+// CHECK: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: call void @_ZNK1T6concatERKS_(%class.T* sret [[T1]], %class.T* [[T2]], %class.T* dereferenceable(16) [[T3]])
 // CHECK: [[T6:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
 //
Index: clang/test/CodeGen/lifetime-call-temp.c
===
--- /dev/null
+++ clang/test/CodeGen/lifetime-call-temp.c
@@ -0,0 +1,67 @@
+// 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 - | 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: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.start.p0i8(i64 400, i8* [[CAST]])
+  // CHECK: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // CHECK: call void @takes_aggregate(%struct.aggregate* byval(%struct.aggregate) align 8 [[AGGTMP]])
+  // CHECK: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.end.p0i8(i64 400, i8* [[CAST]])
+}
+
+// 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.p0i8(i64 400, i8*
+  // CXX: call void @gives_aggregate(%struct.aggregate* sret [[AGG]])
+  // CXX: call void @_ZZ2t2EN1SC1E9aggregate(%struct.S* {{.*}}, %struct.aggregate* byval(%struct.aggregate) align 8 [[AGG]])
+  // CXX: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+#endif
+
+#ifdef __OBJC__
+
+@interface X
+-m:(aggregate)x;
+@end
+
+// OBJC: define void @t3
+void t3(X *x) {
+  [x m: gives_aggregate()];
+
+  // OBJC: [[AGG:%.*]] = alloca %struct.aggregate
+  // OBJC: call void @llvm.lifetime.start.p0i8(i64 400, i8*
+  // OBJC: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // OBJC: call {{.*}}@objc_msgSend
+  // OBJC: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+
+#endif
+
+#ifdef __cplusplus
+}
+#endif
Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -283,6 +283,11 @@
 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) {
@@ -299,6 +304,9 @@
 CleanupsToDeactivate.insert(CleanupsToDeactivate.end(),
 other.CleanupsToDeactivate.begin(),