From patchwork Tue Feb 4 12:17:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: JunMa X-Patchwork-Id: 1233324 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-518872-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=gOB7vgEu; 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 48BkGb3dNczB3xC for ; Tue, 4 Feb 2020 23:17:25 +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:to:cc :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=WqSZ0CyO5Qr0HBdLsJRRj4nqE8jKQaxHmP4K3PktYcJ2eHbKYD wd1LQHS4+DUNtWuFtIFviIxT9AlX1ROAb2Glq8kSMYBrRxaJjxSCqTXosWt41RDF QIKagYWXA1ia5n9iAKAK91fIOC+xT1EpFP0CihdV2bz2ce279Q0D+OjsE= 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:to:cc :from:subject:message-id:date:mime-version:content-type; s= default; bh=aUfKqGU59zT4ao1JT7zn2pR9gqo=; b=gOB7vgEuAG1uaCGK+MgJ vqkmmy4TYnN7qFRQc2QLlpqacrAKoOi/9x32V5Lr3HEZgWxQaj3aFBslQibM9Emz CUAFNTOo0n+DrFhQgPRjqGQ9XeblYs5dMRpz7qfR1iMEn9F7q0kD1HC3ZU7rY7Og vcJggbssIvF91dmss8C99Pc= Received: (qmail 49425 invoked by alias); 4 Feb 2020 12:17:18 -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 49417 invoked by uid 89); 4 Feb 2020 12:17:17 -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=UD:D, build1_loc, X86_64, gro X-HELO: out30-132.freemail.mail.aliyun.com Received: from out30-132.freemail.mail.aliyun.com (HELO out30-132.freemail.mail.aliyun.com) (115.124.30.132) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Feb 2020 12:17:15 +0000 X-Alimail-AntiSpam: AC=PASS; BC=-1|-1; BR=01201311R751e4; CH=green; DM=||false|; DS=||; FP=0|-1|-1|-1|0|-1|-1|-1; HT=e01f04427; MF=junma@linux.alibaba.com; NM=1; PH=DS; RN=3; SR=0; TI=SMTPD_---0Tp7o2u4_1580818630; Received: from 30.27.192.13(mailfrom:JunMa@linux.alibaba.com fp:SMTPD_---0Tp7o2u4_1580818630) by smtp.aliyun-inc.com(127.0.0.1); Tue, 04 Feb 2020 20:17:11 +0800 To: gcc-patches Cc: Iain Sandoe , Nathan Sidwell From: JunMa Subject: [PATCH coroutines] Change lowering behavior of alias variable from copy to substitute Message-ID: <18c7ceea-6529-a6fa-b2cc-ad99c3a9df2e@linux.alibaba.com> Date: Tue, 4 Feb 2020 20:17:10 +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 When testing coroutines with lambda function, I find we copy each captured variable to frame. However, according to gimplify pass, for each declaration that is an alias for another expression(DECL_VALUE_EXPR), we can substitute them directly. Since lambda captured variables is one of this kind. It is better to replace them rather than copy them, This can reduce frame size (all of the captured variables are field of closure class) and avoid extra copy behavior as well. This patch remove all of the code related to copy captured variable. Instead, we first rewrite DECL_VALUE_EXPR with frame field, then we check every variable whether it has DECL_VALUE_EXPR, and then substitute it, this patch does not create frame field for such variables. Bootstrap and test on X86_64, is it OK? Regards JunMa gcc/cp 2020-02-04  Jun Ma         * coroutines.cc (morph_fn_to_coro): Remove code related to         copy captured variable.         (register_local_var_uses):  Ditto.         (register_param_uses):  Collect use of parameters inside         DECL_VALUE_EXPR.         (transform_local_var_uses): Substitute the alias variable         with DECL_VALUE_EXPR if it has one. gcc/testsuite 2020-02-04  Jun Ma         * g++.dg/coroutines/lambda-07-multi-capture.C: New test. --- gcc/cp/coroutines.cc | 117 +++++------------- .../torture/lambda-07-multi-capture.C | 55 ++++++++ 2 files changed, 85 insertions(+), 87 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/lambda-07-multi-capture.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 0c2ec32d7db..1bc2ed7f89c 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -1790,6 +1790,11 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d) cp_walk_tree (&DECL_SIZE (lvar), transform_local_var_uses, d, NULL); cp_walk_tree (&DECL_SIZE_UNIT (lvar), transform_local_var_uses, d, NULL); + if (DECL_HAS_VALUE_EXPR_P (lvar)) + { + tree t = DECL_VALUE_EXPR (lvar); + cp_walk_tree (&t, transform_local_var_uses, d, NULL); + } /* TODO: implement selective generation of fields when vars are known not-used. */ @@ -1815,9 +1820,9 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d) gcc_checking_assert (existed); if (local_var.field_id == NULL_TREE) - pvar = &DECL_CHAIN (*pvar); /* Wasn't used. */ - - *pvar = DECL_CHAIN (*pvar); /* discard this one, we replaced it. */ + pvar = &DECL_CHAIN (*pvar); /* Wasn't used or was an alias. */ + else + *pvar = DECL_CHAIN (*pvar); /* discard this one, we replaced it. */ } *do_subtree = 0; /* We've done the body already. */ @@ -1847,8 +1852,16 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d) if (local_var_i == NULL) return NULL_TREE; - /* This is our revised 'local' i.e. a frame slot. */ - tree revised = local_var_i->field_idx; + /* This is our revised 'local' i.e. a frame slot or an alias. */ + tree revised = NULL_TREE; + if (local_var_i->field_id == NULL_TREE) + { + gcc_checking_assert (DECL_HAS_VALUE_EXPR_P (var_decl)); + /* If the decl is an alias for another expression, substitute it. */ + revised = DECL_VALUE_EXPR (var_decl); + } + else + revised = local_var_i->field_idx; gcc_checking_assert (DECL_CONTEXT (var_decl) == lvd->context); if (decl_expr_p && DECL_INITIAL (var_decl)) @@ -2796,6 +2809,12 @@ register_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d) { param_frame_data *data = (param_frame_data *) d; + if (VAR_P (*stmt) && DECL_HAS_VALUE_EXPR_P (*stmt)) + { + tree x = DECL_VALUE_EXPR (*stmt); + cp_walk_tree (&x, register_param_uses, d, NULL); + } + if (TREE_CODE (*stmt) != PARM_DECL) return NULL_TREE; @@ -2840,7 +2859,6 @@ struct local_vars_frame_data { tree *field_list; hash_map *local_var_uses; - vec *captures; unsigned int nest_depth, bind_indx; location_t loc; bool saw_capture; @@ -2869,26 +2887,27 @@ register_local_var_uses (tree *stmt, int *do_subtree, void *d) local_var_info &local_var = lvd->local_var_uses->get_or_insert (lvar, &existed); gcc_checking_assert (!existed); + /* For var as an alias, just leave it. */ + if (DECL_HAS_VALUE_EXPR_P (lvar)) + continue; tree lvtype = TREE_TYPE (lvar); tree lvname = DECL_NAME (lvar); - bool captured = is_normal_capture_proxy (lvar); /* Make names depth+index unique, so that we can support nested scopes with identically named locals. */ char *buf; size_t namsize = sizeof ("__lv...") + 18; - const char *nm = (captured ? "cp" : "lv"); if (lvname != NULL_TREE) { namsize += IDENTIFIER_LENGTH (lvname); buf = (char *) alloca (namsize); - snprintf (buf, namsize, "__%s.%u.%u.%s", nm, lvd->bind_indx, + snprintf (buf, namsize, "__lv.%u.%u.%s", lvd->bind_indx, lvd->nest_depth, IDENTIFIER_POINTER (lvname)); } else { namsize += 10; /* 'D' followed by an unsigned. */ buf = (char *) alloca (namsize); - snprintf (buf, namsize, "__%s.%u.%u.D%u", nm, lvd->bind_indx, + snprintf (buf, namsize, "__lv.%u.%u.D%u", lvd->bind_indx, lvd->nest_depth, DECL_UID (lvar)); } /* TODO: Figure out if we should build a local type that has any @@ -2898,15 +2917,6 @@ register_local_var_uses (tree *stmt, int *do_subtree, void *d) local_var.def_loc = DECL_SOURCE_LOCATION (lvar); local_var.frame_type = lvtype; local_var.field_idx = NULL_TREE; - if (captured) - { - gcc_checking_assert (DECL_INITIAL (lvar) == NULL_TREE); - local_var.captured = lvar; - lvd->captures->safe_push (local_var); - lvd->saw_capture = true; - } - else - local_var.captured = NULL; lvd->local_var_seen = true; /* We don't walk any of the local var sub-trees, they won't contain any bind exprs. */ @@ -3177,10 +3187,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) /* 4. Now make space for local vars, this is conservative again, and we would expect to delete unused entries later. */ hash_map local_var_uses; - auto_vec captures; local_vars_frame_data local_vars_data - = {&field_list, &local_var_uses, &captures, 0, 0, fn_start, false, false}; + = {&field_list, &local_var_uses, 0, 0, fn_start, false, false}; cp_walk_tree (&fnbody, register_local_var_uses, &local_vars_data, NULL); /* Tie off the struct for now, so that we can build offsets to the @@ -3202,16 +3211,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) tree coro_fp = build_lang_decl (VAR_DECL, get_identifier ("coro.frameptr"), coro_frame_ptr); tree varlist = coro_fp; - local_var_info *cap; - if (!captures.is_empty()) - for (int ix = 0; captures.iterate (ix, &cap); ix++) - { - if (cap->field_id == NULL_TREE) - continue; - tree t = cap->captured; - DECL_CHAIN (t) = varlist; - varlist = t; - } /* Collected the scope vars we need ... only one for now. */ BIND_EXPR_VARS (ramp_bind) = nreverse (varlist); @@ -3477,62 +3476,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) } } - vec *captures_dtor_list = NULL; - while (!captures.is_empty()) - { - local_var_info cap = captures.pop(); - if (cap.field_id == NULL_TREE) - continue; - - tree fld_ref = lookup_member (coro_frame_type, cap.field_id, - /*protect=*/1, /*want_type=*/0, - tf_warning_or_error); - tree fld_idx - = build_class_member_access_expr (deref_fp, fld_ref, NULL_TREE, - false, tf_warning_or_error); - - tree cap_type = cap.frame_type; - - /* When we have a reference, we do not want to change the referenced - item, but actually to set the reference to the proxy var. */ - if (REFERENCE_REF_P (fld_idx)) - fld_idx = TREE_OPERAND (fld_idx, 0); - - if (TYPE_NEEDS_CONSTRUCTING (cap_type)) - { - vec *p_in; - if (TYPE_REF_P (cap_type) - && (CLASSTYPE_LAZY_MOVE_CTOR (cap_type) - || CLASSTYPE_LAZY_MOVE_ASSIGN (cap_type) - || classtype_has_move_assign_or_move_ctor_p - (cap_type, /*user_declared=*/true))) - p_in = make_tree_vector_single (rvalue (cap.captured)); - else - p_in = make_tree_vector_single (cap.captured); - /* Construct in place or move as relevant. */ - r = build_special_member_call (fld_idx, complete_ctor_identifier, - &p_in, cap_type, LOOKUP_NORMAL, - tf_warning_or_error); - release_tree_vector (p_in); - if (captures_dtor_list == NULL) - captures_dtor_list = make_tree_vector (); - vec_safe_push (captures_dtor_list, cap.field_id); - } - else - { - if (!same_type_p (cap_type, TREE_TYPE (cap.captured))) - r = build1_loc (DECL_SOURCE_LOCATION (cap.captured), CONVERT_EXPR, - cap_type, cap.captured); - else - r = cap.captured; - r = build_modify_expr (fn_start, fld_idx, cap_type, - INIT_EXPR, DECL_SOURCE_LOCATION (cap.captured), - r, TREE_TYPE (r)); - } - r = coro_build_cvt_void_expr_stmt (r, fn_start); - add_stmt (r); - } - /* Set up a new bind context for the GRO. */ tree gro_context_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL); /* Make and connect the scope blocks. */ diff --git a/gcc/testsuite/g++.dg/coroutines/torture/lambda-07-multi-capture.C b/gcc/testsuite/g++.dg/coroutines/torture/lambda-07-multi-capture.C new file mode 100644 index 00000000000..fb760472368 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/torture/lambda-07-multi-capture.C @@ -0,0 +1,55 @@ +// { dg-do run } + +// lambda with parm and local state + +#include "../coro.h" + +// boiler-plate for tests of codegen +#include "../coro1-ret-int-yield-int.h" + +int main () +{ + int a_copy = 20; + + auto f = [&a_ref = a_copy, a_copy = a_copy + 10]() -> coro1 + { + a_ref += 20; + co_return a_ref + a_copy; + }; + + { + coro1 A = f (); + A.handle.resume(); + if (a_copy != 40) + { + PRINT ("main: [a_copy = 40]"); + abort (); + } + + int y = A.handle.promise().get_value(); + if (y != 70) + { + PRINTF ("main: A co-ret = %d, should be 30\n", y); + abort (); + } + } + + a_copy = 5; + + coro1 B = f (); + B.handle.resume(); + if (a_copy != 25) + { + PRINT ("main: [a_copy = 25]"); + abort (); + } + + int y = B.handle.promise().get_value(); + if (y != 55) + { + PRINTF ("main: B co-ret = %d, should be 55\n", y); + abort (); + } + + return 0; +}