[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-07-03 Thread JunMa via Phabricator via cfe-commits 
junparser added a comment.

In D82314#2125713 , @lxfind wrote:

> In D82314#2124662 , @junparser wrote:
>
> > In D82314#2124661 , @junparser 
> > wrote:
> >
> > > @lxfind This patch causes some mismatch when variable is used in both 
> > > resume and destroy function. Besides, we should move this patch and the 
> > > check in buildCoroutineFrame.
> >
> >
> > @lxfind, Would you try to fix this？ If you do not have time, then I'll try 
> > do this. Thanks
>
>
> Could you please help take a look, if you have a local repro? Thanks!


of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314



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

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-07-01 Thread Xun Li via Phabricator via cfe-commits 
lxfind added a comment.

In D82314#2124662 , @junparser wrote:

> In D82314#2124661 , @junparser wrote:
>
> > @lxfind This patch causes some mismatch when variable is used in both 
> > resume and destroy function. Besides, we should move this patch and the 
> > check in buildCoroutineFrame.
>
>
> @lxfind, Would you try to fix this？ If you do not have time, then I'll try do 
> this. Thanks


Could you please help take a look, if you have a local repro? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314



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

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-07-01 Thread JunMa via Phabricator via cfe-commits 
junparser added a comment.

@lxfind This patch causes some mismatch when variable is used in both resume 
and destroy function. Besides, we should move this patch and the check in 
buildCoroutineFrame.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314



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

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-07-01 Thread JunMa via Phabricator via cfe-commits 
junparser added a comment.

In D82314#2124661 , @junparser wrote:

> @lxfind This patch causes some mismatch when variable is used in both resume 
> and destroy function. Besides, we should move this patch and the check in 
> buildCoroutineFrame.


@lxfind, Would you try to fix this？ If you do not have time, then I'll try do 
this. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314



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

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-06-29 Thread JunMa via Phabricator via cfe-commits 
junparser accepted this revision.
junparser added a comment.
This revision is now accepted and ready to land.

LGTM， Thank you！


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314



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

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-06-29 Thread Xun Li via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8755b6378c2: [Coroutines] Optimize the lifespan of 
temporary co_await object (authored by lxfind).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314

Files:
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-split-02.ll
  llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll

Index: llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll
+++ llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll
@@ -1,6 +1,5 @@
-; Tests that coro-split can handle the case when a code after coro.suspend uses
-; a value produces between coro.save and coro.suspend (%Result.i19)
-; and checks whether stray coro.saves are properly removed
+; Tests that coro-split will optimize the lifetime.start maker of each local variable,
+; sink them to the places closest to the actual use.
 ; RUN: opt < %s -coro-split -S | FileCheck %s
 ; RUN: opt < %s -passes=coro-split -S | FileCheck %s
 
@@ -15,6 +14,9 @@
 entry:
   %ref.tmp7 = alloca %"struct.lean_future::Awaiter", align 8
   %testval = alloca i32
+  %cast = bitcast i32* %testval to i8*
+  ; lifetime of %testval starts here, but not used until await.ready.
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %alloc = call i8* @malloc(i64 16) #3
   %vFrame = call noalias nonnull i8* @llvm.coro.begin(token %id, i8* %alloc)
@@ -29,8 +31,8 @@
 await.ready:
   %StrayCoroSave = call token @llvm.coro.save(i8* null)
   %val = load i32, i32* %Result.i19
-  %cast = bitcast i32* %testval to i8*
-  call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
+  %test = load i32, i32* %testval
+  call void @print(i32 %test)
   call void @llvm.lifetime.end.p0i8(i64 4, i8*  %cast)
   call void @print(i32 %val)
   br label %exit
@@ -40,14 +42,15 @@
 }
 
 ; CHECK-LABEL: @a.resume(
-; CHECK: %testval = alloca i32
-; CHECK: getelementptr inbounds %a.Frame
+; CHECK: %testval = alloca i32, align 4
+; CHECK-NEXT:getelementptr inbounds %a.Frame
 ; CHECK-NEXT:getelementptr inbounds %"struct.lean_future::Awaiter"
-; CHECK-NOT: call token @llvm.coro.save(i8* null)
+; CHECK-NEXT:%cast1 = bitcast i32* %testval to i8*
 ; CHECK-NEXT:%val = load i32, i32* %Result
-; CHECK-NEXT:%cast = bitcast i32* %testval to i8*
-; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
-; CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 4, i8* %cast)
+; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast1)
+; CHECK-NEXT:%test = load i32, i32* %testval
+; CHECK-NEXT:call void @print(i32 %test)
+; CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 4, i8* %cast1)
 ; CHECK-NEXT:call void @print(i32 %val)
 ; CHECK-NEXT:ret void
 
Index: llvm/test/Transforms/Coroutines/coro-split-02.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-02.ll
+++ llvm/test/Transforms/Coroutines/coro-split-02.ll
@@ -31,6 +31,8 @@
   %val = load i32, i32* %Result.i19
   %cast = bitcast i32* %testval to i8*
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
+  %test = load i32, i32* %testval
+  call void @print(i32 %test)
   call void @llvm.lifetime.end.p0i8(i64 4, i8*  %cast)
   call void @print(i32 %val)
   br label %exit
@@ -47,6 +49,8 @@
 ; CHECK-NEXT:%val = load i32, i32* %Result
 ; CHECK-NEXT:%cast = bitcast i32* %testval to i8*
 ; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
+; CHECK-NEXT:%test = load i32, i32* %testval
+; CHECK-NEXT:call void @print(i32 %test)
 ; CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 4, i8* %cast)
 ; CHECK-NEXT:call void @print(i32 %val)
 ; CHECK-NEXT:ret void
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -75,7 +75,7 @@
 
 namespace {
 
-/// A little helper class for building 
+/// A little helper class for building
 class CoroCloner {
 public:
   enum class Kind {
@@ -563,7 +563,7 @@
   // In the original function, the AllocaSpillBlock is a block immediately
   // following the allocation of the frame object which defines GEPs for
   // all the allocas that have been moved into the frame, and it ends by
-  // branching to the original beginning of the coroutine.  Make this 
+  // branching to the original beginning of the coroutine.  Make this
   // the entry block of the cloned function.
   auto *Entry = cast(VMap[Shape.AllocaSpillBlock]);
   auto *OldEntry = &NewF->getEntryBlock();
@@ -1239,6 +1239,103

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-06-29 Thread Xun Li via Phabricator via cfe-commits 
lxfind updated this revision to Diff 273946.
lxfind added a comment.

Rebase before landing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314

Files:
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-split-02.ll
  llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll

Index: llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll
+++ llvm/test/Transforms/Coroutines/coro-split-sink-lifetime.ll
@@ -1,6 +1,5 @@
-; Tests that coro-split can handle the case when a code after coro.suspend uses
-; a value produces between coro.save and coro.suspend (%Result.i19)
-; and checks whether stray coro.saves are properly removed
+; Tests that coro-split will optimize the lifetime.start maker of each local variable,
+; sink them to the places closest to the actual use.
 ; RUN: opt < %s -coro-split -S | FileCheck %s
 ; RUN: opt < %s -passes=coro-split -S | FileCheck %s
 
@@ -15,6 +14,9 @@
 entry:
   %ref.tmp7 = alloca %"struct.lean_future::Awaiter", align 8
   %testval = alloca i32
+  %cast = bitcast i32* %testval to i8*
+  ; lifetime of %testval starts here, but not used until await.ready.
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %alloc = call i8* @malloc(i64 16) #3
   %vFrame = call noalias nonnull i8* @llvm.coro.begin(token %id, i8* %alloc)
@@ -29,8 +31,8 @@
 await.ready:
   %StrayCoroSave = call token @llvm.coro.save(i8* null)
   %val = load i32, i32* %Result.i19
-  %cast = bitcast i32* %testval to i8*
-  call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
+  %test = load i32, i32* %testval
+  call void @print(i32 %test)
   call void @llvm.lifetime.end.p0i8(i64 4, i8*  %cast)
   call void @print(i32 %val)
   br label %exit
@@ -40,14 +42,15 @@
 }
 
 ; CHECK-LABEL: @a.resume(
-; CHECK: %testval = alloca i32
-; CHECK: getelementptr inbounds %a.Frame
+; CHECK: %testval = alloca i32, align 4
+; CHECK-NEXT:getelementptr inbounds %a.Frame
 ; CHECK-NEXT:getelementptr inbounds %"struct.lean_future::Awaiter"
-; CHECK-NOT: call token @llvm.coro.save(i8* null)
+; CHECK-NEXT:%cast1 = bitcast i32* %testval to i8*
 ; CHECK-NEXT:%val = load i32, i32* %Result
-; CHECK-NEXT:%cast = bitcast i32* %testval to i8*
-; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
-; CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 4, i8* %cast)
+; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast1)
+; CHECK-NEXT:%test = load i32, i32* %testval
+; CHECK-NEXT:call void @print(i32 %test)
+; CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 4, i8* %cast1)
 ; CHECK-NEXT:call void @print(i32 %val)
 ; CHECK-NEXT:ret void
 
Index: llvm/test/Transforms/Coroutines/coro-split-02.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-02.ll
+++ llvm/test/Transforms/Coroutines/coro-split-02.ll
@@ -31,6 +31,8 @@
   %val = load i32, i32* %Result.i19
   %cast = bitcast i32* %testval to i8*
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
+  %test = load i32, i32* %testval
+  call void @print(i32 %test)
   call void @llvm.lifetime.end.p0i8(i64 4, i8*  %cast)
   call void @print(i32 %val)
   br label %exit
@@ -47,6 +49,8 @@
 ; CHECK-NEXT:%val = load i32, i32* %Result
 ; CHECK-NEXT:%cast = bitcast i32* %testval to i8*
 ; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* %cast)
+; CHECK-NEXT:%test = load i32, i32* %testval
+; CHECK-NEXT:call void @print(i32 %test)
 ; CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 4, i8* %cast)
 ; CHECK-NEXT:call void @print(i32 %val)
 ; CHECK-NEXT:ret void
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -75,7 +75,7 @@
 
 namespace {
 
-/// A little helper class for building 
+/// A little helper class for building
 class CoroCloner {
 public:
   enum class Kind {
@@ -563,7 +563,7 @@
   // In the original function, the AllocaSpillBlock is a block immediately
   // following the allocation of the frame object which defines GEPs for
   // all the allocas that have been moved into the frame, and it ends by
-  // branching to the original beginning of the coroutine.  Make this 
+  // branching to the original beginning of the coroutine.  Make this
   // the entry block of the cloned function.
   auto *Entry = cast(VMap[Shape.AllocaSpillBlock]);
   auto *OldEntry = &NewF->getEntryBlock();
@@ -1239,6 +1239,103 @@
   S.resize(N);
 }
 
+/// For every local variable that has lifetime intrinsics markers, we sink
+

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

2020-06-26 Thread Xun Li via Phabricator via cfe-commits 
lxfind added a comment.

In D82314#2117543 , @lewissbaker wrote:

> >   There are still room for improvements, in particular, GCC has a 4K frame 
> > for this function.
>
> I think GCC having a smaller coroutine frame is probably because it does not 
> yet implement the allocation-elision optimisation which inlines the nested 
> coroutine frame (which is 4K) into the parent coroutine frame.
>  I have not looked yet, but I suspect that you'll see within the 
> get_big_object2() code it will perform another heap allocation for the 
> get_big_object() frame.
>
> Until we have more general support for async RVO, I think 8K is probably as 
> good as we're going to be able to get (4K for this coroutine's promise and 4K 
> for child coroutine-frame and its promise).


Yes your observation is correct. It's due to the nesting of the child frame.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314



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