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 <ju...@linux.alibaba.com>
* 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 <ju...@linux.alibaba.com>
* 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<tree, local_var_info> *local_var_uses;
- vec<local_var_info> *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<tree, local_var_info> local_var_uses;
- auto_vec<local_var_info> 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<tree, va_gc> *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<tree, va_gc> *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;
+}
--
2.19.1.3.ge56e4f7
