* [PATCH] coroutines: Copy attributes to the outlined functions [PR95518]
@ 2020-06-11 19:53 Iain Sandoe
2020-06-15 13:57 ` Nathan Sidwell
0 siblings, 1 reply; 4+ messages in thread
From: Iain Sandoe @ 2020-06-11 19:53 UTC (permalink / raw)
To: GCC Patches; +Cc: Nathan Sidwell
Hi
We had omitted the copying of function attributes (including the
'used' status). Mark the outlined functions as artificial, since
they are; some diagnostic processing tests this.
tested on Linux and Darwin,
OK for master?
10.2?
thanks
Iain
gcc/cp/ChangeLog:
PR c++/95518
* coroutines.cc (act_des_fn): Copy function attributes from
the user’s function decl onto the outlined helpers.
gcc/testsuite/ChangeLog:
PR c++/95518
* g++.dg/coroutines/pr95518.C: New test.
---
gcc/cp/coroutines.cc | 5 +++++
gcc/testsuite/g++.dg/coroutines/pr95518.C | 27 +++++++++++++++++++++++
2 files changed, 32 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95518.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 93f1e5ca30d..4f7356e94e5 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3530,12 +3530,17 @@ act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name)
tree fn_name = get_fn_local_identifier (orig, name);
tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type);
DECL_CONTEXT (fn) = DECL_CONTEXT (orig);
+ DECL_ARTIFICIAL (fn) = true;
DECL_INITIAL (fn) = error_mark_node;
tree id = get_identifier ("frame_ptr");
tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
DECL_CONTEXT (fp) = fn;
DECL_ARG_TYPE (fp) = type_passed_as (coro_frame_ptr);
DECL_ARGUMENTS (fn) = fp;
+ /* Copy used-ness from the original function. */
+ TREE_USED (fn) = TREE_USED (orig);
+ /* Apply attributes from the original fn. */
+ DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
return fn;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95518.C b/gcc/testsuite/g++.dg/coroutines/pr95518.C
new file mode 100644
index 00000000000..2d7dd049e1b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95518.C
@@ -0,0 +1,27 @@
+// { dg-additional-options "-O -Wunused-function" }
+
+#if __has_include (<coroutine>)
+#include <coroutine>
+using namespace std;
+#elif defined (__clang__) && __has_include (<experimental/coroutine>)
+#include <experimental/coroutine>
+namespace std { using namespace experimental; }
+#endif
+
+struct dummy
+{
+ struct promise_type
+ {
+ dummy get_return_object() const noexcept { return {}; }
+ std::suspend_never initial_suspend() const noexcept { return {}; }
+ std::suspend_never final_suspend() const noexcept { return {}; }
+ void return_void() const noexcept {}
+ void unhandled_exception() const noexcept {}
+ };
+ int i; // work around #95516
+};
+
+[[maybe_unused]] static dummy foo()
+{
+ co_return;
+}
--
2.24.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] coroutines: Copy attributes to the outlined functions [PR95518]
2020-06-11 19:53 [PATCH] coroutines: Copy attributes to the outlined functions [PR95518] Iain Sandoe
@ 2020-06-15 13:57 ` Nathan Sidwell
2020-06-24 11:00 ` [PATCH v2] coroutines: Copy attributes to the outlined functions [PR95518, PR95813] Iain Sandoe
0 siblings, 1 reply; 4+ messages in thread
From: Nathan Sidwell @ 2020-06-15 13:57 UTC (permalink / raw)
To: Iain Sandoe, GCC Patches
On 6/11/20 3:53 PM, Iain Sandoe wrote:
> Hi
>
> We had omitted the copying of function attributes (including the
> 'used' status). Mark the outlined functions as artificial, since
> they are; some diagnostic processing tests this.
Do we do the right thing for say attribute((section("bob"))? what if the user
tries attribute((alias("bob")), presumable we don't want both ramp and actor to
alias bob? I think this might be tricky.
> tested on Linux and Darwin,
> OK for master?
> 10.2?
> thanks
> Iain
>
> gcc/cp/ChangeLog:
>
> PR c++/95518
> * coroutines.cc (act_des_fn): Copy function attributes from
> the user’s function decl onto the outlined helpers.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/95518
> * g++.dg/coroutines/pr95518.C: New test.
> ---
> gcc/cp/coroutines.cc | 5 +++++
> gcc/testsuite/g++.dg/coroutines/pr95518.C | 27 +++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95518.C
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 93f1e5ca30d..4f7356e94e5 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -3530,12 +3530,17 @@ act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name)
> tree fn_name = get_fn_local_identifier (orig, name);
> tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type);
> DECL_CONTEXT (fn) = DECL_CONTEXT (orig);
> + DECL_ARTIFICIAL (fn) = true;
> DECL_INITIAL (fn) = error_mark_node;
> tree id = get_identifier ("frame_ptr");
> tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
> DECL_CONTEXT (fp) = fn;
> DECL_ARG_TYPE (fp) = type_passed_as (coro_frame_ptr);
> DECL_ARGUMENTS (fn) = fp;
> + /* Copy used-ness from the original function. */
> + TREE_USED (fn) = TREE_USED (orig);
> + /* Apply attributes from the original fn. */
> + DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
> return fn;
> }
>
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr95518.C b/gcc/testsuite/g++.dg/coroutines/pr95518.C
> new file mode 100644
> index 00000000000..2d7dd049e1b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr95518.C
> @@ -0,0 +1,27 @@
> +// { dg-additional-options "-O -Wunused-function" }
> +
> +#if __has_include (<coroutine>)
> +#include <coroutine>
> +using namespace std;
> +#elif defined (__clang__) && __has_include (<experimental/coroutine>)
> +#include <experimental/coroutine>
> +namespace std { using namespace experimental; }
> +#endif
> +
> +struct dummy
> +{
> + struct promise_type
> + {
> + dummy get_return_object() const noexcept { return {}; }
> + std::suspend_never initial_suspend() const noexcept { return {}; }
> + std::suspend_never final_suspend() const noexcept { return {}; }
> + void return_void() const noexcept {}
> + void unhandled_exception() const noexcept {}
> + };
> + int i; // work around #95516
> +};
> +
> +[[maybe_unused]] static dummy foo()
> +{
> + co_return;
> +}
>
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] coroutines: Copy attributes to the outlined functions [PR95518, PR95813]
2020-06-15 13:57 ` Nathan Sidwell
@ 2020-06-24 11:00 ` Iain Sandoe
2020-06-24 13:35 ` Nathan Sidwell
0 siblings, 1 reply; 4+ messages in thread
From: Iain Sandoe @ 2020-06-24 11:00 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: GCC Patches
Hi Nathan,
Nathan Sidwell <nathan@acm.org> wrote:
> On 6/11/20 3:53 PM, Iain Sandoe wrote:
>> Hi
>> We had omitted the copying of function attributes (including the
>> 'used' status). Mark the outlined functions as artificial, since
>> they are; some diagnostic processing tests this.
>
> Do we do the right thing for say attribute((section("bob"))?
I’ve made sure of that in the attached patch (also user-defined alignment).
> what if the user tries attribute((alias("bob")),
> presumable we don't want both ramp and actor to alias bob? I think this might be tricky.
As we discussed off-list, there might be some attributes that are inappropiate for coroutines.
We can diagnose those (when we understand what they are) when building the ramp / splitting the actor out (as a separate patch, when it becomes required; i.e. not part of this patch).
The attached patch copies all attributes from the original function (and those that are already on the decl) onto the outlined ones - this fixes 95518 and 95813 (which is almost a dup).
OK now for master / 10.2?
thanks,
Iain
coroutines: Copy attributes to the outlined functions
[PR95518,PR95813]
We had omitted the copying of function attributes, we now copy
the used, alignment, section values from the original decal and
the complete set of function attributes. It is likely that some
function attributes don't really make sense for coroutines, but
that can be disgnosed separately.
Also mark the outlined functions as artificial, since they are; and
some diagnostic processing tests this.
gcc/cp/ChangeLog:
PR c++/95518
PR c++/95813
* coroutines.cc (act_des_fn): Copy function
attributes onto the outlined coroutine helpers.
gcc/testsuite/ChangeLog:
PR c++/95518
PR c++/95813
* g++.dg/coroutines/pr95518.C: New test.
* g++.dg/coroutines/pr95813.C: New test.
---
gcc/cp/coroutines.cc | 12 ++++++
gcc/testsuite/g++.dg/coroutines/pr95518.C | 28 ++++++++++++++
gcc/testsuite/g++.dg/coroutines/pr95813.C | 46 +++++++++++++++++++++++
3 files changed, 86 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95518.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95813.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 9cdb0c591d5..64b97535c8d 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3530,12 +3530,24 @@ act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name)
tree fn_name = get_fn_local_identifier (orig, name);
tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type);
DECL_CONTEXT (fn) = DECL_CONTEXT (orig);
+ DECL_ARTIFICIAL (fn) = true;
DECL_INITIAL (fn) = error_mark_node;
tree id = get_identifier ("frame_ptr");
tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
DECL_CONTEXT (fp) = fn;
DECL_ARG_TYPE (fp) = type_passed_as (coro_frame_ptr);
DECL_ARGUMENTS (fn) = fp;
+ /* Copy selected attributes from the original function. */
+ TREE_USED (fn) = TREE_USED (orig);
+ if (DECL_SECTION_NAME (orig))
+ set_decl_section_name (fn, DECL_SECTION_NAME (orig));
+ /* Copy any alignment that the FE added. */
+ if (DECL_ALIGN (orig))
+ SET_DECL_ALIGN (fn, DECL_ALIGN (orig));
+ /* Copy any alignment the user added. */
+ DECL_USER_ALIGN (fn) = DECL_USER_ALIGN (orig);
+ /* Apply attributes from the original fn. */
+ DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
return fn;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95518.C b/gcc/testsuite/g++.dg/coroutines/pr95518.C
new file mode 100644
index 00000000000..b1717677810
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95518.C
@@ -0,0 +1,28 @@
+// { dg-additional-options "-O -Wunused-function" }
+
+#if __has_include (<coroutine>)
+#include <coroutine>
+using namespace std;
+#elif defined (__clang__) && __has_include (<experimental/coroutine>)
+#include <experimental/coroutine>
+namespace std { using namespace experimental; }
+#endif
+
+struct dummy
+{
+ struct promise_type
+ {
+ dummy get_return_object() const noexcept { return {}; }
+ std::suspend_never initial_suspend() const noexcept { return {}; }
+ std::suspend_never final_suspend() const noexcept { return {}; }
+ void return_void() const noexcept {}
+ void unhandled_exception() const noexcept {}
+ };
+};
+
+// This checks that the attribute is passed on to the outlined coroutine
+// functions (so that there should be no diagnostic).
+[[maybe_unused]] static dummy foo()
+{
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95813.C b/gcc/testsuite/g++.dg/coroutines/pr95813.C
new file mode 100644
index 00000000000..445cdf1f7ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95813.C
@@ -0,0 +1,46 @@
+// { dg-additional-options "-Wall -O" }
+
+// This should complete without any diagnostic.
+
+#include <coroutine>
+#include <exception>
+
+template <typename T>
+class lazy {
+ T _v = 0;
+public:
+ lazy() {}
+ bool await_ready() {return true;}
+ void await_suspend(auto x) noexcept {}
+ T await_resume() { return _v; }
+};
+
+namespace std {
+
+template <typename T, typename... Args>
+struct coroutine_traits<lazy<T>, Args...> {
+ struct promise_type {
+ suspend_always initial_suspend() const { return {}; }
+ suspend_always final_suspend() const { return {}; }
+ void return_value(T val) {}
+ lazy<T> get_return_object() {
+ return lazy<T>();
+ }
+ void unhandled_exception() {
+ std::terminate();
+ }
+ };
+};
+}
+
+struct xxx {
+ static lazy<int> func() {
+ co_return 1;
+ }
+};
+
+#if 0
+lazy<int> foo() {
+ co_return co_await xxx::func();
+}
+#endif
--
2.24.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] coroutines: Copy attributes to the outlined functions [PR95518, PR95813]
2020-06-24 11:00 ` [PATCH v2] coroutines: Copy attributes to the outlined functions [PR95518, PR95813] Iain Sandoe
@ 2020-06-24 13:35 ` Nathan Sidwell
0 siblings, 0 replies; 4+ messages in thread
From: Nathan Sidwell @ 2020-06-24 13:35 UTC (permalink / raw)
To: Iain Sandoe; +Cc: GCC Patches
On 6/24/20 7:00 AM, Iain Sandoe wrote:
> Hi Nathan,
>
> Nathan Sidwell <nathan@acm.org> wrote:
>
>> On 6/11/20 3:53 PM, Iain Sandoe wrote:
>>> Hi
>>> We had omitted the copying of function attributes (including the
>>> 'used' status). Mark the outlined functions as artificial, since
>>> they are; some diagnostic processing tests this.
>>
>> Do we do the right thing for say attribute((section("bob"))?
>
> I’ve made sure of that in the attached patch (also user-defined alignment).
>
>> what if the user tries attribute((alias("bob")),
>> presumable we don't want both ramp and actor to alias bob? I think this might be tricky.
>
> As we discussed off-list, there might be some attributes that are inappropiate for coroutines.
> We can diagnose those (when we understand what they are) when building the ramp / splitting the actor out (as a separate patch, when it becomes required; i.e. not part of this patch).
>
> The attached patch copies all attributes from the original function (and those that are already on the decl) onto the outlined ones - this fixes 95518 and 95813 (which is almost a dup).
>
> OK now for master / 10.2?
ok, thanks for checking!
> thanks,
> Iain
>
> coroutines: Copy attributes to the outlined functions
> [PR95518,PR95813]
>
> We had omitted the copying of function attributes, we now copy
> the used, alignment, section values from the original decal and
> the complete set of function attributes. It is likely that some
> function attributes don't really make sense for coroutines, but
> that can be disgnosed separately.
> Also mark the outlined functions as artificial, since they are; and
> some diagnostic processing tests this.
>
> gcc/cp/ChangeLog:
>
> PR c++/95518
> PR c++/95813
> * coroutines.cc (act_des_fn): Copy function
> attributes onto the outlined coroutine helpers.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/95518
> PR c++/95813
> * g++.dg/coroutines/pr95518.C: New test.
> * g++.dg/coroutines/pr95813.C: New test.
> ---
> gcc/cp/coroutines.cc | 12 ++++++
> gcc/testsuite/g++.dg/coroutines/pr95518.C | 28 ++++++++++++++
> gcc/testsuite/g++.dg/coroutines/pr95813.C | 46 +++++++++++++++++++++++
> 3 files changed, 86 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95518.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95813.C
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 9cdb0c591d5..64b97535c8d 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -3530,12 +3530,24 @@ act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name)
> tree fn_name = get_fn_local_identifier (orig, name);
> tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type);
> DECL_CONTEXT (fn) = DECL_CONTEXT (orig);
> + DECL_ARTIFICIAL (fn) = true;
> DECL_INITIAL (fn) = error_mark_node;
> tree id = get_identifier ("frame_ptr");
> tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
> DECL_CONTEXT (fp) = fn;
> DECL_ARG_TYPE (fp) = type_passed_as (coro_frame_ptr);
> DECL_ARGUMENTS (fn) = fp;
> + /* Copy selected attributes from the original function. */
> + TREE_USED (fn) = TREE_USED (orig);
> + if (DECL_SECTION_NAME (orig))
> + set_decl_section_name (fn, DECL_SECTION_NAME (orig));
> + /* Copy any alignment that the FE added. */
> + if (DECL_ALIGN (orig))
> + SET_DECL_ALIGN (fn, DECL_ALIGN (orig));
> + /* Copy any alignment the user added. */
> + DECL_USER_ALIGN (fn) = DECL_USER_ALIGN (orig);
> + /* Apply attributes from the original fn. */
> + DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
> return fn;
> }
>
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr95518.C b/gcc/testsuite/g++.dg/coroutines/pr95518.C
> new file mode 100644
> index 00000000000..b1717677810
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr95518.C
> @@ -0,0 +1,28 @@
> +// { dg-additional-options "-O -Wunused-function" }
> +
> +#if __has_include (<coroutine>)
> +#include <coroutine>
> +using namespace std;
> +#elif defined (__clang__) && __has_include (<experimental/coroutine>)
> +#include <experimental/coroutine>
> +namespace std { using namespace experimental; }
> +#endif
> +
> +struct dummy
> +{
> + struct promise_type
> + {
> + dummy get_return_object() const noexcept { return {}; }
> + std::suspend_never initial_suspend() const noexcept { return {}; }
> + std::suspend_never final_suspend() const noexcept { return {}; }
> + void return_void() const noexcept {}
> + void unhandled_exception() const noexcept {}
> + };
> +};
> +
> +// This checks that the attribute is passed on to the outlined coroutine
> +// functions (so that there should be no diagnostic).
> +[[maybe_unused]] static dummy foo()
> +{
> + co_return;
> +}
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr95813.C b/gcc/testsuite/g++.dg/coroutines/pr95813.C
> new file mode 100644
> index 00000000000..445cdf1f7ef
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr95813.C
> @@ -0,0 +1,46 @@
> +// { dg-additional-options "-Wall -O" }
> +
> +// This should complete without any diagnostic.
> +
> +#include <coroutine>
> +#include <exception>
> +
> +template <typename T>
> +class lazy {
> + T _v = 0;
> +public:
> + lazy() {}
> + bool await_ready() {return true;}
> + void await_suspend(auto x) noexcept {}
> + T await_resume() { return _v; }
> +};
> +
> +namespace std {
> +
> +template <typename T, typename... Args>
> +struct coroutine_traits<lazy<T>, Args...> {
> + struct promise_type {
> + suspend_always initial_suspend() const { return {}; }
> + suspend_always final_suspend() const { return {}; }
> + void return_value(T val) {}
> + lazy<T> get_return_object() {
> + return lazy<T>();
> + }
> + void unhandled_exception() {
> + std::terminate();
> + }
> + };
> +};
> +}
> +
> +struct xxx {
> + static lazy<int> func() {
> + co_return 1;
> + }
> +};
> +
> +#if 0
> +lazy<int> foo() {
> + co_return co_await xxx::func();
> +}
> +#endif
>
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-24 13:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 19:53 [PATCH] coroutines: Copy attributes to the outlined functions [PR95518] Iain Sandoe
2020-06-15 13:57 ` Nathan Sidwell
2020-06-24 11:00 ` [PATCH v2] coroutines: Copy attributes to the outlined functions [PR95518, PR95813] Iain Sandoe
2020-06-24 13:35 ` Nathan Sidwell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).