[coroutines] Fix application of NRVO to Coroutine "Gro" or return object.

[EricWF]

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.

[majnemer]

test/CodeGenCoroutines/coro-gro-nrvo.cpp

18

SizeT -> __SIZE_TYPE__ ?

[EricWF]

test/CodeGenCoroutines/coro-gro-nrvo.cpp

18

SGTM.

[EricWF]

• Address inline comments in test.

[EricWF]

• Use a better formulation for detecting when the Gro should be an NRVO variable.

[GorNishanov]

LGTM

[EricWF]

I had to revert due to failing tests when using a non-assert Clang build.

This change fixes those tests to no longer depend on label names.

lib/Sema/SemaCoroutine.cpp

                                                      this->ReturnValue);
   if (Res.isInvalid())
     return false;
 
   Res = S.ActOnFinishFullExpr(Res.get());
   if (Res.isInvalid())
     return false;
 
-  if (GroType == FnRetType) {
-    GroDecl->setNRVOVariable(true);
-  }
-
   S.AddInitializerToDecl(GroDecl, Res.get(),
                          /*DirectInit=*/false);
 
   S.FinalizeDeclaration(GroDecl);
 
   // Form a declaration statement for the return declaration, so that AST
   // visitors can more easily find it.
   StmtResult GroDeclStmt =
       S.ActOnDeclStmt(S.ConvertDeclToDeclGroup(GroDecl), Loc, Loc);
   if (GroDeclStmt.isInvalid())
     return false;
 
   this->ResultDecl = GroDeclStmt.get();
 
   ExprResult declRef = S.BuildDeclRefExpr(GroDecl, GroType, VK_LValue, Loc);
   if (declRef.isInvalid())
     return false;
 
   StmtResult ReturnStmt = S.BuildReturnStmt(Loc, declRef.get());
   if (ReturnStmt.isInvalid()) {
     noteMemberDeclaredHere(S, ReturnValue, Fn);
     return false;
   }
+  if (cast<clang::ReturnStmt>(ReturnStmt.get())->getNRVOCandidate() == GroDecl)
+    GroDecl->setNRVOVariable(true);
 
   this->ReturnStmt = ReturnStmt.get();
   return true;
 }
 
 // Create a static_cast\<T&&>(expr).
 static Expr *castForMoving(Sema &S, Expr *E, QualType T = QualType()) {
   if (T.isNull())

test/CodeGenCoroutines/coro-alloc.cpp

struct promise_type {
     void return_void() {}
     static int get_return_object_on_allocation_failure() { return -1; }
   };
 };
 
 // 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)
   // CHECK: %[[OK:.+]] = icmp ne i8* %[[MEM]], null
   // CHECK: br i1 %[[OK]], label %[[OKBB:.+]], label %[[ERRBB:.+]]
 
   // CHECK: [[ERRBB]]:
   // CHECK:   %[[FailRet:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type39get_return_object_on_allocation_failureEv(
   // CHECK:   store i32 %[[FailRet]], i32* %[[RetVal]]
   // CHECK:   br label %[[RetBB:.+]]
 
   // 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: %[[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
   // CHECK:   ret i32 %[[LoadRet]]
   co_return;
 }

test/CodeGenCoroutines/coro-gro-nrvo.cpp

+// 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: %call = call i8* @_Znwm(
+// CHECK-NEXT: br label %[[CoroInit:.*]]
+
+// CHECK: {{.*}}[[CoroInit]]:
+// 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: %call = call i8* @_ZnwmRKSt9nothrow_t
+// CHECK-NEXT: %[[CheckNull:.*]] = icmp ne i8* %call, null
+// CHECK-NEXT: br i1 %[[CheckNull]], label %[[InitOnSuccess:.*]], label %[[InitOnFailure:.*]]
+
+// CHECK: {{.*}}[[InitOnFailure]]:
+// CHECK-NEXT: call void @{{.*get_return_object_on_allocation_failureEv}}(%struct.coro_two* sret %agg.result
+// CHECK-NEXT: br label %[[RetLabel:.*]]
+
+// CHECK: {{.*}}[[InitOnSuccess]]:
+// 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;
+}