From patchwork Mon Jan 20 09:25:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: JunMa X-Patchwork-Id: 1225789 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-517723-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=Y7UF4oAT; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 481R9h1N1gz9sR1 for ; Mon, 20 Jan 2020 20:25:58 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:message-id:date:mime-version:content-type; q=dns; s= default; b=bMO6tk+hsPl+se7TiRcsDuQbAxY1xYXheH4yEhexQGAB0vfQamNWu 6L5V4+h7Tai04ZeIAJsqscYYN04HWf4WPyGRpwfKnomrnDNO5RkTs+GUJ3vWqn40 IMbwM/vfHKa2aOblLyFlJoiVJ0tYU4Rlg1HXeSdA6yJkQcSdV8v3+Y= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:message-id:date:mime-version:content-type; s= default; bh=4x8aJ9j7Xs4FGZ/OPx3TwGTdFRI=; b=Y7UF4oATgkrbSS6wryDV RKgpDvm9Q3D/lYGE3E7/88LifxkJFIAt6HWCfahBVHAEGKOR/mT8x6sDWu6B+laP 8HUGQV7cqZgBUtp01wYsPvVIh6PVGUZbyYnI9dmO28TQZBi/sE1wRAoJsAXmjt3e Fs3h/bVYj4JiC6BWHSmsKAs= Received: (qmail 83125 invoked by alias); 20 Jan 2020 09:25:49 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 83117 invoked by uid 89); 20 Jan 2020 09:25:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-27.0 required=5.0 tests=AWL, BAYES_00, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS, UNPARSEABLE_RELAY, USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy= X-HELO: out30-44.freemail.mail.aliyun.com Received: from out30-44.freemail.mail.aliyun.com (HELO out30-44.freemail.mail.aliyun.com) (115.124.30.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Jan 2020 09:25:39 +0000 X-Alimail-AntiSpam: AC=PASS; BC=-1|-1; BR=01201311R121e4; CH=green; DM=||false|; DS=||; FP=0|-1|-1|-1|0|-1|-1|-1; HT=e01f04455; MF=junma@linux.alibaba.com; NM=1; PH=DS; RN=2; SR=0; TI=SMTPD_---0ToDTOh7_1579512323; Received: from MacBook-Pro-6.local(mailfrom:JunMa@linux.alibaba.com fp:SMTPD_---0ToDTOh7_1579512323) by smtp.aliyun-inc.com(127.0.0.1); Mon, 20 Jan 2020 17:25:35 +0800 From: JunMa Subject: [PATCH Coroutines] Fix issue with unused corutine function parameters To: gcc-patches , Iain Sandoe Message-ID: <490e331a-e1f2-bee4-e150-4df87c752bfc@linux.alibaba.com> Date: Mon, 20 Jan 2020 17:25:23 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 X-IsSubscribed: yes Hi Accroding to N4835: When a coroutine is invoked, a copy is created for each coroutine parameter. While in current implementation, we only collect used parameters. This may lost behavior inside constructor and destructor of unused parameters. This patch change the implementation to collect all parameter. Bootstrap and test on X86_64, is it OK? Regards JunMa gcc/cp 2020-01-20  Jun Ma         * coroutines.cc (build_actor_fn): Skip rewrite when there is no         param references.         (register_param_uses): Only collect param references here.         (morph_fn_to_coro): Create coroutine frame field for each         function params. gcc/testsuite 2020-01-20  Jun Ma         * g++.dg/coroutines/torture/func-params-07.C: New test. --- gcc/cp/coroutines.cc | 63 +++++++---------- .../coroutines/torture/func-params-07.C | 68 +++++++++++++++++++ 2 files changed, 93 insertions(+), 38 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/func-params-07.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 49e509f4384..74e0f6d1785 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -1877,6 +1877,8 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, actor_frame, fld_ref, NULL_TREE); int i; tree *puse; + if (parm.body_uses == NULL) + continue; FOR_EACH_VEC_ELT (*parm.body_uses, i, puse) { *puse = fld_idx; @@ -2717,7 +2719,6 @@ struct param_frame_data tree *field_list; hash_map *param_uses; location_t loc; - bool param_seen; }; static tree @@ -2731,30 +2732,10 @@ register_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d) bool existed; param_info &parm = data->param_uses->get_or_insert (*stmt, &existed); gcc_checking_assert (existed); - - if (parm.field_id == NULL_TREE) - { - tree actual_type = TREE_TYPE (*stmt); - - if (!COMPLETE_TYPE_P (actual_type)) - actual_type = complete_type_or_else (actual_type, *stmt); - - if (TREE_CODE (actual_type) == REFERENCE_TYPE) - actual_type = build_pointer_type (TREE_TYPE (actual_type)); - - parm.frame_type = actual_type; - tree pname = DECL_NAME (*stmt); - size_t namsize = sizeof ("__parm.") + IDENTIFIER_LENGTH (pname) + 1; - char *buf = (char *) alloca (namsize); - snprintf (buf, namsize, "__parm.%s", IDENTIFIER_POINTER (pname)); - parm.field_id - = coro_make_frame_entry (data->field_list, buf, actual_type, data->loc); - vec_alloc (parm.body_uses, 4); - parm.body_uses->quick_push (stmt); - data->param_seen = true; - } - else - parm.body_uses->safe_push (stmt); + gcc_checking_assert (parm.field_id != NULL_TREE); + if (parm.body_uses == NULL) + vec_alloc (parm.body_uses, 4); + parm.body_uses->safe_push (stmt); return NULL_TREE; } @@ -3060,6 +3041,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) The second two entries start out empty - and only get populated when we see uses. */ param_uses = new hash_map; + struct param_frame_data param_data + = {&field_list, param_uses, fn_start}; for (tree arg = DECL_ARGUMENTS (orig); arg != NULL; arg = DECL_CHAIN (arg)) @@ -3067,24 +3050,28 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) bool existed; param_info &parm = param_uses->get_or_insert (arg, &existed); gcc_checking_assert (!existed); - parm.field_id = NULL_TREE; parm.body_uses = NULL; + tree actual_type = TREE_TYPE (arg); + + if (!COMPLETE_TYPE_P (actual_type)) + actual_type = complete_type_or_else (actual_type, arg); + + if (TREE_CODE (actual_type) == REFERENCE_TYPE) + actual_type = build_pointer_type (TREE_TYPE (actual_type)); + + parm.frame_type = actual_type; + tree pname = DECL_NAME (arg); + size_t namsize = sizeof ("__parm.") + IDENTIFIER_LENGTH (pname) + 1; + char *buf = (char *) alloca (namsize); + snprintf (buf, namsize, "__parm.%s", IDENTIFIER_POINTER (pname)); + parm.field_id + = coro_make_frame_entry (param_data.field_list, buf, + actual_type, param_data.loc); } - param_frame_data param_data - = {&field_list, param_uses, fn_start, false}; /* We want to record every instance of param's use, so don't include a 'visited' hash_set. */ cp_walk_tree (&fnbody, register_param_uses, ¶m_data, NULL); - - /* If no uses for any param were seen, act as if there were no - params (it could be that they are only used to construct the - promise). */ - if (!param_data.param_seen) - { - delete param_uses; - param_uses = NULL; - } } /* 4. Now make space for local vars, this is conservative again, and we @@ -3333,7 +3320,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) add_stmt (r); } - /* Copy in any of the function params we found to be used. + /* Copy in all of the function params. Param types with non-trivial dtors will have to be moved into position and the dtor run before the frame is freed. */ vec *param_dtor_list = NULL; diff --git a/gcc/testsuite/g++.dg/coroutines/torture/func-params-07.C b/gcc/testsuite/g++.dg/coroutines/torture/func-params-07.C new file mode 100644 index 00000000000..7559eb36889 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/torture/func-params-07.C @@ -0,0 +1,68 @@ +// { dg-do run } + +// template parm in a class + +#include "../coro.h" + +// boiler-plate for tests of codegen +#include "../coro1-ret-int-yield-int.h" +int count=0; +struct TestAwaiter { + int recent_test; + TestAwaiter(int test) : recent_test{test} {} + ~TestAwaiter() {count++;} + bool await_ready() { return true; } + void await_suspend(coro::coroutine_handle<>) {} + int await_resume() { return recent_test;} + void return_value(int x) { recent_test = x;} +}; + +coro1 test_lam (struct TestAwaiter t) +{ + + int sum = 0; + for (int i = 0; i < 1000; ++i) + { + sum += co_await TestAwaiter(1); + } + co_return sum; +} + +int main () +{ + + PRINT ("main: create coro1"); + coro1 x = test_lam (TestAwaiter(1)); + if (x.handle.done()) + abort(); + x.handle.resume(); + PRINT ("main: after resume (initial suspend)"); + + while(!x.handle.done()) + x.handle.resume(); + PRINT ("main: after resume (co_await)"); + + /* Now we should have the co_returned value. */ + int y = x.handle.promise().get_value(); + if ( y != 1000 ) + { + PRINTF ("main: wrong result (%d).", y); + abort (); + } + + if (!x.handle.done()) + { + PRINT ("main: apparently not done..."); + abort (); + } + + x.handle.destroy(); + x.handle = NULL; + if (count != 1002) + { + PRINT ("main: missed destructor..."); + abort (); + } + PRINT ("main: returning"); + return 0; +}