Skip to content

Commit

Permalink
[Coroutine][Sema] Tighten the lifetime of symmetric transfer returned…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
lxfind committed Sep 11, 2020
1 parent 9a2bab5 commit df477db
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 1 deletion.
4 changes: 4 additions & 0 deletions clang/lib/Sema/SemaCoroutine.cpp
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenCoroutines/Inputs/coroutine.h
Expand Up @@ -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); }
Expand Down
53 changes: 53 additions & 0 deletions 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 %{{.*}})

0 comments on commit df477db

Please sign in to comment.