[PATCH] D42606: [Coroutines] Use allocator overload when available
modocache added a comment. Hooray, `[dcl.fct.def.coroutine]/7` is *in!* Thank you for all the reviews, @GorNishanov! 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
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325291: [Coroutines] Use allocator overload when available
(authored by modocache, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D42606?vs=134304=134495#toc
Repository:
rC Clang
https://reviews.llvm.org/D42606
Files:
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
test/CodeGenCoroutines/coro-gro-nrvo.cpp
test/CodeGenCoroutines/coro-params.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -781,6 +781,102 @@
}
template coro dependent_uses_nothrow_new(good_promise_13);
+struct good_promise_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+good_coroutine_calls_custom_new_operator(double, float, int) {
+ co_return;
+}
+
+struct coroutine_nonstatic_member_struct;
+
+struct good_promise_nonstatic_member_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, coroutine_nonstatic_member_struct &, double);
+};
+
+struct bad_promise_nonstatic_member_mismatched_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-note@+1 {{candidate function not viable: requires 2 arguments, but 1 was provided}}
+ void *operator new(unsigned long, double);
+};
+
+struct coroutine_nonstatic_member_struct {
+ coro
+ good_coroutine_calls_nonstatic_member_custom_new_operator(double) {
+co_return;
+ }
+
+ coro
+ bad_coroutine_calls_nonstatic_member_mistmatched_custom_new_operator(double) {
+// expected-error@-1 {{no matching function for call to 'operator new'}}
+co_return;
+ }
+};
+
+struct bad_promise_mismatched_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_mismatched_custom_new_operator(double) {
+ // expected-error@-1 {{no matching function for call to 'operator new'}}
+ co_return;
+}
+
+struct bad_promise_throwing_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-error@+1 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_throwing_custom_new_operator(double, float, int) {
+ // expected-note@-1 {{call to 'operator new' implicitly required by coroutine function here}}
+ co_return;
+}
+
+struct good_promise_noexcept_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int) noexcept;
+};
+
+coro
+good_coroutine_calls_noexcept_custom_new_operator(double, float, int) {
+ co_return;
+}
+
struct mismatch_gro_type_tag1 {};
template<>
struct std::experimental::coroutine_traits {
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -106,6 +106,34 @@
co_return;
}
+struct promise_matching_placement_new_tag {};
+
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void *operator new(unsigned long, promise_matching_placement_new_tag,
+ int, float, double);
+void get_return_object() {}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+ };
+};
+
+// CHECK-LABEL: f1a(
+extern "C" void f1a(promise_matching_placement_new_tag, int x, float y , double z) {
+ // CHECK: store i32 %x, i32* %x.addr, align 4
+ // CHECK: store float %y, float* %y.addr, align 4
+ // CHECK: store double %z, double* %z.addr, align 8
+ // CHECK:
[PATCH] D42606: [Coroutines] Use allocator overload when available
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
modocache updated this revision to Diff 134304.
modocache added a comment.
Thanks for the review, @GorNishanov, and for pointing out that part of the
standard! I added a reference to the implicit object parameter, as well as some
tests for that behavior. And thanks for pointing out the flaw in
https://reviews.llvm.org/D41820 -- I'll submit a separate diff to forward the
implicit object parameter to promise type constructors.
Repository:
rC Clang
https://reviews.llvm.org/D42606
Files:
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
test/CodeGenCoroutines/coro-gro-nrvo.cpp
test/CodeGenCoroutines/coro-params.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -781,6 +781,102 @@
}
template coro dependent_uses_nothrow_new(good_promise_13);
+struct good_promise_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+good_coroutine_calls_custom_new_operator(double, float, int) {
+ co_return;
+}
+
+struct coroutine_nonstatic_member_struct;
+
+struct good_promise_nonstatic_member_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, coroutine_nonstatic_member_struct &, double);
+};
+
+struct bad_promise_nonstatic_member_mismatched_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-note@+1 {{candidate function not viable: requires 2 arguments, but 1 was provided}}
+ void *operator new(unsigned long, double);
+};
+
+struct coroutine_nonstatic_member_struct {
+ coro
+ good_coroutine_calls_nonstatic_member_custom_new_operator(double) {
+co_return;
+ }
+
+ coro
+ bad_coroutine_calls_nonstatic_member_mistmatched_custom_new_operator(double) {
+// expected-error@-1 {{no matching function for call to 'operator new'}}
+co_return;
+ }
+};
+
+struct bad_promise_mismatched_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_mismatched_custom_new_operator(double) {
+ // expected-error@-1 {{no matching function for call to 'operator new'}}
+ co_return;
+}
+
+struct bad_promise_throwing_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-error@+1 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_throwing_custom_new_operator(double, float, int) {
+ // expected-note@-1 {{call to 'operator new' implicitly required by coroutine function here}}
+ co_return;
+}
+
+struct good_promise_noexcept_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int) noexcept;
+};
+
+coro
+good_coroutine_calls_noexcept_custom_new_operator(double, float, int) {
+ co_return;
+}
+
struct mismatch_gro_type_tag1 {};
template<>
struct std::experimental::coroutine_traits {
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -69,12 +69,12 @@
// 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: 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
[PATCH] D42606: [Coroutines] Use allocator overload when available
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
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] D42606: [Coroutines] Use allocator overload when available
modocache updated this revision to Diff 133154.
modocache added a comment.
Update the tests to work with scalar parameter copies.
Repository:
rC Clang
https://reviews.llvm.org/D42606
Files:
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
test/CodeGenCoroutines/coro-gro-nrvo.cpp
test/CodeGenCoroutines/coro-params.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -781,6 +781,68 @@
}
template coro dependent_uses_nothrow_new(good_promise_13);
+struct good_promise_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+good_coroutine_calls_custom_new_operator(double, float, int) {
+ co_return;
+}
+
+struct bad_promise_mismatched_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_mismatched_custom_new_operator(double) {
+ // expected-error@-1 {{no matching function for call to 'operator new'}}
+ co_return;
+}
+
+struct bad_promise_throwing_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-error@+1 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_throwing_custom_new_operator(double, float, int) {
+ // expected-note@-1 {{call to 'operator new' implicitly required by coroutine function here}}
+ co_return;
+}
+
+struct good_promise_noexcept_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int) noexcept;
+};
+
+coro
+good_coroutine_calls_noexcept_custom_new_operator(double, float, int) {
+ co_return;
+}
+
struct mismatch_gro_type_tag1 {};
template<>
struct std::experimental::coroutine_traits {
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -69,12 +69,12 @@
// 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: 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: %[[IntParam:.+]] = load i32, i32* %val1
// 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
@@ -150,9 +150,9 @@
// CHECK-LABEL: void @_Z38coroutine_matching_promise_constructor28promise_matching_constructorifd(i32, float, double)
void coroutine_matching_promise_constructor(promise_matching_constructor, int, float, double) {
- // CHECK: %[[INT:.+]] = load i32, i32* %.addr, align 4
- // CHECK: %[[FLOAT:.+]] = load float, float* %.addr1, align 4
- // CHECK: %[[DOUBLE:.+]] = load double, double* %.addr2, align 8
+ // CHECK: %[[INT:.+]] = load i32, i32* %5, align 4
+ // CHECK: %[[FLOAT:.+]] = load float, float* %6, align 4
+ // CHECK: %[[DOUBLE:.+]] = load double, double* %7, align 8
// 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;
}
Index:
[PATCH] D42606: [Coroutines] Use allocator overload when available
modocache updated this revision to Diff 133129.
modocache added a comment.
Prevent coroutine parameter stores from being moved, rely on
https://reviews.llvm.org/D43000 instead.
Repository:
rC Clang
https://reviews.llvm.org/D42606
Files:
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -781,6 +781,68 @@
}
template coro dependent_uses_nothrow_new(good_promise_13);
+struct good_promise_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+good_coroutine_calls_custom_new_operator(double, float, int) {
+ co_return;
+}
+
+struct bad_promise_mismatched_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_mismatched_custom_new_operator(double) {
+ // expected-error@-1 {{no matching function for call to 'operator new'}}
+ co_return;
+}
+
+struct bad_promise_throwing_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-error@+1 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_throwing_custom_new_operator(double, float, int) {
+ // expected-note@-1 {{call to 'operator new' implicitly required by coroutine function here}}
+ co_return;
+}
+
+struct good_promise_noexcept_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int) noexcept;
+};
+
+coro
+good_coroutine_calls_noexcept_custom_new_operator(double, float, int) {
+ co_return;
+}
+
struct mismatch_gro_type_tag1 {};
template<>
struct std::experimental::coroutine_traits {
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -106,6 +106,34 @@
co_return;
}
+struct promise_matching_placement_new_tag {};
+
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void *operator new(unsigned long, promise_matching_placement_new_tag,
+ int, float, double);
+void get_return_object() {}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+ };
+};
+
+// CHECK-LABEL: f1a(
+extern "C" void f1a(promise_matching_placement_new_tag, int x, float y , double z) {
+ // CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
+ // CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
+ // CHECK: store i32 %x, i32* %coro.allocate.x.addr, align 4
+ // CHECK: %[[INT:.+]] = load i32, i32* %coro.allocate.x.addr, align 4
+ // CHECK: store float %y, float* %coro.allocate.y.addr, align 4
+ // CHECK: %[[FLOAT:.+]] = load float, float* %coro.allocate.y.addr, align 4
+ // CHECK: store double %z, double* %coro.allocate.z.addr, align 8
+ // CHECK: %[[DOUBLE:.+]] = load double, double* %coro.allocate.z.addr, align 8
+ // CHECK: call i8* @_ZNSt12experimental16coroutine_traitsIJv34promise_matching_placement_new_tagifdEE12promise_typenwEmS1_ifd(i64 %[[SIZE]], i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
+ co_return;
+}
+
struct promise_delete_tag {};
template<>
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -506,24 +506,15 @@
auto RefExpr = ExprEmpty();
auto Move = Moves.find(PD);
-if (Move != Moves.end()) {
- // If a reference to the function parameter exists in the coroutine
- // frame, use that reference.
- auto *MoveDecl =
- cast(cast(Move->second)->getSingleDecl());
- RefExpr = BuildDeclRefExpr(MoveDecl, MoveDecl->getType(),
- ExprValueKind::VK_LValue,
[PATCH] D42606: [Coroutines] Use allocator overload when available
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
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] D42606: [Coroutines] Use allocator overload when available
modocache updated this revision to Diff 131729.
modocache added a comment.
Add some diagnostics tests to SemaCXX/coroutines.cpp.
Repository:
rC Clang
https://reviews.llvm.org/D42606
Files:
lib/CodeGen/CGCoroutine.cpp
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -781,6 +781,68 @@
}
template coro dependent_uses_nothrow_new(good_promise_13);
+struct good_promise_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+good_coroutine_calls_custom_new_operator(double, float, int) {
+ co_return;
+}
+
+struct bad_promise_mismatched_custom_new_operator {
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_mismatched_custom_new_operator(double) {
+ // expected-error@-1 {{no matching function for call to 'operator new'}}
+ co_return;
+}
+
+struct bad_promise_throwing_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ // expected-error@+1 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}}
+ void *operator new(unsigned long, double, float, int);
+};
+
+coro
+bad_coroutine_calls_throwing_custom_new_operator(double, float, int) {
+ // expected-note@-1 {{call to 'operator new' implicitly required by coroutine function here}}
+ co_return;
+}
+
+struct good_promise_noexcept_custom_new_operator {
+ static coro get_return_object_on_allocation_failure();
+ coro get_return_object();
+ suspend_always initial_suspend();
+ suspend_always final_suspend();
+ void return_void();
+ void unhandled_exception();
+ void *operator new(unsigned long, double, float, int) noexcept;
+};
+
+coro
+good_coroutine_calls_noexcept_custom_new_operator(double, float, int) {
+ co_return;
+}
+
struct mismatch_gro_type_tag1 {};
template<>
struct std::experimental::coroutine_traits {
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -106,6 +106,34 @@
co_return;
}
+struct promise_matching_placement_new_tag {};
+
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void *operator new(unsigned long, promise_matching_placement_new_tag,
+ int, float, double);
+void get_return_object() {}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+ };
+};
+
+// CHECK-LABEL: f1a(
+extern "C" void f1a(promise_matching_placement_new_tag, int x, float y , double z) {
+ // CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
+ // CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
+ // CHECK: store i32 %x, i32* %coro.allocate.x.addr, align 4
+ // CHECK: %[[INT:.+]] = load i32, i32* %coro.allocate.x.addr, align 4
+ // CHECK: store float %y, float* %coro.allocate.y.addr, align 4
+ // CHECK: %[[FLOAT:.+]] = load float, float* %coro.allocate.y.addr, align 4
+ // CHECK: store double %z, double* %coro.allocate.z.addr, align 8
+ // CHECK: %[[DOUBLE:.+]] = load double, double* %coro.allocate.z.addr, align 8
+ // CHECK: call i8* @_ZNSt12experimental16coroutine_traitsIJv34promise_matching_placement_new_tagifdEE12promise_typenwEmS1_ifd(i64 %[[SIZE]], i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
+ co_return;
+}
+
struct promise_delete_tag {};
template<>
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -1050,18 +1050,54 @@
const bool RequiresNoThrowAlloc = ReturnStmtOnAllocFailure != nullptr;
- // FIXME: Add support for stateful allocators.
+ // [dcl.fct.def.coroutine]/7
+ // Lookup allocation functions using a parameter list composed of the
+ // requested size of the coroutine state being allocated, followed by
+ // the coroutine function's arguments. If a matching allocation function
+ // exists, use it. Otherwise, use an allocation function
[PATCH] D42606: [Coroutines] Use allocator overload when available
modocache created this revision.
modocache added reviewers: rsmith, GorNishanov, eric_niebler.
Herald added a subscriber: EricWF.
Depends on https://reviews.llvm.org/D42605.
An implementation of the behavior described in `[dcl.fct.def.coroutine]/7`:
when a promise type overloads `operator new` using a "placement new"
that takes the same argument types as the coroutine function, that
overload is used when allocating the coroutine frame.
Simply passing references to the coroutine function parameters directly
to `operator new` results in invariant violations in LLVM's coroutine
splitting pass, so this implementation modifies Clang codegen to
produce allocator-specific alloc/store/loads for each parameter being
forwarded to the allocator.
Test Plan: `check-clang`
Repository:
rC Clang
https://reviews.llvm.org/D42606
Files:
lib/CodeGen/CGCoroutine.cpp
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -106,6 +106,34 @@
co_return;
}
+struct promise_matching_placement_new_tag {};
+
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void *operator new(unsigned long, promise_matching_placement_new_tag,
+ int, float, double);
+void get_return_object() {}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+ };
+};
+
+// CHECK-LABEL: f1a(
+extern "C" void f1a(promise_matching_placement_new_tag, int x, float y , double z) {
+ // CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
+ // CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
+ // CHECK: store i32 %x, i32* %coro.allocate.x.addr, align 4
+ // CHECK: %[[INT:.+]] = load i32, i32* %coro.allocate.x.addr, align 4
+ // CHECK: store float %y, float* %coro.allocate.y.addr, align 4
+ // CHECK: %[[FLOAT:.+]] = load float, float* %coro.allocate.y.addr, align 4
+ // CHECK: store double %z, double* %coro.allocate.z.addr, align 8
+ // CHECK: %[[DOUBLE:.+]] = load double, double* %coro.allocate.z.addr, align 8
+ // CHECK: call i8* @_ZNSt12experimental16coroutine_traitsIJv34promise_matching_placement_new_tagifdEE12promise_typenwEmS1_ifd(i64 %[[SIZE]], i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
+ co_return;
+}
+
struct promise_delete_tag {};
template<>
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -1050,18 +1050,54 @@
const bool RequiresNoThrowAlloc = ReturnStmtOnAllocFailure != nullptr;
- // FIXME: Add support for stateful allocators.
+ // [dcl.fct.def.coroutine]/7
+ // Lookup allocation functions using a parameter list composed of the
+ // requested size of the coroutine state being allocated, followed by
+ // the coroutine function's arguments. If a matching allocation function
+ // exists, use it. Otherwise, use an allocation function that just takes
+ // the requested size.
FunctionDecl *OperatorNew = nullptr;
FunctionDecl *OperatorDelete = nullptr;
FunctionDecl *UnusedResult = nullptr;
bool PassAlignment = false;
SmallVector PlacementArgs;
+ // [dcl.fct.def.coroutine]/7
+ // "The allocation function’s name is looked up in the scope of P.
+ // [...] If the lookup finds an allocation function in the scope of P,
+ // overload resolution is performed on a function call created by assembling
+ // an argument list."
+ for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())
+ continue;
+
+// Build a reference to the parameter.
+auto PDLoc = PD->getLocation();
+ExprResult PDRefExpr =
+S.BuildDeclRefExpr(PD, PD->getOriginalType().getNonReferenceType(),
+ ExprValueKind::VK_LValue, PDLoc);
+if (PDRefExpr.isInvalid())
+ return false;
+
+PlacementArgs.push_back(PDRefExpr.get());
+ }
S.FindAllocationFunctions(Loc, SourceRange(),
/*UseGlobal*/ false, PromiseType,
/*isArray*/ false, PassAlignment, PlacementArgs,
-OperatorNew, UnusedResult);
+OperatorNew, UnusedResult, /*Diagnose*/ false);
+
+ // [dcl.fct.def.coroutine]/7
+ // "If no matching function is found, overload resolution is performed again
+ // on a function call created by passing just the amount of space required as
+ // an argument of type std::size_t."
+ if (!OperatorNew && !PlacementArgs.empty()) {
+PlacementArgs.clear();
+S.FindAllocationFunctions(Loc, SourceRange(),
+ /*UseGlobal*/ false, PromiseType,
+ /*isArray*/ false, PassAlignment,
