From 234681eadf2c51d7b78270188d64601b7267330d Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Sun, 3 May 2020 14:20:13 +0100 Subject: [PATCH] coroutines: Update TREE_SIDE_EFFECTS on inserted bind exprs. There are several places where we insert bind expressions while making the coroutine AST transforms. These should be marked as having side-effects where relevant, which had been omitted. This leads to at least one failure in the cppcoros test suite, where a loop body is dropped in gimplification because it is not marked. gcc/cp/ChangeLog: 2020-05-08 Iain Sandoe PR c++/95003 * coroutines.cc (build_actor_fn): Ensure that bind scopes are marked as having side-effects where necessary. (replace_statement_captures): Likewise. (morph_fn_to_coro): Likewise. gcc/testsuite/ChangeLog: 2020-05-08 Iain Sandoe PR c++/95003 * g++.dg/coroutines/torture/pr95003.C: New test. --- gcc/cp/ChangeLog | 8 +++ gcc/cp/coroutines.cc | 15 ++++-- gcc/testsuite/ChangeLog | 5 ++ .../g++.dg/coroutines/torture/pr95003.C | 50 +++++++++++++++++++ 4 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/pr95003.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index f3ab02237bd..508ef7026a6 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,11 @@ +2020-05-08 Iain Sandoe + + PR c++/95003 + * coroutines.cc (build_actor_fn): Ensure that bind scopes + are marked as having side-effects where necessary. + (replace_statement_captures): Likewise. + (morph_fn_to_coro): Likewise. + 2020-05-08 Nathan Sidwell * NEWS: Delete, it is so stale. diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 8f0f2d5960d..730e6fef82a 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -1973,8 +1973,6 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, current_stmt_tree ()->stmts_are_full_exprs_p = 1; tree stmt = begin_compound_stmt (BCS_FN_BODY); - /* ??? Can we dispense with the enclosing bind if the function body does - not start with a bind_expr? (i.e. there's no contained scopes). */ tree actor_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL); tree top_block = make_node (BLOCK); BIND_EXPR_BLOCK (actor_bind) = top_block; @@ -2425,8 +2423,8 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, continue_label, continuation, 2}; cp_walk_tree (&actor_body, await_statement_expander, &data, NULL); - actor_body = pop_stmt_list (actor_body); - BIND_EXPR_BODY (actor_bind) = actor_body; + BIND_EXPR_BODY (actor_bind) = pop_stmt_list (actor_body); + TREE_SIDE_EFFECTS (actor_bind) = true; finish_compound_stmt (stmt); DECL_SAVED_TREE (actor) = pop_stmt_list (actor_outer); @@ -2891,6 +2889,7 @@ replace_statement_captures (tree *stmt, void *d) } } BIND_EXPR_BLOCK (aw_bind) = b_block; + TREE_SIDE_EFFECTS (aw_bind) = TREE_SIDE_EFFECTS (BIND_EXPR_BODY (aw_bind)); *stmt = aw_bind; } @@ -3613,10 +3612,13 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) { tree tlist = NULL_TREE; append_to_statement_list_force (fnbody, &tlist); + TREE_SIDE_EFFECTS (tlist) = TREE_SIDE_EFFECTS (fnbody); BIND_EXPR_BODY (update_body) = tlist; } tree new_body_list = NULL_TREE; - append_to_statement_list_force (update_body, &new_body_list); + TREE_SIDE_EFFECTS (update_body) = true; + append_to_statement_list (update_body, &new_body_list); + TREE_SIDE_EFFECTS (new_body_list) = true; fnbody = new_body_list; } @@ -4323,7 +4325,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) /* Finish up the ramp function. */ BIND_EXPR_VARS (gro_context_bind) = gro_bind_vars; BIND_EXPR_BODY (gro_context_bind) = pop_stmt_list (gro_context_body); + TREE_SIDE_EFFECTS (gro_context_bind) = true; BIND_EXPR_BODY (ramp_bind) = pop_stmt_list (ramp_body); + TREE_SIDE_EFFECTS (ramp_bind) = true; /* We know the "real" promise and have a frame layout with a slot for each suspend point, so we can build an actor function (which contains the @@ -4442,6 +4446,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) BLOCK_SUPERCONTEXT (replace_blk) = tcb_block; BLOCK_SUBBLOCKS (tcb_block) = replace_blk; BIND_EXPR_BLOCK (fnbody) = tcb_block; + TREE_SIDE_EFFECTS (fnbody) = true; } } else if (pedantic) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 2381a1c80f9..69c3cac3d71 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-05-08 Iain Sandoe + + PR c++/95003 + * g++.dg/coroutines/torture/pr95003.C: New test. + 2020-05-08 Nathan Sidwell * gcc.dg/cpp/counter-[23].c: Move to c-c+_-common/cpp. diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr95003.C b/gcc/testsuite/g++.dg/coroutines/torture/pr95003.C new file mode 100644 index 00000000000..eda785827ec --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/torture/pr95003.C @@ -0,0 +1,50 @@ +// { dg-do run } + +#include "../coro.h" +#include "../coro1-ret-int-yield-int.h" + +// This tests that, when we insert bind scopes to contain variables that +// have been promoted from compiler temporaries to extend their lifetimes +// to a containing full expression, the inserted bind scopes have their +// tree-side-effects set. + +struct Awaitable { + int v; + Awaitable (int _v) : v(_v) {} + bool await_ready() { return false; } + void await_suspend(std::coroutine_handle) {} + int await_resume() { return v; } + auto operator co_await() { return *this; } +}; + +coro1 +my_coro +(int x) +{ + int sum = 0; + for (unsigned i = 0; i < 100; ++i) { + sum += co_await Awaitable{x+1}; + } + co_return sum; +} + +int main () +{ + PRINT ("main: create coro1"); + struct coro1 f_coro = my_coro (0); + + PRINT ("main: OK -- looping"); + + do { + f_coro.handle.resume(); + } while (!f_coro.handle.done()); + + int y = f_coro.handle.promise().get_value(); + if (y != 100) + { + PRINTF ("main: y is wrong : %d, should be 100\n", y); + abort (); + } + puts ("main: done"); + return 0; +} -- 2.30.2