This is an archive of the discontinued LLVM Phabricator instance.

[C++20] [Coroutines] Implement return value optimization for get_return_object
ClosedPublic

Authored by ChuanqiXu on Jan 11 2022, 10:04 PM.

Details

Summary

This patch tries to implement RVO for coroutine's return object got from get_return_object.
From [dcl.fct.def.coroutine]/p7 we could know that the return value of get_return_object is either a reference or a prvalue. So it makes sense to do copy elision for the return value. The return object should be constructed directly into the storage where they would otherwise be copied/moved to.

I have tested folly and our internal workloads.

Diff Detail

Event Timeline

ChuanqiXu requested review of this revision.Jan 11 2022, 10:04 PM
ChuanqiXu created this revision.
ChuanqiXu edited the summary of this revision. (Show Details)Jan 12 2022, 11:52 PM
ChuanqiXu updated this revision to Diff 399578.Jan 13 2022, 1:05 AM
ChuanqiXu retitled this revision from [WIP] [C++20] [Coroutines] Implement return value optimization for get_return_object to [C++20] [Coroutines] Implement return value optimization for get_return_object.
ChuanqiXu edited the summary of this revision. (Show Details)

Update.

ChuanqiXu set the repository for this revision to rG LLVM Github Monorepo.
ChuanqiXu added inline comments.Jan 13 2022, 1:12 AM
clang/lib/CodeGen/CGCoroutine.cpp
481–485

Since we've deleted the gro variable, we could remove GetReturnObjectManager.

gentle ping~

junparser accepted this revision.Feb 15 2022, 12:01 AM

LGTM. Thanks!

clang/lib/CodeGen/CGCoroutine.cpp
645–647

can we just remove this?

This revision is now accepted and ready to land.Feb 15 2022, 12:01 AM
ChuanqiXu updated this revision to Diff 408761.Feb 15 2022, 1:23 AM

Update test.

clang/lib/CodeGen/CGCoroutine.cpp
645–647

We couldn't. Otherwise it would emit the return expression again.

junparser added inline comments.Feb 15 2022, 3:49 AM
clang/lib/CodeGen/CGCoroutine.cpp
648–649

I mean, remove the if statements here since the retuen expr is null.

ChuanqiXu added inline comments.Feb 15 2022, 6:02 PM
clang/lib/CodeGen/CGCoroutine.cpp
648–649

We couldn't. Since we need emit ret instruction if needed.

junparser added inline comments.Feb 15 2022, 6:25 PM
clang/lib/CodeGen/CGCoroutine.cpp
648–649

make sense to me.

alkis added a subscriber: alkis.Jan 26 2023, 5:29 AM

Does this change the semantics of get_return_value() such that it is eagerly converted to R the return value of the coro? AFAIU before this change one could return T convertible to R and the conversion would happen late - after final_suspend.

Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2023, 5:29 AM

Does this change the semantics of get_return_value() such that it is eagerly converted to R the return value of the coro? AFAIU before this change one could return T convertible to R and the conversion would happen late - after final_suspend.

Yes, this changed the semantics. But the spec is ambiguous for this. Wg21 is tracking this in https://cplusplus.github.io/CWG/issues/2563.html.