[PATCH] D63381: Allow copy/move assignment operator to be coroutine as per N4775

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM! Thank you for the fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63381/new/

https://reviews.llvm.org/D63381



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62550: [coroutines][PR41909] Don't build dependent coroutine statements for generic lambda

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM! Thank you for the fix


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62550/new/

https://reviews.llvm.org/D62550



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59076/new/

https://reviews.llvm.org/D59076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov requested changes to this revision.
GorNishanov added inline comments.
This revision now requires changes to proceed.



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

lewissbaker wrote:
> modocache wrote:
> > modocache wrote:
> > > lewissbaker wrote:
> > > > modocache wrote:
> > > > > EricWF wrote:
> > > > > > We can still build a valid AST after encountering this error, no?
> > > > > > 
> > > > > > 
> > > > > I believe so. Just to be clear: you'd like me to continue building 
> > > > > the AST even after emitting this error diagnostic? My understanding 
> > > > > is that most of this file bails soon after any error is encountered 
> > > > > (correct me if that's wrong). I'm happy to change that, but I wonder 
> > > > > if it'd be better to do that in a separate diff...?
> > > > Do the scope flags reset when a lambda occurs within a catch-block?
> > > > 
> > > > eg. The following should still be valid.
> > > > ```
> > > > try { ... }
> > > > catch (...) {
> > > >   []() -> task { co_await foo(); }();
> > > > }
> > > > ```
> > > I believe they're reset, the example you posted compiles fine with this 
> > > patch. I'll add a test case specifically for this and confirm exactly 
> > > where the scope flags are reset or ignored in the lambda case.
> > When the parser encounters a lambda, it takes the path 
> > `Parser::ParseLambdaExpression -> 
> > Parser::ParseLambdaExpressionAfterIntroducer -> 
> > Parser::ParseCompoundStatementBody`, which creates an inner scope for the 
> > body of the lambda. This inner scope does not have the `Scope::CatchScope` 
> > flag, so it doesn't result in the error.
> > 
> > Although, your question did make me realize that this compiles just fine, 
> > even with this patch:
> > 
> > ```
> > void example() {
> >   try {
> > throw;
> >   } catch (...) {
> > try {
> >   co_await a;
> > }
> >   }
> > }
> > ```
> > 
> > But I believe this above case should also be treated as an error, right?
> Yes, I believe that co_await within a try-block within a catch-block should 
> not be allowed.
Yes. That will result in suspension of the coroutine and we don't yet know how 
to suspend in catch blocks.

Also, I agree with @EricWF, this error should not prevent us from continuing 
semantic analysis with the rest of the function body.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59076/new/

https://reviews.llvm.org/D59076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: test/SemaCXX/coroutines.cpp:299-311
   // FIXME: The spec says this is ill-formed.
   void operator=(CtorDtor&) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }
   void operator=(CtorDtor const &) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }

modocache wrote:
> lewissbaker wrote:
> > Not related to this diff, but...
> > 
> > I don't think that these should be ill-formed.
> > 
> > According to N4775 there are only exclusions added for [class.ctor] and 
> > [class.dtor].
> > I can't see anything in the spec that says that assignment special member 
> > functions cannot be coroutines.
> That's a great point. Could you create a Bugzilla for this work? And maybe we 
> can get @GorNishanov's opinion?
In 2015, such as https://wg21.link/N4499 there was a blank prohibition: 
"A special member function shall not be a coroutine."

Later, at @rsmith suggestion, we relaxed it a little and banned only 
constructors and destructors.

I am not yet aware of any use cases where such coroutines would be useful.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59076/new/

https://reviews.llvm.org/D59076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54075: [coroutines] Fix fallthrough warning on try/catch

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D54075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.

LGTM! Thank you for doing this.


Repository:
  rC Clang

https://reviews.llvm.org/D51741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46140: [coroutines] std::task type (WIP)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: jfb.

LGTM

With couple of suggestions.

1. Remove swap. It is not part of the proposal at the moment.
2. If you feel like, add a lazy alias to task, that way we can taste both 
names and see how it feels.

My apologies for taking so long to review.


https://reviews.llvm.org/D46140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48863: [Sema] Explain coroutine_traits template in diag

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9053
   "a coroutine">;
+def note_coroutine_types_for_traits_here : Note<
+  "the coroutine traits class template is being instantiated using the return "

I am wondering what is the use case here.
Is it to guard against badly designed standard library?

I played around a little and tried to see how I could get to trigger this error 
with a user-code, but, failed. If I did not specify enough parameters it will 
default to primary template defined in the 


Repository:
  rC Clang

https://reviews.llvm.org/D48863



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a subscriber: rsmith.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Sema/SemaCoroutine.cpp:260
   else if (MD && MD->isMoveAssignmentOperator())
 return DiagInvalid(DiagMoveAssign);
+  // [basic.start.main]p3: "The function main shall not be a coroutine."

modocache wrote:
> @GorNishanov Is there anything in the TS that states copy and move assignment 
> operators shall not include await or yield expressions? These were added 
> D25292 but I'm not sure whether I'm missing something in the TS text, or if 
> maybe this language was in a prior revision of the TS.
Yes. N4499/[special] said:

   A special member function shall not be a coroutine.

I think @rsmith wanted to relax it, but, I am not sure if he had a use case in 
mind.

I am thinking putting the restriction from N4499 back. 
My approach is if in doubt, be more restrictive initially, then, we can relax 
if use cases are discovered. It will be a non-breaking change.


Repository:
  rC Clang

https://reviews.llvm.org/D48519



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM with some suggestions.




Comment at: lib/CodeGen/CGCoroutine.cpp:224
+  bool ResumeCanThrow = true;
+  if (const auto *MCE = dyn_cast(S.getResumeExpr()))
+if (const auto *Proto =

This long sequence of if statements seems to be asking to be put into its own 
predicate function: expressionCanThrow or something like that.


Repository:
  rC Clang

https://reviews.llvm.org/D47673



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL79: [coroutines] Pass implicit object parameter to 
promise ctor (fix BUG37604) (authored by GorNishanov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47454?vs=148831=148833#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47454

Files:
  cfe/trunk/lib/Sema/SemaCoroutine.cpp
  cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
  cfe/trunk/test/SemaCXX/coroutines.cpp

Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag1) { 
+extern "C" int f(mismatch_gro_type_tag1) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -848,7 +848,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag2) { 
+extern "C" int f(mismatch_gro_type_tag2) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -866,7 +866,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag3) { 
+extern "C" int f(mismatch_gro_type_tag3) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -885,7 +885,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag4) { 
+extern "C" int f(mismatch_gro_type_tag4) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -1246,7 +1246,10 @@
   co_return;
 }
 
+struct some_class;
+
 struct good_promise_custom_constructor {
+  good_promise_custom_constructor(some_class&, float, int);
   good_promise_custom_constructor(double, float, int);
   good_promise_custom_constructor() = delete;
   coro get_return_object();
@@ -1261,9 +1264,20 @@
   co_return;
 }
 
+struct some_class {
+  coro
+  good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+  }
+  coro
+  static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+  }
+};
+
 struct bad_promise_no_matching_constructor {
   bad_promise_no_matching_constructor(int, int, int);
-  // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+  // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
   bad_promise_no_matching_constructor() = delete;
   coro get_return_object();
   suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
   co_return;
 }
 
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+  // expected-error@-1 {{call to deleted constructor}}
+  co_return;
+}
+};
+
 } // namespace CoroHandleMemberFunctionTest
 
 class awaitable_no_unused_warn {
Index: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
   // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
   co_return;
 }
+
+struct some_class;
+
+struct method {};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+  };
+};
+
+struct some_class {
+  method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+  // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+  co_return;
+}
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
   // Build a list of arguments, based on the coroutine functions arguments,
   // that will be passed to the promise 

[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.
GorNishanov added reviewers: modocache, rsmith, lewissbaker.
Herald added a subscriber: EricWF.

Complete the implementation of p0914r1.
Implicit object parameter should be passed to a promise constructor.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=37604


https://reviews.llvm.org/D47454

Files:
  lib/Sema/SemaCoroutine.cpp
  test/CodeGenCoroutines/coro-params.cpp
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag1) { 
+extern "C" int f(mismatch_gro_type_tag1) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -848,7 +848,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag2) { 
+extern "C" int f(mismatch_gro_type_tag2) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -866,7 +866,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag3) { 
+extern "C" int f(mismatch_gro_type_tag3) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -885,7 +885,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag4) { 
+extern "C" int f(mismatch_gro_type_tag4) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -1246,7 +1246,10 @@
   co_return;
 }
 
+struct some_class;
+
 struct good_promise_custom_constructor {
+  good_promise_custom_constructor(some_class&, float, int);
   good_promise_custom_constructor(double, float, int);
   good_promise_custom_constructor() = delete;
   coro get_return_object();
@@ -1261,9 +1264,20 @@
   co_return;
 }
 
+struct some_class {
+  coro
+  good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+  }
+  coro
+  static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+  }
+};
+
 struct bad_promise_no_matching_constructor {
   bad_promise_no_matching_constructor(int, int, int);
-  // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+  // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
   bad_promise_no_matching_constructor() = delete;
   coro get_return_object();
   suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
   co_return;
 }
 
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+  // expected-error@-1 {{call to deleted constructor}}
+  co_return;
+}
+};
+
 } // namespace CoroHandleMemberFunctionTest
 
 class awaitable_no_unused_warn {
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
   // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
   co_return;
 }
+
+struct some_class;
+
+struct method {};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+  };
+};
+
+struct some_class {
+  method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+  // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+  co_return;
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
   // Build a list of arguments, based on the coroutine functions arguments,
   // that will be passed to the promise type's constructor.
   llvm::SmallVector CtorArgExprs;
+
+  // Add implicit object parameter.
+  if (auto *MD = dyn_cast(FD)) {
+if (MD->isInstance() && !isLambdaCallOperator(MD)) {
+   

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D45860



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

Looks good. Make sure to run the tests in release and debug mode.
On my box, these three test failed in release mode.

  Clang :: CodeGenCoroutines/coro-await-resume-eh.cpp
  Clang :: CodeGenCoroutines/coro-promise-dtor.cpp
  Clang :: CodeGenCoroutines/coro-unhandled-exception.cpp

There is a small difference in friendly names emission in release mode




Comment at: lib/CodeGen/CGCoroutine.cpp:220
   CGF.EmitBlock(ReadyBlock);
+  CXXTryStmt *TryStmt = nullptr;
+  if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {

I suggest to check whether await_resume expression is noexcept and omit 
emission of all of the goo. Most of the time the await_ready for 
initial_suspend will be noexcept and thus we would not need to emit extra stuff.



Repository:
  rC Clang

https://reviews.llvm.org/D45860



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

Thank you for doing this. It looks very elegant, but, it is a little bit wrong. 
It creates two different initial_suspend objects.
Run this example:

https://wandbox.org/permlink/Q1Zd2NUlolmw9YmX

You will observe that in trunk we are getting the output:

  0x216808c: constructed(initial)
  0x216808c: await_ready
  0x216808c: await_suspend
  0x216808c: await_resume
  0x216808c: destroyed
  0x21680b8: constructed(final)
  0x21680b8: await_ready
  0x21680b8: await_suspend
  consumed 10 values with sum 45
  0x21680b8: destroyed
  promise destroyed

With this change, the output becomes:

  0x1965c4c: constructed(initial)
  0x1965c4c: await_ready
  0x1965c4c: await_suspend
  0x1965c4c: destroyed
  0x1965c60: constructed(initial)
  0x1965c60: await_resume
  0x1965c60: destroyed
  0x1965c80: constructed(final)
  0x1965c80: await_ready
  0x1965c80: await_suspend
  consumed 10 values with sum 45
  0x1965c80: destroyed
  0x1965c60: destroyed
  promise destroyed

I suggest, first modify the unit test to check that we do not create two 
initial_suspend objects. Then to fix it :-)




Comment at: lib/CodeGen/CGCoroutine.cpp:595
 
+auto InitSuspend = S.getInitSuspendStmt();
 CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;

auto *InitialSuspend = ...

See: 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable




Comment at: lib/CodeGen/CGCoroutine.cpp:605
   auto Loc = S.getLocStart();
+  auto AwaitExpr =
+  cast(cast(InitSuspend)->getSubExpr());

auto *AwaitExpr =



Comment at: lib/CodeGen/CGCoroutine.cpp:608
+
+  SmallVector Stmts;
+  Stmts.push_back(AwaitExpr->getResumeExpr());

Consider:
```
 std::array Stmts = {AwaitExpr->getResumeExpr(), S.getBody()};
```
instead


Repository:
  rC Clang

https://reviews.llvm.org/D45860



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov closed this revision.
GorNishanov added a comment.

Committed as:

https://reviews.llvm.org/rCXX329237

and a flurry of cleanup fixes:

https://reviews.llvm.org/rCXX329239
https://reviews.llvm.org/rCXX329240
https://reviews.llvm.org/rCXX329245


https://reviews.llvm.org/D45121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 141010.
GorNishanov added a comment.

- s/_LIBCPP_ALWAYS_INLINE/_LIBCPP_INLINE_VISIBILITY throughout 

- Added _LIBCPP_INLINE_VISIBILITY to noop_coroutine constructor

@EricWF , good to go?


https://reviews.llvm.org/D45121

Files:
  include/experimental/coroutine
  
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+// XFAIL: clang-5, clang-6
+// 
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include 
+#include 
+#include 
+
+namespace coro = std::experimental::coroutines_v1;
+
+static_assert(std::is_same::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+  auto h = coro::noop_coroutine();
+  coro::coroutine_handle<> base = h;
+
+  assert(h);
+  assert(base);
+
+  assert(!h.done());
+  assert(!base.done());
+
+  h.resume();
+  h.destroy();
+  h();
+  static_assert(h.done() == false, "");
+  static_assert(h, "");
+
+  h.promise();
+  assert(h.address() == base.address());
+  assert(h.address() != nullptr);
+  assert(coro::coroutine_handle<>::from_address(h.address()) == base);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -93,60 +93,60 @@
 template <>
 class _LIBCPP_TEMPLATE_VIS coroutine_handle {
 public:
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 _LIBCPP_CONSTEXPR coroutine_handle() _NOEXCEPT : __handle_(nullptr) {}
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 _LIBCPP_CONSTEXPR coroutine_handle(nullptr_t) _NOEXCEPT : __handle_(nullptr) {}
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 coroutine_handle& operator=(nullptr_t) _NOEXCEPT {
 __handle_ = nullptr;
 return *this;
 }
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 _LIBCPP_CONSTEXPR void* address() const _NOEXCEPT { return __handle_; }
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 _LIBCPP_CONSTEXPR explicit operator bool() const _NOEXCEPT { return __handle_; }
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 void operator()() { resume(); }
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 void resume() {
   _LIBCPP_ASSERT(__is_suspended(),
  "resume() can only be called on suspended coroutines");
   _LIBCPP_ASSERT(!done(),
 "resume() has undefined behavior when the coroutine is done");
   __builtin_coro_resume(__handle_);
 }
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 void destroy() {
   _LIBCPP_ASSERT(__is_suspended(),
  "destroy() can only be called on suspended coroutines");
   __builtin_coro_destroy(__handle_);
 }
 
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 bool done() const {
   _LIBCPP_ASSERT(__is_suspended(),
  "done() can only be called on suspended coroutines");
   return __builtin_coro_done(__handle_);
 }
 
 public:
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 static coroutine_handle from_address(void* __addr) _NOEXCEPT {
 coroutine_handle __tmp;
 __tmp.__handle_ = __addr;
 return __tmp;
 }
 
 // FIXME: Should from_address(nullptr) be allowed?
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
 static 

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov marked an inline comment as done.
GorNishanov added inline comments.



Comment at: include/experimental/coroutine:294
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {

EricWF wrote:
> GorNishanov wrote:
> > lewissbaker wrote:
> > > EricWF wrote:
> > > > This should just be `_LIBCPP_INLINE_VISIBILITY`. We try not to use 
> > > > `_LIBCPP_ALWAYS_INLINE` in new code.
> > > Should the same change be applied to the other usages of 
> > > `_LIBCPP_ALWAYS_INLINE` in this file?
> > > Should some of them be marked `constexpr` to be consistent with 
> > > `noop_coroutine_handle` member functions above?
> > Those were added by @EricWF, so from my perspective they are immutable.
> I'm not sure what I was thinking when I implemented these things with 
> `_LIBCPP_ALWAYS_INLINE`.
@EricWF would you like me to do wholesale search-and-replace in coroutine 
header and make everything _LIBCPP_INLINE_VISIBILITY. Though, strategic use of 
always_inline in coroutine_handle and related classes can allow heap allocation 
elision to work even in -O0


https://reviews.llvm.org/D45121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov marked 11 inline comments as done.
GorNishanov added inline comments.



Comment at: include/experimental/coroutine:275
+return *reinterpret_cast<_Promise*>(
+__builtin_coro_promise(this->__handle_, __alignof(_Promise), 
false));
+}

CaseyCarter wrote:
> Is `this->non_static_member_of_non_dependent_base_class` idiomatic in libc++? 
> I typically reserve `this->` for forcing lookup into dependent bases. (and on 
> 217.)
I am keeping it consistent with other uses in the file. 



Comment at: include/experimental/coroutine:288
+
+coroutine_handle() {
+  this->__handle_ = __builtin_coro_noop();

CaseyCarter wrote:
> CaseyCarter wrote:
> > GorNishanov wrote:
> > > EricWF wrote:
> > > > Can `__builtin_coro_noop` produce a constant expression?
> > > No. llvm generates this value, so from clang perspective, it is not a 
> > > constant.
> > > At llvm level it is a private per TU constant, so invocations of 
> > > noop_coroutine() in different TUs linked into the same program will 
> > > return you different values.
> > If this class type is not literal - since there's no `constexpr` 
> > constructor - applying `constexpr` to the member functions on 278-283 is at 
> > best misleading and at worst ill-formed NDR due to 
> > http://eel.is/c++draft/dcl.constexpr#5.
> This constructor should be `_NOEXCEPT`, since it's invoked by 
> `noop_coroutine` which is `_NOEXCEPT`.
Issue for Rapperswil? These constexprs were approved by LEWG/LWG in 
Jacksonville 2018


https://reviews.llvm.org/D45121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 140983.
GorNishanov marked 2 inline comments as done.
GorNishanov added a comment.

- static_cast instead of reintrepret_cast in promise()
- 2 spaces indent in added code (the rest of the file stayed as is)
- added static_assert to check for done-ness and capacity
- constexpr => _LIBCPP_CONSTEXPR
- noexcept => _NOEXCEPT


https://reviews.llvm.org/D45121

Files:
  include/experimental/coroutine
  
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+// XFAIL: clang-5, clang-6
+// 
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include 
+#include 
+#include 
+
+namespace coro = std::experimental::coroutines_v1;
+
+static_assert(std::is_same::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+  auto h = coro::noop_coroutine();
+  coro::coroutine_handle<> base = h;
+
+  assert(h);
+  assert(base);
+
+  assert(!h.done());
+  assert(!base.done());
+
+  h.resume();
+  h.destroy();
+  h();
+  static_assert(h.done() == false, "");
+  static_assert(h, "");
+
+  h.promise();
+  assert(h.address() == base.address());
+  assert(h.address() != nullptr);
+  assert(coro::coroutine_handle<>::from_address(h.address()) == base);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -213,7 +213,7 @@
 
 _LIBCPP_INLINE_VISIBILITY
 _Promise& promise() const {
-return *reinterpret_cast<_Promise*>(
+return *static_cast<_Promise*>(
 __builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
 }
 
@@ -259,6 +259,45 @@
 }
 };
 
+#if __has_builtin(__builtin_coro_noop)
+struct noop_coroutine_promise {};
+
+template <>
+class _LIBCPP_TEMPLATE_VIS coroutine_handle
+: public coroutine_handle<> {
+  using _Base = coroutine_handle<>;
+  using _Promise = noop_coroutine_promise;
+public:
+
+  _LIBCPP_INLINE_VISIBILITY
+  _Promise& promise() const {
+return *static_cast<_Promise*>(
+  __builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
+  }
+
+  _LIBCPP_CONSTEXPR explicit operator bool() const _NOEXCEPT { return true; }
+  _LIBCPP_CONSTEXPR bool done() const _NOEXCEPT { return false; }
+
+  _LIBCPP_CONSTEXPR void operator()() const _NOEXCEPT {}
+  _LIBCPP_CONSTEXPR void resume() const _NOEXCEPT {}
+  _LIBCPP_CONSTEXPR void destroy() const _NOEXCEPT {}
+
+private:
+  friend coroutine_handle noop_coroutine() _NOEXCEPT;
+
+  coroutine_handle() _NOEXCEPT {
+this->__handle_ = __builtin_coro_noop();
+  }
+};
+
+using noop_coroutine_handle = coroutine_handle;
+
+inline _LIBCPP_INLINE_VISIBILITY
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
+  return {};
+}
+#endif // __has_builtin(__builtin_coro_noop)
+
 struct _LIBCPP_TYPE_VIS suspend_never {
   _LIBCPP_ALWAYS_INLINE
   bool await_ready() const _NOEXCEPT { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov marked 2 inline comments as done.
GorNishanov added inline comments.



Comment at: include/experimental/coroutine:294
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {

lewissbaker wrote:
> EricWF wrote:
> > This should just be `_LIBCPP_INLINE_VISIBILITY`. We try not to use 
> > `_LIBCPP_ALWAYS_INLINE` in new code.
> Should the same change be applied to the other usages of 
> `_LIBCPP_ALWAYS_INLINE` in this file?
> Should some of them be marked `constexpr` to be consistent with 
> `noop_coroutine_handle` member functions above?
Those were added by @EricWF, so from my perspective they are immutable.



Comment at: include/experimental/coroutine:288
+
+coroutine_handle() {
+  this->__handle_ = __builtin_coro_noop();

EricWF wrote:
> Can `__builtin_coro_noop` produce a constant expression?
No. llvm generates this value, so from clang perspective, it is not a constant.
At llvm level it is a private per TU constant, so invocations of 
noop_coroutine() in different TUs linked into the same program will return you 
different values.


https://reviews.llvm.org/D45121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 140903.
GorNishanov added a comment.

incorporated review feedback


https://reviews.llvm.org/D45121

Files:
  include/experimental/coroutine
  
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,64 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+// XFAIL: clang-5, clang-6
+// 
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include 
+#include 
+#include 
+
+namespace coro = std::experimental;
+
+static_assert(std::is_same::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+  auto h = coro::noop_coroutine();
+  coro::coroutine_handle<> base = h;
+
+  assert(h);
+  assert(base);
+
+  assert(!h.done());
+  assert(!base.done());
+
+  h.resume();
+  h.destroy();
+  h();
+
+  h.promise();
+  assert(h.address() == base.address());
+  assert(h.address() != nullptr);
+  assert(coro::coroutine_handle<>::from_address(h.address()) == base);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -259,6 +259,45 @@
 }
 };
 
+#if __has_builtin(__builtin_coro_noop)
+struct noop_coroutine_promise {};
+
+template <>
+class _LIBCPP_TEMPLATE_VIS coroutine_handle
+: public coroutine_handle<> {
+  using _Base = coroutine_handle<>;
+  using _Promise = noop_coroutine_promise;
+public:
+
+_LIBCPP_INLINE_VISIBILITY
+_Promise& promise() const {
+return *reinterpret_cast<_Promise*>(
+__builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
+}
+
+constexpr explicit operator bool() const noexcept { return true; }
+constexpr bool done() const noexcept { return false; }
+
+constexpr void operator()() const noexcept {}
+constexpr void resume() const noexcept {}
+constexpr void destroy() const noexcept {}
+
+private:
+friend coroutine_handle noop_coroutine()  _NOEXCEPT;
+
+coroutine_handle() {
+  this->__handle_ = __builtin_coro_noop();
+}
+};
+
+using noop_coroutine_handle = coroutine_handle;
+
+inline _LIBCPP_INLINE_VISIBILITY
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
+  return {};
+}
+#endif // __has_builtin(__builtin_coro_noop)
+
 struct _LIBCPP_TYPE_VIS suspend_never {
   _LIBCPP_ALWAYS_INLINE
   bool await_ready() const _NOEXCEPT { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

In https://reviews.llvm.org/D45121#1056408, @lewissbaker wrote:

> The `coroutine_handle` type does not have a 
> `from_address()` or a `from_promise()` static functions in the same way that 
> the `coroutine_handle` implementation does.
>  Is this intentional or an oversight in the TS wording?
>
> They don't seem hugely useful, so I'm not that worried.
>  If you know that you have a `coroutine_handle` then 
> you can just use `noop_coroutine()` to get the handle instead.


This is intentional. The only way to get a noop coroutine is to ask for it via 
`noop_coroutine()`


https://reviews.llvm.org/D45121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

@EricWF , gentle ping. Super tini-tiny change. Last piece missing to be 
post-Jax 2018 compilant


https://reviews.llvm.org/D45121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45120: [coroutines] Add __builtin_coro_noop => llvm.coro.noop

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov closed this revision.
GorNishanov added a comment.

Committed:

https://reviews.llvm.org/rC328993

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328993 
91177308-0d34-0410-b5e6-96231b3b80d8


https://reviews.llvm.org/D45120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45121: [coroutines] Add noop_coroutine to

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.
GorNishanov added reviewers: EricWF, lewissbaker, modocache.
GorNishanov added dependencies: D45114: [coroutines] Add support for 
llvm.coro.noop intrinsics, D45120: [coroutines] Add __builtin_coro_noop => 
llvm.coro.noop.

A recent addition to Coroutines TS (https://wg21.link/p0913) adds a pre-defined
coroutine noop_coroutine that does nothing.

This patch implements require library types in 

Related clang and llvm patches:

https://reviews.llvm.org/D45114
https://reviews.llvm.org/D45120


https://reviews.llvm.org/D45121

Files:
  include/experimental/coroutine
  
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,63 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+
+// 
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include 
+#include 
+#include 
+
+namespace coro = std::experimental;
+
+static_assert(std::is_same::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+  auto h = coro::noop_coroutine();
+  coro::coroutine_handle<> base = h;
+
+  assert(h);
+  assert(base);
+
+  assert(!h.done());
+  assert(!base.done());
+
+  h.resume();
+  h.destroy();
+  h();
+
+  h.promise();
+  assert(h.address() == base.address());
+  assert(h.address() != nullptr);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -259,6 +259,43 @@
 }
 };
 
+struct noop_coroutine_promise {};
+
+template <>
+class _LIBCPP_TEMPLATE_VIS coroutine_handle
+: public coroutine_handle<> {
+  using _Base = coroutine_handle<>;
+  using _Promise = noop_coroutine_promise;
+public:
+
+_LIBCPP_INLINE_VISIBILITY
+_Promise& promise() const {
+return *reinterpret_cast<_Promise*>(
+__builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
+}
+
+constexpr explicit operator bool() const noexcept { return true; }
+constexpr bool done() const noexcept { return false; }
+
+constexpr void operator()() const noexcept {}
+constexpr void resume() const noexcept {}
+constexpr void destroy() const noexcept {}
+
+private:
+friend coroutine_handle noop_coroutine()  _NOEXCEPT;
+
+coroutine_handle() {
+  this->__handle_ = __builtin_coro_noop();
+}
+};
+
+using noop_coroutine_handle = coroutine_handle;
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
+  return {};
+}
+
 struct _LIBCPP_TYPE_VIS suspend_never {
   _LIBCPP_ALWAYS_INLINE
   bool await_ready() const _NOEXCEPT { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45120: [coroutines] Add __builtin_coro_noop => llvm.coro.noop

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.
GorNishanov added reviewers: EricWF, modocache, lewissbaker.

A recent addition to Coroutines TS (https://wg21.link/p0913) adds a pre-defined
coroutine noop_coroutine that does nothing. To implement this feature, we 
implemented
an llvm.coro.noop intrinsic that returns a coroutine handle to a coroutine that
does nothing when resumed or destroyed.

This patch adds a builtin __builtin_coro_noop() that maps to llvm.coro.noop 
intrinsic.

Related llvm change: https://reviews.llvm.org/D45114


https://reviews.llvm.org/D45120

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGenCoroutines/coro-builtins.c


Index: test/CodeGenCoroutines/coro-builtins.c
===
--- test/CodeGenCoroutines/coro-builtins.c
+++ test/CodeGenCoroutines/coro-builtins.c
@@ -17,6 +17,9 @@
   // CHECK-NEXT: call i1 @llvm.coro.alloc(token %[[COROID]])
   __builtin_coro_alloc();
 
+  // CHECK-NEXT: call i8* @llvm.coro.noop()
+  __builtin_coro_noop();
+
   // CHECK-NEXT: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
   // CHECK-NEXT: %[[MEM:.+]] = call i8* @myAlloc(i64 %[[SIZE]])
   // CHECK-NEXT: %[[FRAME:.+]] = call i8* @llvm.coro.begin(token %[[COROID]], 
i8* %[[MEM]])
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2796,6 +2796,8 @@
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_resume);
   case Builtin::BI__builtin_coro_frame:
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_frame);
+  case Builtin::BI__builtin_coro_noop:
+return EmitCoroutineIntrinsic(E, Intrinsic::coro_noop);
   case Builtin::BI__builtin_coro_free:
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_free);
   case Builtin::BI__builtin_coro_destroy:
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -1391,6 +1391,7 @@
 
 BUILTIN(__builtin_coro_size, "z", "n")
 BUILTIN(__builtin_coro_frame, "v*", "n")
+BUILTIN(__builtin_coro_noop, "v*", "n")
 BUILTIN(__builtin_coro_free, "v*v*", "n")
 
 BUILTIN(__builtin_coro_id, "v*Iiv*v*v*", "n")


Index: test/CodeGenCoroutines/coro-builtins.c
===
--- test/CodeGenCoroutines/coro-builtins.c
+++ test/CodeGenCoroutines/coro-builtins.c
@@ -17,6 +17,9 @@
   // CHECK-NEXT: call i1 @llvm.coro.alloc(token %[[COROID]])
   __builtin_coro_alloc();
 
+  // CHECK-NEXT: call i8* @llvm.coro.noop()
+  __builtin_coro_noop();
+
   // CHECK-NEXT: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
   // CHECK-NEXT: %[[MEM:.+]] = call i8* @myAlloc(i64 %[[SIZE]])
   // CHECK-NEXT: %[[FRAME:.+]] = call i8* @llvm.coro.begin(token %[[COROID]], i8* %[[MEM]])
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2796,6 +2796,8 @@
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_resume);
   case Builtin::BI__builtin_coro_frame:
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_frame);
+  case Builtin::BI__builtin_coro_noop:
+return EmitCoroutineIntrinsic(E, Intrinsic::coro_noop);
   case Builtin::BI__builtin_coro_free:
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_free);
   case Builtin::BI__builtin_coro_destroy:
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -1391,6 +1391,7 @@
 
 BUILTIN(__builtin_coro_size, "z", "n")
 BUILTIN(__builtin_coro_frame, "v*", "n")
+BUILTIN(__builtin_coro_noop, "v*", "n")
 BUILTIN(__builtin_coro_free, "v*v*", "n")
 
 BUILTIN(__builtin_coro_id, "v*Iiv*v*v*", "n")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44552: [Coroutines] Find custom allocators in class scope

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM with some stylistic suggestions




Comment at: lib/Sema/SemaExprCXX.cpp:2351
+return true;
+  else
+LookupQualifiedName(R, Context.getTranslationUnitDecl());

drop else?

```
if (R.empty()) {
  if (NewScope == AFS_Class)
return true;

  LookupQualifiedName(R, Context.getTranslationUnitDecl());
}
```



Comment at: lib/Sema/SemaExprCXX.cpp:2402
+  return true;
+else {
+  DeclareGlobalNewDelete();

drop else? so that it will read:

```
  if (FoundDelete.empty()) {
if (DeleteScope == AFS_Class)
  return true;

DeclareGlobalNewDelete();
LookupQualifiedName(FoundDelete, Context.getTranslationUnitDecl());
  }
```


Repository:
  rC Clang

https://reviews.llvm.org/D44552



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov closed this revision.
GorNishanov added a comment.

Fixed:
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328663 
91177308-0d34-0410-b5e6-96231b3b80d8


https://reviews.llvm.org/D37115



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43927: [Coroutines] Schedule coro-split before asan

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D43927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42606: [Coroutines] Use allocator overload when available

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D42606



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42606: [Coroutines] Use allocator overload when available

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:1062
+  // an argument list."
+  for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())

GorNishanov wrote:
> This does not implement TS specified behavior for non static member functions:
> 
> [dcl.fct.def.coroutine]/7 states that  an argument list is build up as 
> follows:
> 
> >   The first argument is the amount of space requested, and has type 
> > std::size_t. The lvalues p1 ... pn are the succeeding arguments. 
> 
> Where p1 ... pn are defined earlier in
> 
> [dcl.fct.def.coroutine]/3 as:
> 
> > For a coroutine f that is a non-static member function, let P1 denote the 
> > type of the implicit object parameter (13.3.1) and P2 ... Pn be the types 
> > of the function parameters; otherwise let P1 ... Pn be the types of the 
> > function parameters. 
> 
> Essentially for non-static member functions, we need to insert implicit 
> object parameter.
> 
> Note that lookupPromiseType in SemaCoroutine.cpp when building specialization 
> of `std::experimental::coroutine_traits<...>` includes implicit object 
> parameter:
> 
> ```
>   // If the function is a non-static member function, add the type
>   // of the implicit object parameter before the formal parameters.
>   if (auto *MD = dyn_cast(FD)) {
> if (MD->isInstance()) {
>   // [over.match.funcs]4
>   // For non-static member functions, the type of the implicit object
>   // parameter is
>   //  -- "lvalue reference to cv X" for functions declared without a
>   //  ref-qualifier or with the & ref-qualifier
>   //  -- "rvalue reference to cv X" for functions declared with the &&
>   //  ref-qualifier
>   QualType T =
>   MD->getThisType(S.Context)->getAs()->getPointeeType();
>   T = FnType->getRefQualifier() == RQ_RValue
>   ? S.Context.getRValueReferenceType(T)
>   : S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
>   AddArg(T);
> }
>   }
> ```
I think promise constructor argument preview is also missing the implicit 
object parameter.


Repository:
  rC Clang

https://reviews.llvm.org/D42606



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42606: [Coroutines] Use allocator overload when available

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:1062
+  // an argument list."
+  for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())

This does not implement TS specified behavior for non static member functions:

[dcl.fct.def.coroutine]/7 states that  an argument list is build up as follows:

>   The first argument is the amount of space requested, and has type 
> std::size_t. The lvalues p1 ... pn are the succeeding arguments. 

Where p1 ... pn are defined earlier in

[dcl.fct.def.coroutine]/3 as:

> For a coroutine f that is a non-static member function, let P1 denote the 
> type of the implicit object parameter (13.3.1) and P2 ... Pn be the types of 
> the function parameters; otherwise let P1 ... Pn be the types of the function 
> parameters. 

Essentially for non-static member functions, we need to insert implicit object 
parameter.

Note that lookupPromiseType in SemaCoroutine.cpp when building specialization 
of `std::experimental::coroutine_traits<...>` includes implicit object 
parameter:

```
  // If the function is a non-static member function, add the type
  // of the implicit object parameter before the formal parameters.
  if (auto *MD = dyn_cast(FD)) {
if (MD->isInstance()) {
  // [over.match.funcs]4
  // For non-static member functions, the type of the implicit object
  // parameter is
  //  -- "lvalue reference to cv X" for functions declared without a
  //  ref-qualifier or with the & ref-qualifier
  //  -- "rvalue reference to cv X" for functions declared with the &&
  //  ref-qualifier
  QualType T =
  MD->getThisType(S.Context)->getAs()->getPointeeType();
  T = FnType->getRefQualifier() == RQ_RValue
  ? S.Context.getRValueReferenceType(T)
  : S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
  AddArg(T);
}
  }
```


Repository:
  rC Clang

https://reviews.llvm.org/D42606



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42605: [Sema] Toggle diags when finding allocators (NFCI)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D42605



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42606: [Coroutines] Use allocator overload when available

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/CodeGen/CGCoroutine.cpp:526
   EmitBlock(AllocBB);
-  auto *AllocateCall = EmitScalarExpr(S.getAllocate());
+  // Emit the call to the coroutine frame allocation function.
+  auto *AllocateCall = cast(EmitScalarExpr(S.getAllocate()));

GorNishanov wrote:
> First, thank you for doing this change!
> 
> Second, I am not sure that this part (CGCoroutine.cpp change) belongs in 
> clang.
> llvm CoroFrame is doing an unsafe transformation (it was safe until we got 
> the arguments to operator new :-) ). 
> 
> Moving the stores after the new that loads from those stores is an incorrect 
> transformation. I think it needs to be addressed in llvm. 
> getNotRelocatableInstructions function in CoroSplit.cpp needs to add special 
> handling for AllocaInst and freeze the stores to that Alloca in the blocks 
> preceeding the one with CoroBegin (getCoroBeginPredBlocks).
> 
> Also, for this to work for cases where parameter is used both in allocating 
> function and in the body of the coroutine we need to have a copy. Currently, 
> front-end does not create copies for scalar types (see 
> CoroutineStmtBuilder::makeParamMoves() in SemaCoroutine.cpp). I think if we 
> always create copies for all parameters, it will make this change more 
> straightforward.
> 
> Third, this code does not handle cases where scalar values passed by 
> reference to an allocation function or a struct passed by value:
> 
> Try this code on these:
> 
> void *operator new(unsigned long, promise_matching_placement_new_tag,
>int&, float, double);
> 
> or
> 
> ```
> struct Dummy { int x, y, z; ~Dummy(); };
> 
> template<>
> struct std::experimental::coroutine_traits promise_matching_placement_new_tag, Dummy&, float, double> {
>struct promise_type {
>void *operator new(unsigned long, 
> promise_matching_placement_new_tag,
>Dummy, float, double);
> ```
> 
> I think if this code is changed according to my earlier suggestion of doing 
> copies in clang and  freezing stores in the llvm, it should take care the 
> cases above.
> 
> These cases need to be added as tests to llvm\tests\Transforms\Coroutines
Alternatively, we can keep current clang behavior with respect to not doing 
copies for scalars and instead create a copy in llvm if we decided to freeze 
the store AND that alloca is used after a suspend point (CoroFrame decided that 
it needs to spill it into the coroutine frame).


Repository:
  rC Clang

https://reviews.llvm.org/D42606



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42606: [Coroutines] Use allocator overload when available

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov requested changes to this revision.
GorNishanov added inline comments.
This revision now requires changes to proceed.



Comment at: lib/CodeGen/CGCoroutine.cpp:526
   EmitBlock(AllocBB);
-  auto *AllocateCall = EmitScalarExpr(S.getAllocate());
+  // Emit the call to the coroutine frame allocation function.
+  auto *AllocateCall = cast(EmitScalarExpr(S.getAllocate()));

First, thank you for doing this change!

Second, I am not sure that this part (CGCoroutine.cpp change) belongs in clang.
llvm CoroFrame is doing an unsafe transformation (it was safe until we got the 
arguments to operator new :-) ). 

Moving the stores after the new that loads from those stores is an incorrect 
transformation. I think it needs to be addressed in llvm. 
getNotRelocatableInstructions function in CoroSplit.cpp needs to add special 
handling for AllocaInst and freeze the stores to that Alloca in the blocks 
preceeding the one with CoroBegin (getCoroBeginPredBlocks).

Also, for this to work for cases where parameter is used both in allocating 
function and in the body of the coroutine we need to have a copy. Currently, 
front-end does not create copies for scalar types (see 
CoroutineStmtBuilder::makeParamMoves() in SemaCoroutine.cpp). I think if we 
always create copies for all parameters, it will make this change more 
straightforward.

Third, this code does not handle cases where scalar values passed by reference 
to an allocation function or a struct passed by value:

Try this code on these:

void *operator new(unsigned long, promise_matching_placement_new_tag,
   int&, float, double);

or

```
struct Dummy { int x, y, z; ~Dummy(); };

template<>
struct std::experimental::coroutine_traits {
   struct promise_type {
   void *operator new(unsigned long, promise_matching_placement_new_tag,
   Dummy, float, double);
```

I think if this code is changed according to my earlier suggestion of doing 
copies in clang and  freezing stores in the llvm, it should take care the cases 
above.

These cases need to be added as tests to llvm\tests\Transforms\Coroutines


Repository:
  rC Clang

https://reviews.llvm.org/D42606



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D42343



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41820: [coroutines] Pass coro func args to promise ctor

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D41820



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41820: [coroutines] Pass coro func args to promise ctor

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

In https://reviews.llvm.org/D41820#974116, @EricWF wrote:

> Is this behavior specified somewhere? Or are we simply adding an extension to 
> Clang? If so I would really prefer to add my `co_promise` solution (but I 
> need to write a paper in favor of it first).


Before bringing the language change proposal, it makes sense to implement the 
feature and test it out on real examples.  MSVC compiler implemented a number 
of coroutine features to do sanity testing and put in the customers hands 
before a feature was proposed.

Since this is a non-breaking change, I think it makes sense to put it in and 
play with it. This particular approach was talked about since 2014, but, no 
formal proposal was made. I am bringing the paper to upcoming Jacksonville 
meeting and would like to make sure that we have both implementation and usage 
experience for the feature.


Repository:
  rC Clang

https://reviews.llvm.org/D41820



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41820: [coroutines] Pass coro func args to promise ctor

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov requested changes to this revision.
GorNishanov added a comment.
This revision now requires changes to proceed.

Thank you for doing this change!




Comment at: lib/Sema/SemaCoroutine.cpp:475
 
+// Create a static_cast\(expr).
+static Expr *castForMoving(Sema , Expr *E, QualType T = QualType()) {

I would keep this block of functions in their original place. (Or move them 
here as a separate NFC) commit.

Easier to review what was changed and what was simply moved.



Comment at: lib/Sema/SemaCoroutine.cpp:570
+auto RefExpr = ExprEmpty();
+auto Moves = ScopeInfo->CoroutineParameterMoves;
+if (Moves.find(PD) != Moves.end()) {

This creates a copy of the CoroutineParameterMoves map in every iteration of 
the loop. 
It seems like an intent is just to create a shorter alias "Moves" to it to 
refer later.

I suggest:
1) Make Moves a reference to the map: `auto  = ...`
2) Move it out of the loop




Comment at: lib/Sema/SemaCoroutine.cpp:572
+if (Moves.find(PD) != Moves.end()) {
+  // If a reference to the function parameter exists in the coroutine
+  // frame, use that reference.

Instead of doing lookup twice, once, using `find`,  another, using 
`operator[]`, you can just say:

```
auto EntryIter = Moves.find(PD);
if (EntryIter != Moves.end()) {
  auto *VD = cast(cast(EntryIter->second)->getSingleDecl());
...
```



Comment at: lib/Sema/SemaCoroutine.cpp:574
+  // frame, use that reference.
+  auto *VD = cast(cast(Moves[PD])->getSingleDecl());
+  RefExpr = BuildDeclRefExpr(VD, VD->getType(), ExprValueKind::VK_LValue,

This VD hides outer VD referring to the promise.
I would rename one of them to some other name.



Comment at: lib/Sema/SemaCoroutine.cpp:619
   FD->addDecl(VD);
   assert(!VD->isInvalidDecl());
   return VD;

I believe this assert needs to be removed. We can get an invalid decl if we 
failed to find an appropriate constructor. (Couple of negative tests in 
SemaCXX/coroutines.cpp would help to flush those cases out)



Comment at: test/CodeGenCoroutines/coro-alloc.cpp:196
 }
+
+struct promise_matching_constructor {};

I would move this test to coro-params.cpp, as it is closer to parameter moves 
than to allocations.

I would also add a negative test or two to SemaCXX/coroutines.cpp to verify 
that we emit sane errors when something goes wrong with promise constructor and 
parameter copies.


Repository:
  rC Clang

https://reviews.llvm.org/D41820



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39768: [coroutines] Promote cleanup.dest.slot allocas to registers to avoid storing it in the coroutine frame

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317981: [coroutines] Promote cleanup.dest.slot allocas to 
registers to avoid storing it… (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D39768?vs=122004=122585#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39768

Files:
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/test/CodeGenCoroutines/coro-dest-slot.cpp

Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
@@ -33,9 +33,11 @@
 #include "clang/Frontend/CodeGenOptions.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/Dominators.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/Transforms/Utils/PromoteMemToReg.h"
 using namespace clang;
 using namespace CodeGen;
 
@@ -419,6 +421,18 @@
 I->first->replaceAllUsesWith(I->second);
 I->first->eraseFromParent();
   }
+
+  // Eliminate CleanupDestSlot alloca by replacing it with SSA values and
+  // PHIs if the current function is a coroutine. We don't do it for all
+  // functions as it may result in slight increase in numbers of instructions
+  // if compiled with no optimizations. We do it for coroutine as the lifetime
+  // of CleanupDestSlot alloca make correct coroutine frame building very
+  // difficult.
+  if (NormalCleanupDest && isCoroutine()) {
+llvm::DominatorTree DT(*CurFn);
+llvm::PromoteMemToReg(NormalCleanupDest, DT);
+NormalCleanupDest = nullptr;
+  }
 }
 
 /// ShouldInstrumentFunction - Return true if the current function should be
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -225,6 +225,10 @@
   };
   CGCoroInfo CurCoro;
 
+  bool isCoroutine() const {
+return CurCoro.Data != nullptr;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
@@ -764,7 +768,7 @@
 ForceCleanup();
 }
 
-/// Checks if the global variable is captured in current function. 
+/// Checks if the global variable is captured in current function.
 bool isGlobalVarCaptured(const VarDecl *VD) const {
   VD = VD->getCanonicalDecl();
   return !VD->isLocalVarDeclOrParm() && CGF.LocalDeclMap.count(VD) > 0;
@@ -826,7 +830,7 @@
   /// block through the normal cleanup handling code (if any) and then
   /// on to \arg Dest.
   void EmitBranchThroughCleanup(JumpDest Dest);
-  
+
   /// isObviouslyBranchWithoutCleanups - Return true if a branch to the
   /// specified destination obviously has no cleanups to run.  'false' is always
   /// a conservatively correct answer for this method.
@@ -1045,7 +1049,7 @@
   if (Data.isValid()) Data.unbind(CGF);
 }
   };
-  
+
 private:
   CGDebugInfo *DebugInfo;
   bool DisableDebugInfo;
@@ -1434,19 +1438,19 @@
 
   /// Add OpenCL kernel arg metadata and the kernel attribute meatadata to
   /// the function metadata.
-  void EmitOpenCLKernelMetadata(const FunctionDecl *FD, 
+  void EmitOpenCLKernelMetadata(const FunctionDecl *FD,
 llvm::Function *Fn);
 
 public:
   CodeGenFunction(CodeGenModule , bool suppressNewContext=false);
   ~CodeGenFunction();
 
   CodeGenTypes () const { return CGM.getTypes(); }
   ASTContext () const { return CGM.getContext(); }
-  CGDebugInfo *getDebugInfo() { 
-if (DisableDebugInfo) 
+  CGDebugInfo *getDebugInfo() {
+if (DisableDebugInfo)
   return nullptr;
-return DebugInfo; 
+return DebugInfo;
   }
   void disableDebugInfo() { DisableDebugInfo = true; }
   void enableDebugInfo() { DisableDebugInfo = false; }
@@ -2509,7 +2513,7 @@
   };
   AutoVarEmission EmitAutoVarAlloca(const VarDecl );
   void EmitAutoVarInit(const AutoVarEmission );
-  void EmitAutoVarCleanups(const AutoVarEmission );  
+  void EmitAutoVarCleanups(const AutoVarEmission );
   void emitAutoVarTypeCleanup(const AutoVarEmission ,
   QualType::DestructionKind dtorKind);
 
@@ -2531,7 +2535,7 @@
 
 bool isIndirect() const { return Alignment != 0; }
 llvm::Value *getAnyValue() const { return Value; }
-
+
 llvm::Value *getDirectValue() const {
   assert(!isIndirect());
   return Value;
@@ -3188,7 +3192,7 @@
   LValue EmitCastLValue(const CastExpr *E);
   LValue EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   LValue EmitOpaqueValueLValue(const OpaqueValueExpr *e);
-  
+
   Address EmitExtVectorElementLValue(LValue V);
 
   RValue EmitRValueForField(LValue LV, const FieldDecl *FD, SourceLocation Loc);
@@ -3305,12 +3309,12 @@
   void EmitNoreturnRuntimeCallOrInvoke(llvm::Value 

[PATCH] D39768: [coroutines] Promote cleanup.dest.slot allocas to registers to avoid storing it in the coroutine frame

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.
Herald added a subscriber: EricWF.

We don't want to store cleanup dest slot saved into the coroutine frame (as 
some of the cleanup code may
access them after coroutine frame destroyed).

This is an alternative to https://reviews.llvm.org/D37093

It is possible to do this for all functions, but, cursory check showed that in 
-O0, we get slightly longer function (by 1-3 instructions), thus, we are only 
limiting cleanup.dest.slot elimination to coroutines.


https://reviews.llvm.org/D39768

Files:
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenCoroutines/coro-dest-slot.cpp

Index: test/CodeGenCoroutines/coro-dest-slot.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-dest-slot.cpp
@@ -0,0 +1,26 @@
+// 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;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_always initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+extern "C" coro f(int) { co_return; }
+// Verify that cleanup.dest.slot is eliminated in a coroutine.
+// CHECK-LABEL: f(
+// CHECK: call void @_ZNSt12experimental13coroutines_v113suspend_never12await_resumeEv(
+// CHECK: %[[CLEANUP_DEST:.+]] = phi i32 [ 0, %{{.+}} ], [ 2, %{{.+}} ], [ 2, %{{.+}} ]
+// CHECK: call i8* @llvm.coro.free(
+// CHECK: switch i32 %cleanup.dest.slot.0, label %{{.+}} [
+// CHECK-NEXT: i32 0
+// CHECK-NEXT: i32 2
+// CHECK-NEXT: ]
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -225,6 +225,10 @@
   };
   CGCoroInfo CurCoro;
 
+  bool isCoroutine() const {
+return CurCoro.Data != nullptr;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
@@ -764,7 +768,7 @@
 ForceCleanup();
 }
 
-/// Checks if the global variable is captured in current function. 
+/// Checks if the global variable is captured in current function.
 bool isGlobalVarCaptured(const VarDecl *VD) const {
   VD = VD->getCanonicalDecl();
   return !VD->isLocalVarDeclOrParm() && CGF.LocalDeclMap.count(VD) > 0;
@@ -826,7 +830,7 @@
   /// block through the normal cleanup handling code (if any) and then
   /// on to \arg Dest.
   void EmitBranchThroughCleanup(JumpDest Dest);
-  
+
   /// isObviouslyBranchWithoutCleanups - Return true if a branch to the
   /// specified destination obviously has no cleanups to run.  'false' is always
   /// a conservatively correct answer for this method.
@@ -1045,7 +1049,7 @@
   if (Data.isValid()) Data.unbind(CGF);
 }
   };
-  
+
 private:
   CGDebugInfo *DebugInfo;
   bool DisableDebugInfo;
@@ -1434,19 +1438,19 @@
 
   /// Add OpenCL kernel arg metadata and the kernel attribute meatadata to
   /// the function metadata.
-  void EmitOpenCLKernelMetadata(const FunctionDecl *FD, 
+  void EmitOpenCLKernelMetadata(const FunctionDecl *FD,
 llvm::Function *Fn);
 
 public:
   CodeGenFunction(CodeGenModule , bool suppressNewContext=false);
   ~CodeGenFunction();
 
   CodeGenTypes () const { return CGM.getTypes(); }
   ASTContext () const { return CGM.getContext(); }
-  CGDebugInfo *getDebugInfo() { 
-if (DisableDebugInfo) 
+  CGDebugInfo *getDebugInfo() {
+if (DisableDebugInfo)
   return nullptr;
-return DebugInfo; 
+return DebugInfo;
   }
   void disableDebugInfo() { DisableDebugInfo = true; }
   void enableDebugInfo() { DisableDebugInfo = false; }
@@ -2512,7 +2516,7 @@
   };
   AutoVarEmission EmitAutoVarAlloca(const VarDecl );
   void EmitAutoVarInit(const AutoVarEmission );
-  void EmitAutoVarCleanups(const AutoVarEmission );  
+  void EmitAutoVarCleanups(const AutoVarEmission );
   void emitAutoVarTypeCleanup(const AutoVarEmission ,
   QualType::DestructionKind dtorKind);
 
@@ -2534,7 +2538,7 @@
 
 bool isIndirect() const { return Alignment != 0; }
 llvm::Value *getAnyValue() const { return Value; }
-
+
 llvm::Value *getDirectValue() const {
   assert(!isIndirect());
   return Value;
@@ -3183,7 +3187,7 @@
   LValue EmitCastLValue(const CastExpr *E);
   LValue EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   LValue EmitOpaqueValueLValue(const OpaqueValueExpr *e);
-  
+
   Address EmitExtVectorElementLValue(LValue V);
 
   RValue EmitRValueForField(LValue LV, const FieldDecl *FD, SourceLocation Loc);
@@ -3300,12 +3304,12 @@
   void EmitNoreturnRuntimeCallOrInvoke(llvm::Value *callee,
ArrayRef args);
 
-  CGCallee BuildAppleKextVirtualCall(const 

[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

@rsmith , here is a fix for the funny diagnostics you mentioned.


https://reviews.llvm.org/D37115



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35046: [coroutines] Include "this" type when looking up coroutine_traits

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

@toby-allsopp: You mentioned that @EricWF already got this in. Can you close 
("abandon") this patch if it is no longer needed.


https://reviews.llvm.org/D35046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

ping


https://reviews.llvm.org/D37115



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312565: [coroutines] Make sure auto return type of 
await_resume is properly handled (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D37454?vs=113800=113894#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37454

Files:
  cfe/trunk/lib/Sema/SemaCoroutine.cpp
  cfe/trunk/test/SemaCXX/coroutines.cpp


Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
 //   - await-suspend is the expression e.await_suspend(h), which shall be
 // a prvalue of type void or bool.
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
 // Experimental support for coroutine_handle returning await_suspend.
 if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
-  QualType AdjRetType = RetType.getUnqualifiedType();
   if (RetType->isReferenceType() ||
-  (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+  (!RetType->isBooleanType() && !RetType->isVoidType())) {
 S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
 << RetType;
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
   void await_resume() {}
 };
 
+struct auto_await_suspend {
+  bool await_ready();
+  template  auto await_suspend(F) {}
+  void await_resume();
+};
+
 struct DummyVoidTag {};
 DummyVoidTag no_specialization() { // expected-error {{this function cannot be 
a coroutine: 'std::experimental::coroutine_traits' has no member 
named 'promise_type'}}
   co_await a;
@@ -159,6 +165,10 @@
   co_yield yield; // expected-error {{no member named 'await_ready' in 
'not_awaitable'}}
 }
 
+void check_auto_await_suspend() {
+  co_await auto_await_suspend{}; // Should compile successfully.
+}
+
 void coreturn(int n) {
   co_await a;
   if (n == 0)


Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
 //   - await-suspend is the expression e.await_suspend(h), which shall be
 // a prvalue of type void or bool.
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
 // Experimental support for coroutine_handle returning await_suspend.
 if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
-  QualType AdjRetType = RetType.getUnqualifiedType();
   if (RetType->isReferenceType() ||
-  (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+  (!RetType->isBooleanType() && !RetType->isVoidType())) {
 S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
 << RetType;
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
   void await_resume() {}
 };
 
+struct auto_await_suspend {
+  bool await_ready();
+  template  auto await_suspend(F) {}
+  void await_resume();
+};
+
 struct DummyVoidTag {};
 DummyVoidTag no_specialization() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
   co_await a;
@@ -159,6 +165,10 @@
   co_yield yield; // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
 }
 
+void check_auto_await_suspend() {
+  co_await auto_await_suspend{}; // Should compile successfully.
+}
+
 void coreturn(int n) {
   co_await a;
   if (n == 0)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov marked an inline comment as done.
GorNishanov added inline comments.



Comment at: test/SemaCXX/coroutines.cpp:169
+void check_auto_await_suspend() {
+  co_await auto_await_suspend{}; // OK
+}

javed.absar wrote:
> maybe change the comment to something more meaningful, or remove it.
Sure thing. I'll remove the comment.


https://reviews.llvm.org/D37454



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov marked an inline comment as done.
GorNishanov added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:451
   if (RetType->isReferenceType() ||
   (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
 S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),

rsmith wrote:
> Don't desugar the type; instead, use `AdjRetType->isBooleanType() || 
> AdjRetType->isVoidType()` here. You can also delete the 
> `getUnqualifiedType()` call above if you do this, and just use `RetType` 
> here, since `->isBooleanType()` on `const bool` returns `true`.
Cool! Much shorter and prettier. Thank you!


https://reviews.llvm.org/D37454



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 113800.
GorNishanov retitled this revision from "[coroutines] desugar auto type of 
await_resume before checking whether it is void or bool" to "[coroutines] Make 
sure auto return type of await_resume is properly handled".
GorNishanov added a comment.

use isVoidType() and isBooleantType() instead of cruft that used to be there 
before


https://reviews.llvm.org/D37454

Files:
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
   void await_resume() {}
 };
 
+struct auto_await_suspend {
+  bool await_ready();
+  template  auto await_suspend(F) {}
+  void await_resume();
+};
+
 struct DummyVoidTag {};
 DummyVoidTag no_specialization() { // expected-error {{this function cannot be 
a coroutine: 'std::experimental::coroutine_traits' has no member 
named 'promise_type'}}
   co_await a;
@@ -159,6 +165,10 @@
   co_yield yield; // expected-error {{no member named 'await_ready' in 
'not_awaitable'}}
 }
 
+void check_auto_await_suspend() {
+  co_await auto_await_suspend{}; // OK
+}
+
 void coreturn(int n) {
   co_await a;
   if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
 //   - await-suspend is the expression e.await_suspend(h), which shall be
 // a prvalue of type void or bool.
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
 // Experimental support for coroutine_handle returning await_suspend.
 if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
-  QualType AdjRetType = RetType.getUnqualifiedType();
   if (RetType->isReferenceType() ||
-  (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+  (!RetType->isBooleanType() && !RetType->isVoidType())) {
 S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
 << RetType;


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
   void await_resume() {}
 };
 
+struct auto_await_suspend {
+  bool await_ready();
+  template  auto await_suspend(F) {}
+  void await_resume();
+};
+
 struct DummyVoidTag {};
 DummyVoidTag no_specialization() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
   co_await a;
@@ -159,6 +165,10 @@
   co_yield yield; // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
 }
 
+void check_auto_await_suspend() {
+  co_await auto_await_suspend{}; // OK
+}
+
 void coreturn(int n) {
   co_await a;
   if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
 //   - await-suspend is the expression e.await_suspend(h), which shall be
 // a prvalue of type void or bool.
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
 // Experimental support for coroutine_handle returning await_suspend.
 if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
-  QualType AdjRetType = RetType.getUnqualifiedType();
   if (RetType->isReferenceType() ||
-  (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+  (!RetType->isBooleanType() && !RetType->isVoidType())) {
 S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
 << RetType;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37454: [coroutines] desugar auto type of await_resume before checking whether it is void or bool

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

https://reviews.llvm.org/D37454

Files:
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
   void await_resume() {}
 };
 
+struct auto_await_suspend {
+  bool await_ready();
+  template  auto await_suspend(F) {}
+  void await_resume();
+};
+
 struct DummyVoidTag {};
 DummyVoidTag no_specialization() { // expected-error {{this function cannot be 
a coroutine: 'std::experimental::coroutine_traits' has no member 
named 'promise_type'}}
   co_await a;
@@ -159,6 +165,10 @@
   co_yield yield; // expected-error {{no member named 'await_ready' in 
'not_awaitable'}}
 }
 
+void check_auto_await_suspend() {
+  co_await auto_await_suspend{}; // OK
+}
+
 void coreturn(int n) {
   co_await a;
   if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -444,6 +444,9 @@
 else {
   // non-class prvalues always have cv-unqualified types
   QualType AdjRetType = RetType.getUnqualifiedType();
+  if (auto *AT = dyn_cast(AdjRetType))
+AdjRetType = AT->desugar();
+
   if (RetType->isReferenceType() ||
   (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
 S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
   void await_resume() {}
 };
 
+struct auto_await_suspend {
+  bool await_ready();
+  template  auto await_suspend(F) {}
+  void await_resume();
+};
+
 struct DummyVoidTag {};
 DummyVoidTag no_specialization() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
   co_await a;
@@ -159,6 +165,10 @@
   co_yield yield; // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
 }
 
+void check_auto_await_suspend() {
+  co_await auto_await_suspend{}; // OK
+}
+
 void coreturn(int n) {
   co_await a;
   if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -444,6 +444,9 @@
 else {
   // non-class prvalues always have cv-unqualified types
   QualType AdjRetType = RetType.getUnqualifiedType();
+  if (auto *AT = dyn_cast(AdjRetType))
+AdjRetType = AT->desugar();
+
   if (RetType->isReferenceType() ||
   (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
 S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37433: [cxx_status] Mark coroutine TS support as clang 5.0 and change class from svn to full for P0096R4

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

Shall we?


https://reviews.llvm.org/D37433

Files:
  www/cxx_status.html


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -872,7 +872,7 @@
   
 
 
-  
+  
 Clang 5 (http://wg21.link/p0096r4;>P0096R4)
   
 
@@ -898,9 +898,9 @@
 
 
   [DRAFT TS] Coroutines
-  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf;>N4680
   -fcoroutines-ts-stdlib=libc++
-  SVN
+  Clang 5 (http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf;>N4680)
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -872,7 +872,7 @@
   
 
 
-  
+  
 Clang 5 (http://wg21.link/p0096r4;>P0096R4)
   
 
@@ -898,9 +898,9 @@
 
 
   [DRAFT TS] Coroutines
-  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf;>N4680
   -fcoroutines-ts-stdlib=libc++
-  SVN
+  Clang 5 (http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf;>N4680)
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37131: [coroutines] Support coroutine-handle returning await-suspend (i.e symmetric control transfer)

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311762: [coroutines] Support coroutine-handle returning 
await-suspend (i.e symmetric… (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D37131?vs=112658=112659#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37131

Files:
  cfe/trunk/lib/CodeGen/CGCoroutine.cpp
  cfe/trunk/lib/Sema/SemaCoroutine.cpp
  cfe/trunk/test/CodeGenCoroutines/coro-await.cpp

Index: cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
@@ -12,6 +12,7 @@
 struct coroutine_handle {
   void *ptr;
   static coroutine_handle from_address(void *);
+  void *address();
 };
 
 template 
@@ -326,3 +327,20 @@
   // CHECK-NEXT: store %struct.RefTag* %[[RES3]], %struct.RefTag** %[[ZVAR]],
   RefTag& z = co_yield 42;
 }
+
+struct TailCallAwait {
+  bool await_ready();
+  std::experimental::coroutine_handle<> await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+
+// CHECK-LABEL: @TestTailcall(
+extern "C" void TestTailcall() {
+  co_await TailCallAwait{};
+
+  // CHECK: %[[RESULT:.+]] = call i8* @_ZN13TailCallAwait13await_suspendENSt12experimental16coroutine_handleIvEE(%struct.TailCallAwait*
+  // CHECK: %[[COERCE:.+]] = getelementptr inbounds %"struct.std::experimental::coroutine_handle", %"struct.std::experimental::coroutine_handle"* %[[TMP:.+]], i32 0, i32 0
+  // CHECK: store i8* %[[RESULT]], i8** %[[COERCE]]
+  // CHECK: %[[ADDR:.+]] = call i8* @_ZNSt12experimental16coroutine_handleIvE7addressEv(%"struct.std::experimental::coroutine_handle"* %[[TMP]])
+  // CHECK: call void @llvm.coro.resume(i8* %[[ADDR]])
+}
Index: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
===
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp
@@ -181,10 +181,8 @@
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
-  if (SuspendRet != nullptr) {
+  if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
 // Veto suspension if requested by bool returning await_suspend.
-assert(SuspendRet->getType()->isIntegerTy(1) &&
-   "Sema should have already checked that it is void or bool");
 BasicBlock *RealSuspendBlock =
 CGF.createBasicBlock(Prefix + Twine(".suspend.bool"));
 CGF.Builder.CreateCondBr(SuspendRet, RealSuspendBlock, ReadyBlock);
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -363,6 +363,32 @@
   return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
 }
 
+// See if return type is coroutine-handle and if so, invoke builtin coro-resume
+// on its address. This is to enable experimental support for coroutine-handle
+// returning await_suspend that results in a guranteed tail call to the target
+// coroutine.
+static Expr *maybeTailCall(Sema , QualType RetType, Expr *E,
+   SourceLocation Loc) {
+  if (RetType->isReferenceType())
+return nullptr;
+  Type const *T = RetType.getTypePtr();
+  if (!T->isClassType() && !T->isStructureType())
+return nullptr;
+
+  // FIXME: Add convertability check to coroutine_handle<>. Possibly via
+  // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
+  // a private function in SemaExprCXX.cpp
+
+  ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", None);
+  if (AddressExpr.isInvalid())
+return nullptr;
+
+  Expr *JustAddress = AddressExpr.get();
+  // FIXME: Check that the type of AddressExpr is void*
+  return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
+  JustAddress);
+}
+
 /// Build calls to await_ready, await_suspend, and await_resume for a co_await
 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema , VarDecl *CoroPromise,
@@ -412,16 +438,21 @@
 //   - await-suspend is the expression e.await_suspend(h), which shall be
 // a prvalue of type void or bool.
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
-// non-class prvalues always have cv-unqualified types
-QualType AdjRetType = RetType.getUnqualifiedType();
-if (RetType->isReferenceType() ||
-(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
-  S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
- diag::err_await_suspend_invalid_return_type)
-  << RetType;
-  S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
-  << AwaitSuspend->getDirectCallee();
-  Calls.IsInvalid = true;
+// Experimental support for coroutine_handle returning 

[PATCH] D37131: [coroutines] Support coroutine-handle returning await-suspend (i.e symmetric control transfer)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

If await_suspend returns a coroutine_handle, as in the example below:

  coroutine_handle<> await_suspend(coroutine_handle<> h) {
coro.promise().waiter = h;
return coro;
  }

suspensionExpression processing will resume the coroutine pointed at by that 
handle.
Related LLVM change https://reviews.llvm.org/rL311751 makes resume calls of 
this kind `musttail` at any optimization level.

This enables unlimited symmetric control transfer from coroutine to coroutine 
without blowing up the stack.


https://reviews.llvm.org/D37131

Files:
  lib/CodeGen/CGCoroutine.cpp
  lib/Sema/SemaCoroutine.cpp
  test/CodeGenCoroutines/coro-await.cpp

Index: test/CodeGenCoroutines/coro-await.cpp
===
--- test/CodeGenCoroutines/coro-await.cpp
+++ test/CodeGenCoroutines/coro-await.cpp
@@ -12,6 +12,7 @@
 struct coroutine_handle {
   void *ptr;
   static coroutine_handle from_address(void *);
+  void *address();
 };
 
 template 
@@ -326,3 +327,20 @@
   // CHECK-NEXT: store %struct.RefTag* %[[RES3]], %struct.RefTag** %[[ZVAR]],
   RefTag& z = co_yield 42;
 }
+
+struct TailCallAwait {
+  bool await_ready();
+  std::experimental::coroutine_handle<> await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+
+// CHECK-LABEL: @TestTailcall(
+extern "C" void TestTailcall() {
+  co_await TailCallAwait{};
+
+  // CHECK: %[[RESULT:.+]] = call i8* @_ZN13TailCallAwait13await_suspendENSt12experimental16coroutine_handleIvEE(%struct.TailCallAwait*
+  // CHECK: %[[COERCE:.+]] = getelementptr inbounds %"struct.std::experimental::coroutine_handle", %"struct.std::experimental::coroutine_handle"* %[[TMP:.+]], i32 0, i32 0
+  // CHECK: store i8* %[[RESULT]], i8** %[[COERCE]]
+  // CHECK: %[[ADDR:.+]] = call i8* @_ZNSt12experimental16coroutine_handleIvE7addressEv(%"struct.std::experimental::coroutine_handle"* %[[TMP]])
+  // CHECK: call void @llvm.coro.resume(i8* %[[ADDR]])
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -363,6 +363,32 @@
   return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
 }
 
+// See if return type is coroutine-handle and if so, invoke builtin coro-resume
+// on its address. This is to enable experimental support for coroutine-handle
+// returning await_suspend that results in a guranteed tail call to the target
+// coroutine.
+static Expr *maybeTailCall(Sema , QualType RetType, Expr *E,
+   SourceLocation Loc) {
+  if (RetType->isReferenceType())
+return nullptr;
+  Type const *T = RetType.getTypePtr();
+  if (!T->isClassType() && !T->isStructureType())
+return nullptr;
+
+  // FIXME: Add convertability check to coroutine_handle<>. Possibly via
+  // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
+  // a private function in SemaExprCXX.cpp
+
+  ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", None);
+  if (AddressExpr.isInvalid())
+return nullptr;
+
+  Expr *JustAddress = AddressExpr.get();
+  // FIXME: Check that the type of AddressExpr is void*
+  return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
+  JustAddress);
+}
+
 /// Build calls to await_ready, await_suspend, and await_resume for a co_await
 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema , VarDecl *CoroPromise,
@@ -412,16 +438,21 @@
 //   - await-suspend is the expression e.await_suspend(h), which shall be
 // a prvalue of type void or bool.
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
-// non-class prvalues always have cv-unqualified types
-QualType AdjRetType = RetType.getUnqualifiedType();
-if (RetType->isReferenceType() ||
-(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
-  S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
- diag::err_await_suspend_invalid_return_type)
-  << RetType;
-  S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
-  << AwaitSuspend->getDirectCallee();
-  Calls.IsInvalid = true;
+// Experimental support for coroutine_handle returning await_suspend.
+if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
+  Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
+else {
+  // non-class prvalues always have cv-unqualified types
+  QualType AdjRetType = RetType.getUnqualifiedType();
+  if (RetType->isReferenceType() ||
+  (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
+   diag::err_await_suspend_invalid_return_type)
+<< RetType;
+S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+<< AwaitSuspend->getDirectCallee();
+  

[PATCH] D37131: [coroutines] Support coroutine-handle returning await-suspend (i.e symmetric control transfer)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D37131



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

When SemaCoroutine looks for await_resume, it means it. No need for helpful: 
"Did you mean await_ready?" messages.

Fixes PR33477 and a couple of FIXMEs in test/SemaCXX/coroutines.cpp


https://reviews.llvm.org/D37115

Files:
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -500,8 +500,7 @@
 
 struct bad_promise_2 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always final_suspend(); // expected-note {{here}}
+  suspend_always final_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -512,8 +511,7 @@
 
 struct bad_promise_3 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always initial_suspend(); // expected-note {{here}}
+  suspend_always initial_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -1162,3 +1160,22 @@
 template CoroMemberTag DepTestType::test_static_template(const char 
*volatile &, unsigned);
 
 } // namespace CoroHandleMemberFunctionTest
+
+struct missing_await_ready {
+  void await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct missing_await_suspend {
+  bool await_ready();
+  void await_resume();
+};
+struct missing_await_resume {
+  bool await_ready();
+  void await_suspend(std::experimental::coroutine_handle<>);
+};
+
+void test_missing_awaitable_members() {
+  co_await missing_await_ready{}; // expected-error {{no member named 
'await_ready' in 'missing_await_ready'}}
+  co_await missing_await_suspend{}; // expected-error {{no member named 
'await_suspend' in 'missing_await_suspend'}}
+  co_await missing_await_resume{}; // expected-error {{no member named 
'await_resume' in 'missing_await_resume'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -360,6 +360,15 @@
   if (Result.isInvalid())
 return ExprError();
 
+  // We meant exactly what we asked for. No need for typo correction.
+  if (auto *TE = dyn_cast(Result.get())) {
+S.clearDelayedTypo(TE);
+S.Diag(Loc, diag::err_no_member)
+<< NameInfo.getName() << Base->getType()->getAsCXXRecordDecl()
+<< Base->getSourceRange();
+return ExprError();
+  }
+
   return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
 }
 


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -500,8 +500,7 @@
 
 struct bad_promise_2 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always final_suspend(); // expected-note {{here}}
+  suspend_always final_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -512,8 +511,7 @@
 
 struct bad_promise_3 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always initial_suspend(); // expected-note {{here}}
+  suspend_always initial_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -1162,3 +1160,22 @@
 template CoroMemberTag DepTestType::test_static_template(const char *volatile &, unsigned);
 
 } // namespace CoroHandleMemberFunctionTest
+
+struct missing_await_ready {
+  void await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct missing_await_suspend {
+  bool await_ready();
+  void await_resume();
+};
+struct missing_await_resume {
+  bool await_ready();
+  void await_suspend(std::experimental::coroutine_handle<>);
+};
+
+void test_missing_awaitable_members() {
+  co_await missing_await_ready{}; // expected-error {{no member named 'await_ready' in 'missing_await_ready'}}
+  co_await missing_await_suspend{}; // expected-error {{no member named 'await_suspend' in 'missing_await_suspend'}}
+  co_await missing_await_resume{}; // expected-error {{no member named 'await_resume' in 'missing_await_resume'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -360,6 +360,15 @@
   if (Result.isInvalid())
 return ExprError();
 
+  // We meant exactly what we asked for. No need for typo correction.
+  if (auto *TE = dyn_cast(Result.get())) {
+S.clearDelayedTypo(TE);
+S.Diag(Loc, diag::err_no_member)
+<< NameInfo.getName() << Base->getType()->getAsCXXRecordDecl()
+<< Base->getSourceRange();
+return ExprError();
+  }
+
   return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D36070: [coroutines] Evaluate the operand of void `co_return` expressions.

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D36070



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35383: [coroutines] Add serialization/deserialization of coroutines

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
GorNishanov marked an inline comment as done.
Closed by commit rL308996: [coroutines] Add serialization/deserialization of 
coroutines (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D35383?vs=108111=108121#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35383

Files:
  cfe/trunk/include/clang/AST/StmtCXX.h
  cfe/trunk/include/clang/Serialization/ASTBitCodes.h
  cfe/trunk/lib/AST/StmtCXX.cpp
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
  cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
  cfe/trunk/test/PCH/coroutines.cpp

Index: cfe/trunk/include/clang/AST/StmtCXX.h
===
--- cfe/trunk/include/clang/AST/StmtCXX.h
+++ cfe/trunk/include/clang/AST/StmtCXX.h
@@ -317,6 +317,7 @@
   unsigned NumParams;
 
   friend class ASTStmtReader;
+  friend class ASTReader;
   friend TrailingObjects;
 
   Stmt **getStoredStmts() { return getTrailingObjects(); }
@@ -347,6 +348,8 @@
 
 public:
   static CoroutineBodyStmt *Create(const ASTContext , CtorArgs const );
+  static CoroutineBodyStmt *Create(const ASTContext , EmptyShell,
+   unsigned NumParams);
 
   bool hasDependentPromiseType() const {
 return getPromiseDecl()->getType()->isDependentType();
@@ -444,6 +447,8 @@
 SubStmts[SubStmt::PromiseCall] = PromiseCall;
   }
 
+  CoreturnStmt(EmptyShell) : CoreturnStmt({}, {}, {}) {}
+
   SourceLocation getKeywordLoc() const { return CoreturnLoc; }
 
   /// \brief Retrieve the operand of the 'co_return' statement. Will be nullptr
Index: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
===
--- cfe/trunk/include/clang/Serialization/ASTBitCodes.h
+++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h
@@ -1545,9 +1545,14 @@
 
   // ARC
   EXPR_OBJC_BRIDGED_CAST, // ObjCBridgedCastExpr
-  
+
   STMT_MS_DEPENDENT_EXISTS,   // MSDependentExistsStmt
-  EXPR_LAMBDA // LambdaExpr
+  EXPR_LAMBDA,// LambdaExpr
+  STMT_COROUTINE_BODY,
+  STMT_CORETURN,
+  EXPR_COAWAIT,
+  EXPR_COYIELD,
+  EXPR_DEPENDENT_COAWAIT,
 };
 
 /// \brief The kinds of designators that can occur in a
Index: cfe/trunk/test/PCH/coroutines.cpp
===
--- cfe/trunk/test/PCH/coroutines.cpp
+++ cfe/trunk/test/PCH/coroutines.cpp
@@ -0,0 +1,77 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -verify -std=c++1z -fcoroutines-ts %s
+
+// Test with pch.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts  -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -verify -std=c++1z -fcoroutines-ts %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+suspend_always yield_value(int) noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_value(int) noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+template 
+void f(T x) {  // checks coawait_expr and coroutine_body_stmt
+  co_yield 42; // checks coyield_expr
+  co_await x;  // checks dependent_coawait
+  co_return;   // checks coreturn_stmt
+}
+
+template 
+int f2(T x) {  // checks coawait_expr and coroutine_body_stmt
+  co_return x;   // checks coreturn_stmt with expr
+}
+
+#else
+
+// expected-no-diagnostics
+void g() {
+  f(suspend_always{});
+  f2(42);
+}
+
+#endif
Index: cfe/trunk/lib/AST/StmtCXX.cpp
===
--- cfe/trunk/lib/AST/StmtCXX.cpp
+++ cfe/trunk/lib/AST/StmtCXX.cpp
@@ -96,6 +96,20 @@
   return new (Mem) CoroutineBodyStmt(Args);
 }
 
+CoroutineBodyStmt *CoroutineBodyStmt::Create(const ASTContext , EmptyShell,
+ 

[PATCH] D35383: [coroutines] Add serialization/deserialization of coroutines

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 108111.
GorNishanov added a comment.

Thank you for the review. Feedback implemented. Preparing to land


https://reviews.llvm.org/D35383

Files:
  include/clang/AST/StmtCXX.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/StmtCXX.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/PCH/coroutines.cpp

Index: test/PCH/coroutines.cpp
===
--- /dev/null
+++ test/PCH/coroutines.cpp
@@ -0,0 +1,77 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -verify -std=c++1z -fcoroutines-ts %s
+
+// Test with pch.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts  -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -verify -std=c++1z -fcoroutines-ts %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+suspend_always yield_value(int) noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_value(int) noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+template 
+void f(T x) {  // checks coawait_expr and coroutine_body_stmt
+  co_yield 42; // checks coyield_expr
+  co_await x;  // checks dependent_coawait
+  co_return;   // checks coreturn_stmt
+}
+
+template 
+int f2(T x) {  // checks coawait_expr and coroutine_body_stmt
+  co_return x;   // checks coreturn_stmt with expr
+}
+
+#else
+
+// expected-no-diagnostics
+void g() {
+  f(suspend_always{});
+  f2(42);
+}
+
+#endif
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -286,7 +286,7 @@
   }
 
   // Outputs
-  for (unsigned I = 0, N = S->getNumOutputs(); I != N; ++I) {  
+  for (unsigned I = 0, N = S->getNumOutputs(); I != N; ++I) {
 Record.AddStmt(S->getOutputExpr(I));
 Record.AddString(S->getOutputConstraint(I));
   }
@@ -300,29 +300,48 @@
   Code = serialization::STMT_MSASM;
 }
 
-void ASTStmtWriter::VisitCoroutineBodyStmt(CoroutineBodyStmt *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void ASTStmtWriter::VisitCoroutineBodyStmt(CoroutineBodyStmt *CoroStmt) {
+  VisitStmt(CoroStmt);
+  Record.push_back(CoroStmt->getParamMoves().size());
+  for (Stmt *S : CoroStmt->children())
+Record.AddStmt(S);
+  Code = serialization::STMT_COROUTINE_BODY;
 }
 
 void ASTStmtWriter::VisitCoreturnStmt(CoreturnStmt *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+  VisitStmt(S);
+  Record.AddSourceLocation(S->getKeywordLoc());
+  Record.AddStmt(S->getOperand());
+  Record.AddStmt(S->getPromiseCall());
+  Record.push_back(S->isImplicit());
+  Code = serialization::STMT_CORETURN;
 }
 
-void ASTStmtWriter::VisitCoawaitExpr(CoawaitExpr *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void ASTStmtWriter::VisitCoroutineSuspendExpr(CoroutineSuspendExpr *E) {
+  VisitExpr(E);
+  Record.AddSourceLocation(E->getKeywordLoc());
+  for (Stmt *S : E->children())
+Record.AddStmt(S);
+  Record.AddStmt(E->getOpaqueValue());
 }
 
-void ASTStmtWriter::VisitDependentCoawaitExpr(DependentCoawaitExpr *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void ASTStmtWriter::VisitCoawaitExpr(CoawaitExpr *E) {
+  VisitCoroutineSuspendExpr(E);
+  Record.push_back(E->isImplicit());
+  Code = serialization::EXPR_COAWAIT;
+}
+
+void ASTStmtWriter::VisitCoyieldExpr(CoyieldExpr *E) {
+  VisitCoroutineSuspendExpr(E);
+  Code = serialization::EXPR_COYIELD;
 }
 
-void ASTStmtWriter::VisitCoyieldExpr(CoyieldExpr *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void 

[PATCH] D35383: [coroutines] Add serialization/deserialization of coroutines

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

@rsmith, ping. Would be good to get this in for clang 5.0


https://reviews.llvm.org/D35383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35383: [coroutines] Add serialization/deserialization of coroutines

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.
Herald added a subscriber: EricWF.

https://reviews.llvm.org/D35383

Files:
  include/clang/AST/StmtCXX.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/StmtCXX.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/PCH/coroutines.cpp

Index: test/PCH/coroutines.cpp
===
--- /dev/null
+++ test/PCH/coroutines.cpp
@@ -0,0 +1,77 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -verify -std=c++1z -fcoroutines-ts %s
+
+// Test with pch.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts  -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -verify -std=c++1z -fcoroutines-ts %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+suspend_always yield_value(int) noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_value(int) noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+template 
+void f(T x) {  // checks coawait_expr and coroutine_body_stmt
+  co_yield 42; // checks coyield_expr
+  co_await x;  // checks dependent_coawait
+  co_return;   // checks coreturn_stmt
+}
+
+template 
+int f2(T x) {  // checks coawait_expr and coroutine_body_stmt
+  co_return x;   // checks coreturn_stmt with expr
+}
+
+#else
+
+// expected-no-diagnostics
+void g() {
+  f(suspend_always{});
+  f2(42);
+}
+
+#endif
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -286,7 +286,7 @@
   }
 
   // Outputs
-  for (unsigned I = 0, N = S->getNumOutputs(); I != N; ++I) {  
+  for (unsigned I = 0, N = S->getNumOutputs(); I != N; ++I) {
 Record.AddStmt(S->getOutputExpr(I));
 Record.AddString(S->getOutputConstraint(I));
   }
@@ -300,29 +300,49 @@
   Code = serialization::STMT_MSASM;
 }
 
-void ASTStmtWriter::VisitCoroutineBodyStmt(CoroutineBodyStmt *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void ASTStmtWriter::VisitCoroutineBodyStmt(CoroutineBodyStmt *CoroStmt) {
+  VisitStmt(CoroStmt);
+  Record.push_back(CoroStmt->getParamMoves().size());
+  for (Stmt *S : CoroStmt->children())
+Record.AddStmt(S);
+  Code = serialization::STMT_COROUTINE_BODY;
 }
 
 void ASTStmtWriter::VisitCoreturnStmt(CoreturnStmt *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+  VisitStmt(S);
+  Record.AddSourceLocation(S->getKeywordLoc());
+  Record.AddStmt(S->getOperand());
+  Record.AddStmt(S->getPromiseCall());
+  Record.push_back(S->isImplicit());
+  Code = serialization::STMT_CORETURN;
+}
+
+static void serializeSuspendExpr(ASTStmtWriter , ASTRecordWriter ,
+ CoroutineSuspendExpr *E) {
+  Writer.VisitExpr(E);
+  Record.AddSourceLocation(E->getKeywordLoc());
+  for (Stmt *S : E->children())
+Record.AddStmt(S);
+  Record.AddStmt(E->getOpaqueValue());
 }
 
-void ASTStmtWriter::VisitCoawaitExpr(CoawaitExpr *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void ASTStmtWriter::VisitCoawaitExpr(CoawaitExpr *E) {
+  serializeSuspendExpr(*this, Record, E);
+  Record.push_back(E->isImplicit());
+  Code = serialization::EXPR_COAWAIT;
 }
 
-void ASTStmtWriter::VisitDependentCoawaitExpr(DependentCoawaitExpr *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void ASTStmtWriter::VisitCoyieldExpr(CoyieldExpr *E) {
+  serializeSuspendExpr(*this, Record, E);
+  Code = serialization::EXPR_COYIELD;
 }
 
-void ASTStmtWriter::VisitCoyieldExpr(CoyieldExpr *S) {
-  // FIXME: Implement coroutine serialization.
-  llvm_unreachable("unimplemented");
+void 

[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: test/CodeGenCoroutines/coro-await.cpp:310
+  int& x = co_await a;
+  // CHECK: await2.ready:
+  // CHECK-NEXT: %[[RES2:.+]] = call dereferenceable(4) i32* 
@_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue*
 %ref.tmp{{.+}})

In release compiler, labels won't have friendly names. //CHECK: await2.ready 
will fail


https://reviews.llvm.org/D34194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

Added John McCall as he made great suggestions last time I touched 
emitSuspendExpression


https://reviews.llvm.org/D34194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM with changes


https://reviews.llvm.org/D34194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov requested changes to this revision.
GorNishanov added inline comments.
This revision now requires changes to proceed.



Comment at: lib/CodeGen/CGCoroutine.cpp:255
+
+static QualType getCoroutineSuspendExprReturnType(const ASTContext ,
+  const CoroutineSuspendExpr *E) {

#ifndef NDEBUG
...
#endif


https://reviews.llvm.org/D34194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: test/CodeGenCoroutines/coro-await.cpp:301
+
+// Verifies that we don't chrash when returning an lvalue from a await_resume()
+// expression.

s/chrash/crash
s/a await_resume/an await_resume/


https://reviews.llvm.org/D34194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34021: [coroutines] Fix co_await for range statement

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D34021



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33797: [coroutines] Fix rebuilding of dependent coroutine parameters

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 101282.
GorNishanov added a comment.

test updated to use %[[copy_x]] instead of %x1


https://reviews.llvm.org/D33797

Files:
  lib/Sema/CoroutineStmtBuilder.h
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/TreeTransform.h
  test/CodeGenCoroutines/coro-params.cpp


Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
   // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
   // CHECK-NEXT: call i8* @llvm.coro.free(
 }
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template 
+void dependent_params(T x) {
+  // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+  // CHECK: call i8* @llvm.coro.begin
+  // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %[[x_copy]], %struct.A* 
dereferenceable(512) %x)
+  // CHECK-NEXT: invoke void 
@_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+  co_return;
+}
+
+struct A {
+  int WontFitIntoRegisterForSure[128];
+  A() noexcept;
+  A(A&&) noexcept;
+  ~A();
+};
+
+void call_dependent_params() {
+  dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
   Builder.ReturnStmt = Res.get();
 }
   }
+  Builder.buildParameterMoves();
 
   return getDerived().RebuildCoroutineBodyStmt(Builder);
 }
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
   return this->IsValid;
 }
 
+bool CoroutineStmtBuilder::buildParameterMoves() {
+  assert(this->IsValid && "coroutine already invalid");
+  assert(this->ParamMoves.empty() && "param moves already built");
+  return this->IsValid = makeParamMoves();
+}
+
 bool CoroutineStmtBuilder::buildDependentStatements() {
   assert(this->IsValid && "coroutine already invalid");
   assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
   /// name lookup.
   bool buildDependentStatements();
 
+  /// \brief Build just parameter moves. To use for rebuilding in 
TreeTransform.
+  bool buildParameterMoves();
+
   bool isInvalid() const { return !this->IsValid; }
 
 private:


Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
   // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
   // CHECK-NEXT: call i8* @llvm.coro.free(
 }
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template 
+void dependent_params(T x) {
+  // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+  // CHECK: call i8* @llvm.coro.begin
+  // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %[[x_copy]], %struct.A* dereferenceable(512) %x)
+  // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+  co_return;
+}
+
+struct A {
+  int WontFitIntoRegisterForSure[128];
+  A() noexcept;
+  A(A&&) noexcept;
+  ~A();
+};
+
+void call_dependent_params() {
+  dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
   Builder.ReturnStmt = Res.get();
 }
   }
+  Builder.buildParameterMoves();
 
   return getDerived().RebuildCoroutineBodyStmt(Builder);
 }
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
   return this->IsValid;
 }
 
+bool CoroutineStmtBuilder::buildParameterMoves() {
+  assert(this->IsValid && "coroutine already invalid");
+  assert(this->ParamMoves.empty() && "param moves already built");
+  return this->IsValid = makeParamMoves();
+}
+
 bool CoroutineStmtBuilder::buildDependentStatements() {
   assert(this->IsValid && "coroutine already invalid");
   assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
   /// name lookup.
   bool buildDependentStatements();
 
+  /// \brief Build just parameter moves. To use for rebuilding in TreeTransform.
+  bool buildParameterMoves();
+
   bool isInvalid() const { return !this->IsValid; }
 
 private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D33797: [coroutines] Fix rebuilding of dependent coroutine parameters

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: test/CodeGenCoroutines/coro-params.cpp:103
+  // CHECK: call i8* @llvm.coro.begin
+  // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %x1, %struct.A* 
dereferenceable(512) %x)
+  // CHECK-NEXT: call void 
@_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev

use %[[x_copy]] instead of %x1 here


https://reviews.llvm.org/D33797



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33797: [coroutines] Fix rebuilding of dependent coroutine parameters

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

We were not handling correctly rebuilding of parameter and were not creating 
copies for them.
With this checking, we will be always rebuilding parameter moves in 
TreeTransform's TransformCoroutineBodyStmt.


https://reviews.llvm.org/D33797

Files:
  lib/Sema/CoroutineStmtBuilder.h
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/TreeTransform.h
  test/CodeGenCoroutines/coro-params.cpp


Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
   // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
   // CHECK-NEXT: call i8* @llvm.coro.free(
 }
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template 
+void dependent_params(T x) {
+  // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+  // CHECK: call i8* @llvm.coro.begin
+  // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %x1, %struct.A* 
dereferenceable(512) %x)
+  // CHECK-NEXT: call void 
@_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+  co_return;
+}
+
+struct A {
+  int WontFitIntoRegisterForSure[128];
+  A();
+  A(A&&);
+  ~A();
+};
+
+void call_dependent_params() {
+  dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
   Builder.ReturnStmt = Res.get();
 }
   }
+  Builder.buildParameterMoves();
 
   return getDerived().RebuildCoroutineBodyStmt(Builder);
 }
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
   return this->IsValid;
 }
 
+bool CoroutineStmtBuilder::buildParameterMoves() {
+  assert(this->IsValid && "coroutine already invalid");
+  assert(this->ParamMoves.empty() && "param moves already built");
+  return this->IsValid = makeParamMoves();
+}
+
 bool CoroutineStmtBuilder::buildDependentStatements() {
   assert(this->IsValid && "coroutine already invalid");
   assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
   /// name lookup.
   bool buildDependentStatements();
 
+  /// \brief Build just parameter moves. To use for rebuilding in 
TreeTransform.
+  bool buildParameterMoves();
+
   bool isInvalid() const { return !this->IsValid; }
 
 private:


Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
   // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
   // CHECK-NEXT: call i8* @llvm.coro.free(
 }
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template 
+void dependent_params(T x) {
+  // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+  // CHECK: call i8* @llvm.coro.begin
+  // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %x1, %struct.A* dereferenceable(512) %x)
+  // CHECK-NEXT: call void @_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+  co_return;
+}
+
+struct A {
+  int WontFitIntoRegisterForSure[128];
+  A();
+  A(A&&);
+  ~A();
+};
+
+void call_dependent_params() {
+  dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
   Builder.ReturnStmt = Res.get();
 }
   }
+  Builder.buildParameterMoves();
 
   return getDerived().RebuildCoroutineBodyStmt(Builder);
 }
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
   return this->IsValid;
 }
 
+bool CoroutineStmtBuilder::buildParameterMoves() {
+  assert(this->IsValid && "coroutine already invalid");
+  assert(this->ParamMoves.empty() && "param moves already built");
+  return this->IsValid = makeParamMoves();
+}
+
 bool CoroutineStmtBuilder::buildDependentStatements() {
   assert(this->IsValid && "coroutine already invalid");
   assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
   /// name lookup.
   bool buildDependentStatements();
 
+  /// \brief Build just parameter moves. To use for rebuilding in TreeTransform.
+  bool buildParameterMoves();
+
   bool isInvalid() const { return !this->IsValid; }
 
 private:
___
cfe-commits mailing 

[PATCH] D33733: CGCleanup: No need to do domination fixups for static allocas

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov closed this revision.
GorNishanov added a comment.

Tested checked in. Thank you for looking into this, Reid!

r304380 = 1c0d15a4bf10876bb233e921907e114d52bee82e


https://reviews.llvm.org/D33733



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33750: CGCleanup: (NFC) add another test for r304335 - Don't try to spill static allocas

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL304380: CGCleanup: (NFC) add another test for r304335 - 
Don't try to spill static… (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D33750?vs=100951=100955#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33750

Files:
  cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp


Index: cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume() { return 8; }
+  template  void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+  consume(x);
+}
+


Index: cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume() { return 8; }
+  template  void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+  consume(x);
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33750: CGCleanup: (NFC) add a test that used to trigger broken IR

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

Coroutine related test that used to trigger broken IR prior to r304335.


https://reviews.llvm.org/D33750

Files:
  test/CodeGenCoroutines/coro-await-domination.cpp


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume() { return 8; }
+  template  void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+  consume(x);
+}
+


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume() { return 8; }
+  template  void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+  consume(x);
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov abandoned this revision.
GorNishanov added a comment.

Abandoning... Reid fixed the problem with "r304335 - Don't try to spill static 
allocas when emitting expr cleanups with branches"

Thank you Reid for looking into it!


https://reviews.llvm.org/D33663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33733: CGCleanup: No need to do domination fixups for static allocas

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

This is a follow up for the https://reviews.llvm.org/rL297084 which made sure 
that dominance is preserved during expression cleanup emission.
We ran into a case where dominance fixup code was inserting the store in the 
middle of allocas.

  %x = alloca i32, align 4
  store i32* %x, i32** %tmp.exprcleanup, align 4 ; <= HERE
  %ref.tmp3 = alloca %struct.A, align 1
  %agg.tmp5 = alloca 
%"struct.std::experimental::coroutines_v1::coroutine_handle.0", align 4
  %tmp.exprcleanup = alloca i32*, align 4
  %allocapt = bitcast i32 undef to i32
  store i32 %0, i32* %.addr, align 4

This fix make sure that domination fixup is not done static allocas.

(Alternative of https://reviews.llvm.org/D33663)


https://reviews.llvm.org/D33733

Files:
  lib/CodeGen/CGCleanup.cpp
  test/CodeGenCoroutines/coro-await-domination.cpp


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume();
+  template  void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -448,6 +448,11 @@
 auto *Inst = dyn_cast_or_null(*ReloadedValue);
 if (!Inst)
   continue;
+// If it is a static alloca, no need to spill.
+if (auto *AI = dyn_cast(Inst))
+  if (AI->isStaticAlloca())
+continue;
+
 Address Tmp =
 CreateDefaultAlignTempAlloca(Inst->getType(), "tmp.exprcleanup");
 


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume();
+  template  void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -448,6 +448,11 @@
 auto *Inst = dyn_cast_or_null(*ReloadedValue);
 if (!Inst)
   continue;
+// If it is a static alloca, no need to spill.
+if (auto *AI = dyn_cast(Inst))
+  if (AI->isStaticAlloca())
+continue;
+
 Address Tmp =
 CreateDefaultAlignTempAlloca(Inst->getType(), "tmp.exprcleanup");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/CodeGen/CGCleanup.cpp:458
   InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isa(Inst))
+  InsertBefore = std::next(AllocaInsertPt->getIterator());

rnk wrote:
> This doesn't seem right, `Inst` could be a dynamic alloca. If it's static, we 
> definitely don't need to store and reload it. All static allocas better be in 
> the entry block... You might want to use `isStaticAlloca`, but that still 
> feels like we're hacking around some deeper problem.
An extra context. Without the fix:

```
coro f(int) {
  int x = co_await A{}; // compiles fine
}
```

```
coro f(int) {
  int x = 42;
  x = co_await A{}; // does not compile due to broken IR
}
```


https://reviews.llvm.org/D33663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 100889.
GorNishanov added a comment.

Make sure that we use AllocaInsertPt for only static allocas in the entry block


https://reviews.llvm.org/D33663

Files:
  lib/CodeGen/CGCleanup.cpp
  test/CodeGenCoroutines/coro-await-domination.cpp


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume();
+  template  void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -416,6 +416,12 @@
 EHStack.popNullFixups();
 }
 
+static bool isStaticAlloca(llvm::Value* V) {
+  if (auto *AI = dyn_cast(V))
+return AI->isStaticAlloca();
+  return false;
+}
+
 /// Pops cleanup blocks until the given savepoint is reached.
 void CodeGenFunction::PopCleanupBlocks(
 EHScopeStack::stable_iterator Old,
@@ -455,6 +461,8 @@
 llvm::BasicBlock::iterator InsertBefore;
 if (auto *Invoke = dyn_cast(Inst))
   InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isStaticAlloca(Inst))
+  InsertBefore = std::next(AllocaInsertPt->getIterator());
 else
   InsertBefore = std::next(Inst->getIterator());
 CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume();
+  template  void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -416,6 +416,12 @@
 EHStack.popNullFixups();
 }
 
+static bool isStaticAlloca(llvm::Value* V) {
+  if (auto *AI = dyn_cast(V))
+return AI->isStaticAlloca();
+  return false;
+}
+
 /// Pops cleanup blocks until the given savepoint is reached.
 void CodeGenFunction::PopCleanupBlocks(
 EHScopeStack::stable_iterator Old,
@@ -455,6 +461,8 @@
 llvm::BasicBlock::iterator InsertBefore;
 if (auto *Invoke = dyn_cast(Inst))
   InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isStaticAlloca(Inst))
+  InsertBefore = std::next(AllocaInsertPt->getIterator());
 else
   InsertBefore = std::next(Inst->getIterator());
 CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33660: [coroutines] Fix assertion during -Wuninitialized analysis

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LBTM (Looks brilliant to me)


https://reviews.llvm.org/D33660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

This is a follow up for the https://reviews.llvm.org/rL297084 which made sure 
that dominance is preserved during expression cleanup emission.
We ran into a case where dominance fixup code was inserting the store in the 
middle of allocas.

  %x = alloca i32, align 4
  store i32* %x, i32** %tmp.exprcleanup, align 4 ; <= HERE
  %ref.tmp3 = alloca %struct.A, align 1
  %agg.tmp5 = alloca 
%"struct.std::experimental::coroutines_v1::coroutine_handle.0", align 4
  %tmp.exprcleanup = alloca i32*, align 4
  %allocapt = bitcast i32 undef to i32
  store i32 %0, i32* %.addr, align 4

This fix makes sure that if we are fixing up domination for an alloca 
instruction we will do it after the AllocaInsertPt


https://reviews.llvm.org/D33663

Files:
  lib/CodeGen/CGCleanup.cpp
  test/CodeGenCoroutines/coro-await-domination.cpp


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume();
+  template  void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -455,6 +455,8 @@
 llvm::BasicBlock::iterator InsertBefore;
 if (auto *Invoke = dyn_cast(Inst))
   InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isa(Inst))
+  InsertBefore = std::next(AllocaInsertPt->getIterator());
 else
   InsertBefore = std::next(Inst->getIterator());
 CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);


Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume();
+  template  void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -455,6 +455,8 @@
 llvm::BasicBlock::iterator InsertBefore;
 if (auto *Invoke = dyn_cast(Inst))
   InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isa(Inst))
+  InsertBefore = std::next(AllocaInsertPt->getIterator());
 else
   InsertBefore = std::next(Inst->getIterator());
 CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33632: [coroutines] www/cxx_status.html: add non-breaking hyphen

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL304092: [coroutines] www/cxx_status.html: add non-breaking 
hyphen (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D33632?vs=100555=100556#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33632

Files:
  cfe/trunk/www/cxx_status.html


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutinests-stdlib=libc++
+  fcoroutinests-stdlib=libc++
   SVN
 
 


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutinests-stdlib=libc++
+  fcoroutinests-stdlib=libc++
   SVN
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33632: [coroutines] www/cxx_status.html: add non-breaking hyphen

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 100555.
GorNishanov added a comment.

one more non-breaking hyphen to make it pretty in chrome. In edge it is perfect 
already, naturally


https://reviews.llvm.org/D33632

Files:
  www/cxx_status.html


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutinests-stdlib=libc++
+  fcoroutinests-stdlib=libc++
   SVN
 
 


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutinests-stdlib=libc++
+  fcoroutinests-stdlib=libc++
   SVN
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33632: [coroutines] www/cxx_status.html: add non-breaking hyphen

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL304091: [coroutines] www/cxx_status.html: add non-breaking 
hyphen (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D33632?vs=100552=100553#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33632

Files:
  cfe/trunk/www/cxx_status.html


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutines-ts-stdlib=libc++
+  -fcoroutinests-stdlib=libc++
   SVN
 
 


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutines-ts-stdlib=libc++
+  -fcoroutinests-stdlib=libc++
   SVN
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33632: [coroutines] cxx_status.hml: add non-breaking hyphen

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.
Herald added a subscriber: EricWF.

https://reviews.llvm.org/D33632

Files:
  www/cxx_status.html


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutines-ts-stdlib=libc++
+  -fcoroutinests-stdlib=libc++
   SVN
 
 


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -832,7 +832,7 @@
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  -fcoroutines-ts-stdlib=libc++
+  -fcoroutinests-stdlib=libc++
   SVN
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33625: [coroutines] Diagnose invalid result types for `await_resume` and `await_suspend` and add missing conversions.

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:324
 struct ReadySuspendResumeResult {
+  enum AwaitCallType { ACT_Ready, ACT_Suspend, ACT_Resume };
   Expr *Results[3];

enum class?


https://reviews.llvm.org/D33625



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33625: [coroutines] Diagnose invalid result types for `await_resume` and `await_suspend` and add missing conversions.

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: test/SemaCXX/coroutines.cpp:842
+struct bad_await_suspend_return {
+  bool await_ready();
+  // expected-error@+1 {{the return type of 'await_suspend' is required to be 
'void' or 'bool' (have 'char')}}

For test completeness, I would have a positive case for something like a 
std::true_type returning await_ready


https://reviews.llvm.org/D33625



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL304081: [coroutines] Mark cxx_status.html of Coroutines TS 
as (SVN) (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D33624?vs=100538=100539#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33624

Files:
  cfe/trunk/www/cxx_status.html


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

Drumroll.. Approved!


https://reviews.llvm.org/D33624



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

It is time!


https://reviews.llvm.org/D33624

Files:
  www/cxx_status.html


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33536: [coroutines] Bump __cpp_coroutines version

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D33536



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33534: [coroutines] Diagnose when promise types fail to declare either return_void or return_value.

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D33534



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33532: [coroutines] Fix fallthrough diagnostics for coroutines

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D33532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33532: [coroutines] Fix fallthrough diagnostics for coroutines

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: lib/Sema/AnalysisBasedWarnings.cpp:378
 const Stmt *S = CS.getStmt();
-if ((isa(S) && !IsCoroutine) || isa(S)) {
+if (isa(S) || isa(S)) {
   HasLiveReturn = true;

Is this check no longer needed because of the changes to 
AnalysisDeclContext.cpp?

At some point it was needed because we were emitting "return gro" to produce 
the immediate return value from the coroutine, but, we wanted to flag the case 
when the user forgot to add "co_return"


https://reviews.llvm.org/D33532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33507: [coroutines] Add support for coroutines with non-scalar parameters

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL303803: [coroutines] Add support for coroutines with 
non-scalar parameters (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D33507?vs=100132=100152#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33507

Files:
  cfe/trunk/lib/CodeGen/CGCoroutine.cpp
  cfe/trunk/lib/Sema/SemaCoroutine.cpp
  cfe/trunk/test/CodeGenCoroutines/coro-params.cpp

Index: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
@@ -0,0 +1,95 @@
+// Verifies that parameters are copied with move constructors
+// Verifies that parameter copies are destroyed
+// Vefifies that parameter copies are used in the body of the coroutine
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes -fexceptions | FileCheck %s
+
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+// TODO: Not supported yet
+struct CopyOnly {
+  int val;
+  CopyOnly(const CopyOnly&) noexcept;
+  CopyOnly(CopyOnly&&) = delete;
+  ~CopyOnly();
+};
+
+struct MoveOnly {
+  int val;
+  MoveOnly(const MoveOnly&) = delete;
+  MoveOnly(MoveOnly&&) noexcept;
+  ~MoveOnly();
+};
+
+struct MoveAndCopy {
+  int val;
+  MoveAndCopy(const MoveAndCopy&)noexcept;
+  MoveAndCopy(MoveAndCopy&&) noexcept;
+  ~MoveAndCopy();
+};
+
+void consume(int,int,int) noexcept;
+
+// TODO: Add support for CopyOnly params
+// CHECK: define void @_Z1fi8MoveOnly11MoveAndCopy(i32 %val, %struct.MoveOnly* %[[MoParam:.+]], %struct.MoveAndCopy* %[[McParam:.+]]) #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*
+void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
+  // CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly
+  // CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy
+  // CHECK: store i32 %val, i32* %[[ValAddr:.+]]
+
+  // CHECK: call i8* @llvm.coro.begin(
+  // CHECK-NEXT: call void @_ZN8MoveOnlyC1EOS_(%struct.MoveOnly* %[[MoCopy]], %struct.MoveOnly* dereferenceable(4) %[[MoParam]])
+  // CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(%struct.MoveAndCopy* %[[McCopy]], %struct.MoveAndCopy* dereferenceable(4) %[[McParam]]) #
+  // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
+
+  // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+  // CHECK: %[[IntParam:.+]] = load i32, i32* %val.addr
+  // CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, %struct.MoveOnly* %[[MoCopy]], i32 0, i32 0
+  // CHECK: %[[MoVal:.+]] = load i32, i32* %[[MoGep]]
+  // CHECK: %[[McGep:.+]] =  getelementptr inbounds %struct.MoveAndCopy, %struct.MoveAndCopy* %[[McCopy]], i32 0, i32 0
+  // CHECK: %[[McVal:.+]] = load i32, i32* %[[McGep]]
+  // CHECK: call void @_Z7consumeiii(i32 %[[IntParam]], i32 %[[MoVal]], i32 %[[McVal]])
+
+  consume(val, moParam.val, mcParam.val);
+  co_return;
+
+  // Skip to final suspend:
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
+  // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+
+  // Destroy promise, then parameter copies:
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise) #2
+  // CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(%struct.MoveAndCopy* %[[McCopy]])
+  // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
+  // CHECK-NEXT: call i8* @llvm.coro.free(
+}
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -1160,8 +1160,68 @@
   return true;
 }
 
+// Create a static_cast\(expr).
+static Expr *castForMoving(Sema , Expr *E, QualType T = QualType()) {
+  

[PATCH] D33507: [coroutines] Add support for coroutines with non-scalar parameters

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D33507



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33507: [coroutines] Add support for coroutines with non-scalar parameters

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

Simple types like int are handled by LLVM Coroutines just fine.
But for non-scalar parameters we need to create copies of those parameters in 
the coroutine frame and make all uses of those parameters to refer to parameter 
copies.


https://reviews.llvm.org/D33507

Files:
  lib/CodeGen/CGCoroutine.cpp
  lib/Sema/SemaCoroutine.cpp
  test/CodeGenCoroutines/coro-params.cpp

Index: test/CodeGenCoroutines/coro-params.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -0,0 +1,95 @@
+// Verifies that parameters are copied with move constructors
+// Verifies that parameter copies are destroyed
+// Vefifies that parameter copies are used in the body of the coroutine
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes -fexceptions | FileCheck %s
+
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+// TODO: Not supported yet
+struct CopyOnly {
+  int val;
+  CopyOnly(const CopyOnly&) noexcept;
+  CopyOnly(CopyOnly&&) = delete;
+  ~CopyOnly();
+};
+
+struct MoveOnly {
+  int val;
+  MoveOnly(const MoveOnly&) = delete;
+  MoveOnly(MoveOnly&&) noexcept;
+  ~MoveOnly();
+};
+
+struct MoveAndCopy {
+  int val;
+  MoveAndCopy(const MoveAndCopy&)noexcept;
+  MoveAndCopy(MoveAndCopy&&) noexcept;
+  ~MoveAndCopy();
+};
+
+void consume(int,int,int) noexcept;
+
+// TODO: Add support for CopyOnly params
+// CHECK: define void @_Z1fi8MoveOnly11MoveAndCopy(i32 %val, %struct.MoveOnly* %[[MoParam:.+]], %struct.MoveAndCopy* %[[McParam:.+]]) #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*
+void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
+  // CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly
+  // CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy
+  // CHECK: store i32 %val, i32* %[[ValAddr:.+]]
+
+  // CHECK: call i8* @llvm.coro.begin(
+  // CHECK-NEXT: call void @_ZN8MoveOnlyC1EOS_(%struct.MoveOnly* %[[MoCopy]], %struct.MoveOnly* dereferenceable(4) %[[MoParam]])
+  // CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(%struct.MoveAndCopy* %[[McCopy]], %struct.MoveAndCopy* dereferenceable(4) %[[McParam]]) #
+  // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
+
+  // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+  // CHECK: %[[IntParam:.+]] = load i32, i32* %val.addr
+  // CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, %struct.MoveOnly* %[[MoCopy]], i32 0, i32 0
+  // CHECK: %[[MoVal:.+]] = load i32, i32* %[[MoGep]]
+  // CHECK: %[[McGep:.+]] =  getelementptr inbounds %struct.MoveAndCopy, %struct.MoveAndCopy* %[[McCopy]], i32 0, i32 0
+  // CHECK: %[[McVal:.+]] = load i32, i32* %[[McGep]]
+  // CHECK: call void @_Z7consumeiii(i32 %[[IntParam]], i32 %[[MoVal]], i32 %[[McVal]])
+
+  consume(val, moParam.val, mcParam.val);
+  co_return;
+
+  // Skip to final suspend:
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
+  // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+
+  // Destroy promise, then parameter copies:
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise) #2
+  // CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(%struct.MoveAndCopy* %[[McCopy]])
+  // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
+  // CHECK-NEXT: call i8* @llvm.coro.free(
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -1160,8 +1160,68 @@
   return true;
 }
 
+// Create a static_cast\(expr).
+static Expr *castForMoving(Sema , Expr *E, QualType T = QualType()) {
+  if (T.isNull())
+T = E->getType();
+  QualType TargetType = S.BuildReferenceType(
+  T, /*SpelledAsLValue*/ false, SourceLocation(), 

[PATCH] D31670: [coroutines] Implement correct GRO lifetime

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 99923.
GorNishanov added a comment.

merge to top of the trunk


https://reviews.llvm.org/D31670

Files:
  lib/CodeGen/CGCoroutine.cpp
  test/CodeGenCoroutines/coro-gro.cpp

Index: test/CodeGenCoroutines/coro-gro.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-gro.cpp
@@ -0,0 +1,86 @@
+// Verifies lifetime of __gro local variable
+// Verify that coroutine promise and allocated memory are freed up on exception.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+struct GroType {
+  ~GroType();
+  operator int() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+  struct promise_type {
+GroType get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type();
+void unhandled_exception() noexcept;
+  };
+};
+
+struct Cleanup { ~Cleanup(); };
+void doSomething() noexcept;
+
+// CHECK: define i32 @_Z1fv(
+int f() {
+  // CHECK: %[[RetVal:.+]] = alloca i32
+  // CHECK: %[[GroActive:.+]] = alloca i1
+
+  // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
+  // CHECK: call i8* @_Znwm(i64 %[[Size]])
+  // CHECK: store i1 false, i1* %[[GroActive]]
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev(
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv(
+  // CHECK: store i1 true, i1* %[[GroActive]]
+
+  Cleanup cleanup;
+  doSomething();
+  co_return;
+
+  // CHECK: call void @_Z11doSomethingv(
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv(
+  // CHECK: call void @_ZN7CleanupD1Ev(
+
+  // Destroy promise and free the memory.
+
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev(
+  // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free(
+  // CHECK: call void @_ZdlPv(i8* %[[Mem]])
+
+  // Initialize retval from Gro and destroy Gro
+
+  // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv(
+  // CHECK: store i32 %[[Conv]], i32* %[[RetVal]]
+  // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]]
+  // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]]
+
+  // CHECK: [[CleanupGro]]:
+  // CHECK:   call void @_ZN7GroTypeD1Ev(
+  // CHECK:   br label %[[Done]]
+
+  // CHECK: [[Done]]:
+  // CHECK:   %[[LoadRet:.+]] = load i32, i32* %[[RetVal]]
+  // CHECK:   ret i32 %[[LoadRet]]
+}
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -11,6 +11,7 @@
 //
 //===--===//
 
+#include "CGCleanup.h"
 #include "CodeGenFunction.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "clang/AST/StmtCXX.h"
@@ -326,6 +327,72 @@
 };
 }
 
+namespace {
+struct GetReturnObjectManager {
+  CodeGenFunction 
+  CGBuilderTy 
+  const CoroutineBodyStmt 
+
+  Address GroActiveFlag;
+  CodeGenFunction::AutoVarEmission GroEmission;
+
+  GetReturnObjectManager(CodeGenFunction , const CoroutineBodyStmt )
+  : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()),
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+  // The gro variable has to outlive coroutine frame and coroutine promise, but,
+  // it can only be initialized after coroutine promise was created, thus, we
+  // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+  // cleanups. Later when coroutine promise is available we initialize the gro
+  // and sets the flag that the cleanup is now active.
+
+  void EmitGroAlloca() {
+auto *GroDeclStmt = dyn_cast(S.getResultDecl());
+if (!GroDeclStmt) {
+  // If get_return_object returns void, no need to do an alloca.
+  return;
+}
+
+auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl());
+
+// Set GRO flag that it is not initialized yet
+GroActiveFlag =
+  CGF.CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(), "gro.active");
+Builder.CreateStore(Builder.getFalse(), GroActiveFlag);
+
+GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl);
+
+// Remember the 

[PATCH] D31692: [coroutines] Wrap the body of the coroutine in try-catch

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov updated this revision to Diff 99814.
GorNishanov added a comment.

1. Heap allocate CxxTryStmt in CGCoroutine.
2. Merge with trunk


https://reviews.llvm.org/D31692

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CGCoroutine.cpp
  lib/Sema/SemaCoroutine.cpp
  test/CodeGenCoroutines/coro-cleanup.cpp
  test/CodeGenCoroutines/coro-eh-cleanup.cpp
  test/SemaCXX/coroutine-seh.cpp

Index: test/SemaCXX/coroutine-seh.cpp
===
--- /dev/null
+++ test/SemaCXX/coroutine-seh.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -triple x86_64-windows-msvc -fms-extensions
+namespace std::experimental {
+template  struct coroutine_traits;
+
+template  struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+void unhandled_exception() noexcept;
+  };
+};
+
+void SEH_used() {
+  __try { // expected-error {{cannot use SEH '__try' in a coroutine when C++ exceptions are enabled}}
+co_return; // expected-note {{function is a coroutine due to use of 'co_return' here}}
+  } __except(0) {}
+}
Index: test/CodeGenCoroutines/coro-eh-cleanup.cpp
===
--- test/CodeGenCoroutines/coro-eh-cleanup.cpp
+++ test/CodeGenCoroutines/coro-eh-cleanup.cpp
@@ -51,7 +51,13 @@
 // CHECK: [[EHCLEANUP]]:
 // CHECK:   %[[INNERPAD:.+]] = cleanuppad within none []
 // CHECK:   call void @"\01??_DCleanup@@QEAAXXZ"(
-// CHECK:   cleanupret from %[[INNERPAD]] unwind label %[[COROENDBB:.+]]
+// CHECK:   cleanupret from %4 unwind label %[[CATCHDISPATCH:.+]]
+
+// CHECK: [[CATCHDISPATCH]]:
+// CHECK:   catchswitch within none [label %[[CATCHPAD:.+]]] unwind label %[[COROENDBB:.+]]
+// CHECK: [[CATCHPAD]]:
+// CHECK:   call void @"\01?unhandled_exception@promise_type@coro_t@@QEAAXXZ"
+
 // CHECK: [[COROENDBB]]:
 // CHECK-NEXT: %[[CLPAD:.+]] = cleanuppad within none
 // CHECK-NEXT: call i1 @llvm.coro.end(i8* null, i1 true) [ "funclet"(token %[[CLPAD]]) ]
@@ -62,8 +68,14 @@
 // CHECK-LPAD:   to label %[[CONT:.+]] unwind label %[[EHCLEANUP:.+]]
 // CHECK-LPAD: [[EHCLEANUP]]:
 // CHECK-LPAD:landingpad { i8*, i32 }
-// CHECK-LPAD:  cleanup
+// CHECK-LPAD:  catch
 // CHECK-LPAD:   call void @_ZN7CleanupD1Ev(
+// CHECK-LPAD:   call i8* @__cxa_begin_catch
+// CHECK-LPAD:   call void @_ZN6coro_t12promise_type19unhandled_exceptionEv
+// CHECK-LPAD:   invoke void @__cxa_end_catch()
+// CHECK-LPAD: to label %{{.+}} unwind label %[[UNWINDBB:.+]]
+
+// CHECK-LPAD: [[UNWINDBB]]:
 // CHECK-LPAD:   %[[I1RESUME:.+]] = call i1 @llvm.coro.end(i8* null, i1 true)
 // CHECK-LPAD:   br i1  %[[I1RESUME]], label %[[EHRESUME:.+]], label
 // CHECK-LPAD: [[EHRESUME]]:
Index: test/CodeGenCoroutines/coro-cleanup.cpp
===
--- test/CodeGenCoroutines/coro-cleanup.cpp
+++ test/CodeGenCoroutines/coro-cleanup.cpp
@@ -6,38 +6,38 @@
 
 template  struct coroutine_handle {
   coroutine_handle() = default;
-  static coroutine_handle from_address(void *) { return {}; }
+  static coroutine_handle from_address(void *) noexcept;
 };
 template <> struct coroutine_handle {
-  static coroutine_handle from_address(void *) { return {}; }
+  static coroutine_handle from_address(void *) noexcept;
   coroutine_handle() = default;
   template 
-  coroutine_handle(coroutine_handle) {}
+  coroutine_handle(coroutine_handle) noexcept;
 };
 }
 
 struct suspend_always {
-  bool await_ready();
-  void await_suspend(std::experimental::coroutine_handle<>);
-  void await_resume();
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
 };
 
 template <> struct std::experimental::coroutine_traits {
   struct promise_type {
-void get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend();
-void return_void();
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
 promise_type();
 ~promise_type();
-void unhandled_exception();
+void unhandled_exception() noexcept;
   };
 };
 
 struct Cleanup { 

[PATCH] D31646: [coroutines] Build GRO declaration and return GRO statement

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

@rsmith  barely audible ping


https://reviews.llvm.org/D31646



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31608: [coroutines] Add emission of initial and final suspends

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

@EricWF: Can you take a quick look? Just a sanity check. The change is rather 
trivial.


https://reviews.llvm.org/D31608



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31608: [coroutines] Add emission of initial and final suspends

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added a comment.

soft ping


https://reviews.llvm.org/D31608



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits