[clang] df477db - [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2020-09-11T13:35:37-07:00
New Revision: df477db5f9e0ea2a4890040b65002d93e33209b0

URL: https://github.com/llvm/llvm-project/commit/df477db5f9e0ea2a4890040b65002d93e33209b0
DIFF: https://github.com/llvm/llvm-project/commit/df477db5f9e0ea2a4890040b65002d93e33209b0.diff

LOG: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle

In generating the code for symmetric transfer, a temporary object is created to store the returned handle from await_suspend() call of the awaiter. Previously this temp won't be cleaned up until very later, which ends up causing this temp to be spilled to the heap. However, we know that this temp will no longer be needed after the coro_resume call. We can clean it up right after.

Differential Revision: https://reviews.llvm.org/D87470

Added: 
    clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp

Modified: 
    clang/lib/Sema/SemaCoroutine.cpp
    clang/test/CodeGenCoroutines/Inputs/coroutine.h

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 990ab2633520..565f907e05b2 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -398,6 +398,10 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
            diag::warn_coroutine_handle_address_invalid_return_type)
         << JustAddress->getType();
 
+  // The coroutine handle used to obtain the address is no longer needed
+  // at this point, clean it up to avoid unnecessarily long lifetime which
+  // could lead to unnecessary spilling.
+  JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
                           JustAddress);
 }

diff  --git a/clang/test/CodeGenCoroutines/Inputs/coroutine.h b/clang/test/CodeGenCoroutines/Inputs/coroutine.h
index 5cc78a4904aa..2dd1ce7e9735 100644
--- a/clang/test/CodeGenCoroutines/Inputs/coroutine.h
+++ b/clang/test/CodeGenCoroutines/Inputs/coroutine.h
@@ -15,7 +15,7 @@ template <> struct coroutine_handle<void> {
     return me;
   }
   void operator()() { resume(); }
-  void *address() const { return ptr; }
+  void *address() const noexcept { return ptr; }
   void resume() const { __builtin_coro_resume(ptr); }
   void destroy() const { __builtin_coro_destroy(ptr); }
   bool done() const { return __builtin_coro_done(ptr); }

diff  --git a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp b/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
new file mode 100644
index 000000000000..09205799c3f7
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o -
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+struct detached_task {
+  struct promise_type {
+    detached_task get_return_object() noexcept {
+      return detached_task{coro::coroutine_handle<promise_type>::from_promise(*this)};
+    }
+
+    void return_void() noexcept {}
+
+    struct final_awaiter {
+      bool await_ready() noexcept { return false; }
+      coro::coroutine_handle<> await_suspend(coro::coroutine_handle<promise_type> h) noexcept {
+        h.destroy();
+        return {};
+      }
+      void await_resume() noexcept {}
+    };
+
+    void unhandled_exception() noexcept {}
+
+    final_awaiter final_suspend() noexcept { return {}; }
+
+    coro::suspend_always initial_suspend() noexcept { return {}; }
+  };
+
+  ~detached_task() {
+    if (coro_) {
+      coro_.destroy();
+      coro_ = {};
+    }
+  }
+
+  void start() && {
+    auto tmp = coro_;
+    coro_ = {};
+    tmp.resume();
+  }
+
+  coro::coroutine_handle<promise_type> coro_;
+};
+
+detached_task foo() {
+  co_return;
+}
+
+// check that the lifetime of the coroutine handle used to obtain the address ended right away.
+// CHECK:       %{{.*}} = call i8* @{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* nonnull %{{.*}})
+// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})