* [PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps
@ 2020-02-11 2:50 JunMa
2020-02-11 8:45 ` JunMa
2020-02-27 2:17 ` [PING PATCH " JunMa
0 siblings, 2 replies; 6+ messages in thread
From: JunMa @ 2020-02-11 2:50 UTC (permalink / raw)
To: gcc-patches; +Cc: Iain Sandoe, Nathan Sidwell
Hi
As title. in maybe_promote_captured_temps, we promote captured temporaries
and co_await_expr into a new BIND_EXPR. As the BIND_EXPR contains
co_await_expr and maybe other function calls, the side effects flag should
be set.
This patch fix one mismatch in cppcoro, the testcase comes from cppcoro
and is reduced by creduce.
Bootstrap and test on X86_64, is it OK?
Regards
JunMa
gcc/cp
2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
       * coroutines.cc (maybe_promote_captured_temps): Set side effects
       flag for BIND_EXPR.
gcc/testsuite
2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
       * g++.dg/coroutines/torture/lambda-10-co-await-lambda.C: New test.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps
2020-02-11 2:50 [PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps JunMa
@ 2020-02-11 8:45 ` JunMa
2020-02-27 2:17 ` [PING PATCH " JunMa
1 sibling, 0 replies; 6+ messages in thread
From: JunMa @ 2020-02-11 8:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Iain Sandoe, Nathan Sidwell
[-- Attachment #1: Type: text/plain, Size: 912 bytes --]
å¨ 2020/2/11 ä¸å10:50, JunMa åé:
> Hi
> As title. in maybe_promote_captured_temps, we promote captured
> temporaries
> and co_await_expr into a new BIND_EXPR. As the BIND_EXPR contains
> co_await_expr and maybe other function calls, the side effects flag
> should
> be set.
>
> This patch fix one mismatch in cppcoro, the testcase comes from cppcoro
> and is reduced by creduce.
>
> Bootstrap and test on X86_64, is it OK?
>
> Regards
> JunMa
>
> gcc/cp
> 2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
>
> Â Â Â Â Â Â Â * coroutines.cc (maybe_promote_captured_temps): Set side effects
> Â Â Â Â Â Â Â flag for BIND_EXPR.
>
> gcc/testsuite
> 2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
>
> Â Â Â Â Â Â Â * g++.dg/coroutines/torture/lambda-10-co-await-lambda.C: New
> test.
Hi
Sorry for forgetting the patch. Here it is.
Regards
JunMa
[-- Attachment #2: 0001-Set-side-effects-flag-when-the-BIND_EXPR-contains-co.patch --]
[-- Type: text/plain, Size: 2193 bytes --]
---
gcc/cp/coroutines.cc | 2 +
.../torture/lambda-10-co-await-lambda.C | 47 +++++++++++++++++++
2 files changed, 49 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/lambda-10-co-await-lambda.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index decec4550af..004e0a1a659 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2735,6 +2735,8 @@ maybe_promote_captured_temps (tree *stmt, void *d)
}
BIND_EXPR_BLOCK (aw_bind) = b_block;
+ /* Set side effects flag since the BIND_EXPR contains co_await expr. */
+ TREE_SIDE_EFFECTS (aw_bind) = 1;
*stmt = aw_bind;
}
return res;
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/lambda-10-co-await-lambda.C b/gcc/testsuite/g++.dg/coroutines/torture/lambda-10-co-await-lambda.C
new file mode 100644
index 00000000000..43d4ff6b77e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/lambda-10-co-await-lambda.C
@@ -0,0 +1,47 @@
+// { dg-do run }
+
+#include "../coro.h"
+template <typename> struct a;
+struct b {
+ struct c {
+ int await_ready() {return 0;}
+ template <typename d> void await_suspend(d) {}
+ void await_resume() noexcept {}
+ };
+ auto initial_suspend() { return std::suspend_never{}; }
+ auto final_suspend() { return c{}; }
+};
+template <typename> struct e {
+ int f() {return 1;}
+};
+template <> struct e<void> : b {
+ a<void> get_return_object() noexcept;
+ void unhandled_exception();
+};
+template <typename g = void> struct a {
+ using promise_type = e<g>;
+ struct h {
+ std::coroutine_handle<promise_type> j;
+ int await_ready() {return 0;}
+ bool await_suspend(std::coroutine_handle<>) { return false;}
+ };
+ auto operator co_await() {
+ struct awaitable : h {
+ auto await_resume() { return this->j.promise().f(); }
+ };
+ return awaitable{};
+ }
+};
+a<> e<void>::get_return_object() noexcept { return a<>{};}
+
+int main() {
+ auto k = []() -> a<int> { return a<int>{};};
+ int l = 0 ;
+ auto m = [&]() -> a<> {
+ for (int i; i < 100; ++i)
+ l += co_await k();
+ };
+ m();
+ if (l != 100)
+ abort();
+}
--
2.19.1.3.ge56e4f7
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PING PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps
2020-02-11 2:50 [PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps JunMa
2020-02-11 8:45 ` JunMa
@ 2020-02-27 2:17 ` JunMa
2020-03-05 11:55 ` JunMa
1 sibling, 1 reply; 6+ messages in thread
From: JunMa @ 2020-02-27 2:17 UTC (permalink / raw)
To: gcc-patches; +Cc: Iain Sandoe, Nathan Sidwell
å¨ 2020/2/11 ä¸å10:50, JunMa åé:
Hi
kindly ping~
Regards
JunMa
> Hi
> As title. in maybe_promote_captured_temps, we promote captured
> temporaries
> and co_await_expr into a new BIND_EXPR. As the BIND_EXPR contains
> co_await_expr and maybe other function calls, the side effects flag
> should
> be set.
>
> This patch fix one mismatch in cppcoro, the testcase comes from cppcoro
> and is reduced by creduce.
>
> Bootstrap and test on X86_64, is it OK?
>
> Regards
> JunMa
>
> gcc/cp
> 2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
>
> Â Â Â Â Â Â Â * coroutines.cc (maybe_promote_captured_temps): Set side effects
> Â Â Â Â Â Â Â flag for BIND_EXPR.
>
> gcc/testsuite
> 2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
>
> Â Â Â Â Â Â Â * g++.dg/coroutines/torture/lambda-10-co-await-lambda.C: New
> test.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PING PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps
2020-02-27 2:17 ` [PING PATCH " JunMa
@ 2020-03-05 11:55 ` JunMa
2020-03-05 13:52 ` Iain Sandoe
0 siblings, 1 reply; 6+ messages in thread
From: JunMa @ 2020-03-05 11:55 UTC (permalink / raw)
To: gcc-patches; +Cc: Iain Sandoe, Nathan Sidwell
Ping
Regards
JunMa
å¨ 2020/2/27 ä¸å10:17, JunMa åé:
> å¨ 2020/2/11 ä¸å10:50, JunMa åé:
> Hi
> kindly ping~
>
> Regards
> JunMa
>> Hi
>> As title. in maybe_promote_captured_temps, we promote captured
>> temporaries
>> and co_await_expr into a new BIND_EXPR. As the BIND_EXPR contains
>> co_await_expr and maybe other function calls, the side effects flag
>> should
>> be set.
>>
>> This patch fix one mismatch in cppcoro, the testcase comes from cppcoro
>> and is reduced by creduce.
>>
>> Bootstrap and test on X86_64, is it OK?
>>
>> Regards
>> JunMa
>>
>> gcc/cp
>> 2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
>>
>> Â Â Â Â Â Â Â * coroutines.cc (maybe_promote_captured_temps): Set side effects
>> Â Â Â Â Â Â Â flag for BIND_EXPR.
>>
>> gcc/testsuite
>> 2020-02-11Â Jun Ma <JunMa@linux.alibaba.com>
>>
>> Â Â Â Â Â Â Â * g++.dg/coroutines/torture/lambda-10-co-await-lambda.C: New
>> test.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PING PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps
2020-03-05 11:55 ` JunMa
@ 2020-03-05 13:52 ` Iain Sandoe
2020-03-06 5:44 ` JunMa
0 siblings, 1 reply; 6+ messages in thread
From: Iain Sandoe @ 2020-03-05 13:52 UTC (permalink / raw)
To: JunMa; +Cc: gcc-patches, Nathan Sidwell
Hello JunMa,
JunMa <JunMa@linux.alibaba.com> wrote:
> Ping
Thanks for your patch(es) and I am sorry this has taken some time to review.
(right now, weâre trying to ensure that we have the latest standard
represented in
GCC10, so updating to n4849).
>
> å¨ 2020/2/27 ä¸å10:17, JunMa åé:
>> å¨ 2020/2/11 ä¸å10:50, JunMa åé:
>> Hi
>> kindly ping~
>>
>> Regards
>> JunMa
>>> Hi
>>> As title. in maybe_promote_captured_temps, we promote captured
>>> temporaries
>>> and co_await_expr into a new BIND_EXPR. As the BIND_EXPR contains
>>> co_await_expr and maybe other function calls, the side effects flag
>>> should
>>> be set.
>>>
>>> This patch fix one mismatch in cppcoro, the testcase comes from cppcoro
>>> and is reduced by creduce.
With the following test conditions;
r10-7040-ga2ec7c4aafbcd517
+ the two approved patches by Bin Cheng applied.
1/ the test case in this patch (lambda-10-co-await-lambda.C) fails both with and without the patch.
2/ the patch regresses one of my local testcases.
So, it appears that the testcase might show a bug - but the fix is not the
right one for current trunk?
Please could you re-check ?
thanks
Iain
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PING PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps
2020-03-05 13:52 ` Iain Sandoe
@ 2020-03-06 5:44 ` JunMa
0 siblings, 0 replies; 6+ messages in thread
From: JunMa @ 2020-03-06 5:44 UTC (permalink / raw)
To: Iain Sandoe; +Cc: gcc-patches, Nathan Sidwell
å¨ 2020/3/5 ä¸å9:51, Iain Sandoe åé:
> Hello JunMa,
>
> JunMa <JunMa@linux.alibaba.com> wrote:
>
>> Ping
>
> Thanks for your patch(es) and I am sorry this has taken some time to
> review.
>
> (right now, weâre trying to ensure that we have the latest standard
> represented in
> Â GCC10, so updating to n4849).
>>
>> å¨ 2020/2/27 ä¸å10:17, JunMa åé:
>>> å¨ 2020/2/11 ä¸å10:50, JunMa åé:
>>> Hi
>>> kindly ping~
>>>
>>> Regards
>>> JunMa
>>>> Hi
>>>> As title. in maybe_promote_captured_temps, we promote captured
>>>> temporaries
>>>> and co_await_expr into a new BIND_EXPR. As the BIND_EXPR contains
>>>> co_await_expr and maybe other function calls, the side effects flag
>>>> should
>>>> be set.
>>>>
>>>> This patch fix one mismatch in cppcoro, the testcase comes from
>>>> cppcoro
>>>> and is reduced by creduce.
>
> With the following test conditions;
>
> r10-7040-ga2ec7c4aafbcd517
> Â + the two approved patches by Bin Cheng applied.
>
> Â 1/ the test case in this patch (lambda-10-co-await-lambda.C) fails
> both with and without the patch.
> Â 2/ the patch regresses one of my local testcases.
>
The test case fails because of ICE which is fixed by the
[PING PATCH coroutines] Do not strip cleanup_point when promote
temporaries out of current stmt
This patch fix the runtime mismatch.
The extra regression is co-await-syntax-11.C which comes from Bin's
patch and also is fixed by that
patch.
Regards
JunMa
> So, it appears that the testcase might show a bug - but the fix is not
> the right one for current trunk?
>
> Please could you re-check ?
>
> thanks
> Iain
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-06 5:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 2:50 [PATCH coroutines] Set side effects flag for BIND_EXPR which build in maybe_promote_captured_temps JunMa
2020-02-11 8:45 ` JunMa
2020-02-27 2:17 ` [PING PATCH " JunMa
2020-03-05 11:55 ` JunMa
2020-03-05 13:52 ` Iain Sandoe
2020-03-06 5:44 ` JunMa
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).