Author: ericwf Date: Mon Jan 29 15:52:57 2018 New Revision: 323712 URL: http://llvm.org/viewvc/llvm-project?rev=323712&view=rev Log: [coroutines] Fix application of NRVO to Coroutine "Gro" or return object.
Summary: Fix NRVO for Gro variable. Previously, we only marked the GRO declaration as an NRVO variable when its QualType and the function return's QualType matched exactly (using operator==). However, this was incorrect for two reasons: 1. We were marking non-class types, such as ints, as being NRVO variables. 2. We failed to handle cases where the canonical types were the same, but the actual `QualType` objects were different. For example, if one was represented by a typedef. (Example: https://godbolt.org/g/3UFgsL) This patch fixes these bugs by marking the Gro variable as supporting NRVO only when `BuildReturnStmt` marks the Gro variable as a coroutine candidate. Reviewers: rsmith, GorNishanov, nicholas Reviewed By: GorNishanov Subscribers: majnemer, cfe-commits Differential Revision: https://reviews.llvm.org/D42343 Added: cfe/trunk/test/CodeGenCoroutines/coro-gro-nrvo.cpp Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=323712&r1=323711&r2=323712&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original) +++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Mon Jan 29 15:52:57 2018 @@ -1316,10 +1316,6 @@ bool CoroutineStmtBuilder::makeGroDeclAn if (Res.isInvalid()) return false; - if (GroType == FnRetType) { - GroDecl->setNRVOVariable(true); - } - S.AddInitializerToDecl(GroDecl, Res.get(), /*DirectInit=*/false); @@ -1343,6 +1339,8 @@ bool CoroutineStmtBuilder::makeGroDeclAn noteMemberDeclaredHere(S, ReturnValue, Fn); return false; } + if (cast<clang::ReturnStmt>(ReturnStmt.get())->getNRVOCandidate() == GroDecl) + GroDecl->setNRVOVariable(true); this->ReturnStmt = ReturnStmt.get(); return true; Modified: cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp?rev=323712&r1=323711&r2=323712&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp (original) +++ cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp Mon Jan 29 15:52:57 2018 @@ -173,6 +173,7 @@ struct std::experimental::coroutine_trai // CHECK-LABEL: f4( extern "C" int f4(promise_on_alloc_failure_tag) { // CHECK: %[[RetVal:.+]] = alloca i32 + // CHECK: %[[Gro:.+]] = alloca i32 // CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16 // CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64() // CHECK: %[[MEM:.+]] = call i8* @_ZnwmRKSt9nothrow_t(i64 %[[SIZE]], %"struct.std::nothrow_t"* dereferenceable(1) @_ZStL7nothrow) @@ -186,7 +187,12 @@ extern "C" int f4(promise_on_alloc_failu // CHECK: [[OKBB]]: // CHECK: %[[OkRet:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type17get_return_objectEv( - // CHECK: store i32 %[[OkRet]], i32* %[[RetVal]] + // CHECK: store i32 %[[OkRet]], i32* %[[Gro]] + + // CHECK: coro.ret: + // CHECK: %[[Tmp1:.*]] = load i32, i32* %[[Gro]] + // CHECK-NEXT: store i32 %[[Tmp1]], i32* %[[RetVal]] + // CHECK-NEXT: br label %[[RetBB]] // CHECK: [[RetBB]]: // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]], align 4 Added: cfe/trunk/test/CodeGenCoroutines/coro-gro-nrvo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-gro-nrvo.cpp?rev=323712&view=auto ============================================================================== --- cfe/trunk/test/CodeGenCoroutines/coro-gro-nrvo.cpp (added) +++ cfe/trunk/test/CodeGenCoroutines/coro-gro-nrvo.cpp Mon Jan 29 15:52:57 2018 @@ -0,0 +1,80 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s + +#include "Inputs/coroutine.h" + +using namespace std::experimental; + +namespace std { + +struct nothrow_t {}; +constexpr nothrow_t nothrow = {}; + +} // end namespace std + +// Required when get_return_object_on_allocation_failure() is defined by +// the promise. +void* operator new(__SIZE_TYPE__ __sz, const std::nothrow_t&) noexcept; +void operator delete(void* __p, const std::nothrow_t&) noexcept; + + +template <class RetObject> +struct promise_type { + RetObject get_return_object(); + suspend_always initial_suspend(); + suspend_never final_suspend(); + void return_void(); + static void unhandled_exception(); +}; + +struct coro { + using promise_type = promise_type<coro>; + coro(coro const&); + struct Impl; + Impl *impl; +}; + +// Verify that the NRVO is applied to the Gro object. +// CHECK-LABEL: define void @_Z1fi(%struct.coro* noalias sret %agg.result, i32) +coro f(int) { +// CHECK: coro.init: +// CHECK: store i1 false, i1* %gro.active +// CHECK-NEXT: call void @{{.*get_return_objectEv}}(%struct.coro* sret %agg.result +// CHECK-NEXT: store i1 true, i1* %gro.active + co_return; +} + + +template <class RetObject> +struct promise_type_with_on_alloc_failure { + static RetObject get_return_object_on_allocation_failure(); + RetObject get_return_object(); + suspend_always initial_suspend(); + suspend_never final_suspend(); + void return_void(); + static void unhandled_exception(); +}; + +struct coro_two { + using promise_type = promise_type_with_on_alloc_failure<coro_two>; + coro_two(coro_two const&); + struct Impl; + Impl *impl; +}; + +// Verify that the NRVO is applied to the Gro object. +// CHECK-LABEL: define void @_Z1hi(%struct.coro_two* noalias sret %agg.result, i32) + coro_two h(int) { + +// CHECK: coro.ret.on.failure: +// CHECK-NEXT: call void @{{.*get_return_object_on_allocation_failureEv}}(%struct.coro_two* sret %agg.result +// CHECK-NEXT: br label %[[RetLabel:.*]] + +// CHECK: coro.init: +// CHECK: store i1 false, i1* %gro.active +// CHECK-NEXT: call void @{{.*get_return_objectEv}}(%struct.coro_two* sret %agg.result +// CHECK-NEXT: store i1 true, i1* %gro.active + +// CHECK: [[RetLabel]]: +// CHECK-NEXT: ret void + co_return; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
