On Fri, Jun 7, 2019 at 5:08 PM Jason Merrill <ja...@redhat.com> wrote:
>
> All expression statements and some other places express implicit conversion to
> void with a CONVERT_EXPR. There's no reason to build up a new one as part of
> constexpr evaluation.
>
> The ADDR_EXPR change also avoids a bit of garbage by discarding an ADDR_EXPR
> we
> just built but didn't end up using.
>
> The location wrapper change doesn't affect garbage, it's just a minor
> optimization.
More constexpr garbage reduction:
1) Using a TREE_VEC instead of TREE_LIST to store bindings saves a
bunch of space when dealing with multiple arguments.
2) We don't need to unshare any expressions containing a CONSTRUCTOR,
or expressions within a CONSTRUCTOR; we're only interested in an
initializer which is itself a CONSTRUCTOR, as that's what can get
modified. A CONSTRUCTOR appearing as a subexpression won't be used to
directly initialize a variable, so we don't need to unshare it. We
also don't need to unshare the return value of the function, as it
will be unshared if it is used as an initializer.
3) When we free the TREE_VEC for a call we end up not using, we can
also free any CONSTRUCTORs we unshared for it. This unsharing isn't
necessary, but oddly it significantly reduces the max resident memory
on the testcase I've been looking at, so I'm going to leave it in. We
also free CONSTRUCTORs for initialized parameters when we're done
evaluating the function.
Tested x86_64-pc-linux-gnu, applied to trunk.
commit 20c4756f8e94a0278b3b74d1e674e3163b1e6a33
Author: Jason Merrill <ja...@redhat.com>
Date: Mon Jun 10 00:16:05 2019 -0400
Reduce constexpr_call memory consumption.
* constexpr.c (cxx_bind_parameters_in_call): Use TREE_VEC rather
than TREE_LIST.
(constexpr_call_hasher::equal, cxx_bind_parameters_in_call)
(cxx_eval_call_expression): Adjust.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 10afc419a33..74752bc72dd 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -974,12 +974,7 @@ explain_invalid_constexpr_fn (tree fun)
struct GTY((for_user)) constexpr_call {
/* Description of the constexpr function definition. */
constexpr_fundef *fundef;
- /* Parameter bindings environment. A TREE_LIST where each TREE_PURPOSE
- is a parameter _DECL and the TREE_VALUE is the value of the parameter.
- Note: This arrangement is made to accommodate the use of
- iterative_hash_template_arg (see pt.c). If you change this
- representation, also change the hash calculation in
- cxx_eval_call_expression. */
+ /* Parameter bindings environment. A TREE_VEC of arguments. */
tree bindings;
/* Result of the call.
NULL means the call is being evaluated.
@@ -1069,8 +1064,6 @@ constexpr_call_hasher::hash (constexpr_call *info)
bool
constexpr_call_hasher::equal (constexpr_call *lhs, constexpr_call *rhs)
{
- tree lhs_bindings;
- tree rhs_bindings;
if (lhs == rhs)
return true;
if (lhs->hash != rhs->hash)
@@ -1079,19 +1072,7 @@ constexpr_call_hasher::equal (constexpr_call *lhs,
constexpr_call *rhs)
return false;
if (!constexpr_fundef_hasher::equal (lhs->fundef, rhs->fundef))
return false;
- lhs_bindings = lhs->bindings;
- rhs_bindings = rhs->bindings;
- while (lhs_bindings != NULL && rhs_bindings != NULL)
- {
- tree lhs_arg = TREE_VALUE (lhs_bindings);
- tree rhs_arg = TREE_VALUE (rhs_bindings);
- gcc_assert (same_type_p (TREE_TYPE (lhs_arg), TREE_TYPE (rhs_arg)));
- if (!cp_tree_equal (lhs_arg, rhs_arg))
- return false;
- lhs_bindings = TREE_CHAIN (lhs_bindings);
- rhs_bindings = TREE_CHAIN (rhs_bindings);
- }
- return lhs_bindings == rhs_bindings;
+ return cp_tree_equal (lhs->bindings, rhs->bindings);
}
/* Initialize the constexpr call table, if needed. */
@@ -1380,7 +1361,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t,
tree fun = new_call->fundef->decl;
tree parms = new_call->fundef->parms;
int i;
- tree *p = &new_call->bindings;
+ /* We don't record ellipsis args below. */
+ int nparms = list_length (parms);
+ int nbinds = nargs < nparms ? nargs : nparms;
+ tree binds = new_call->bindings = make_tree_vec (nbinds);
for (i = 0; i < nargs; ++i)
{
tree x, arg;
@@ -1417,8 +1401,7 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t,
arg = adjust_temp_type (type, arg);
if (!TREE_CONSTANT (arg))
*non_constant_args = true;
- *p = build_tree_list (parms, arg);
- p = &TREE_CHAIN (*p);
+ TREE_VEC_ELT (binds, i) = arg;
}
parms = TREE_CHAIN (parms);
}
@@ -1745,14 +1728,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
void preserve () { do_free = false; }
~free_bindings () {
if (do_free)
- {
- while (bindings)
- {
- tree b = bindings;
- bindings = TREE_CHAIN (bindings);
- ggc_free (b);
- }
- }
+ ggc_free (bindings);
}
} fb (new_call.bindings);
@@ -1833,15 +1809,12 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
tree t,
/* Associate the bindings with the remapped parms. */
tree bound = new_call.bindings;
tree remapped = parms;
- while (bound)
+ for (int i = 0; i < TREE_VEC_LENGTH (bound); ++i)
{
- tree oparm = TREE_PURPOSE (bound);
- tree arg = TREE_VALUE (bound);
- gcc_assert (DECL_NAME (remapped) == DECL_NAME (oparm));
+ tree arg = TREE_VEC_ELT (bound, i);
/* Don't share a CONSTRUCTOR that might be changed. */
arg = unshare_constructor (arg);
ctx->values->put (remapped, arg);
- bound = TREE_CHAIN (bound);
remapped = DECL_CHAIN (remapped);
}
/* Add the RESULT_DECL to the values map, too. */
commit b624a048c602f46be2f15c84e55f31ce869d8e08
Author: Jason Merrill <ja...@redhat.com>
Date: Sat Jun 8 09:30:43 2019 -0400
Reduce unsharing in constexpr call evaluation.
* constexpr.c (unshare_constructor): Only unshare if T is itself a
CONSTRUCTOR.
(cxx_eval_call_expression): Don't call it on the result here.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 74752bc72dd..d7adb469b45 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1321,27 +1321,32 @@ adjust_temp_type (tree type, tree temp)
return cp_fold_convert (type, temp);
}
-/* Callback for walk_tree used by unshare_constructor. */
+/* If T is a CONSTRUCTOR, return an unshared copy of T and any
+ sub-CONSTRUCTORs. Otherwise return T.
-static tree
-find_constructor (tree *tp, int *walk_subtrees, void *)
-{
- if (TYPE_P (*tp))
- *walk_subtrees = 0;
- if (TREE_CODE (*tp) == CONSTRUCTOR)
- return *tp;
- return NULL_TREE;
-}
-
-/* If T is a CONSTRUCTOR or an expression that has a CONSTRUCTOR node as a
- subexpression, return an unshared copy of T. Otherwise return T. */
+ We use this whenever we initialize an object as a whole, whether it's a
+ parameter, a local variable, or a subobject, so that subsequent
+ modifications don't affect other places where it was used. */
tree
unshare_constructor (tree t)
{
- tree ctor = walk_tree (&t, find_constructor, NULL, NULL);
- if (ctor != NULL_TREE)
- return unshare_expr (t);
+ if (!t || TREE_CODE (t) != CONSTRUCTOR)
+ return t;
+ auto_vec <tree*, 4> ptrs;
+ ptrs.safe_push (&t);
+ while (!ptrs.is_empty ())
+ {
+ tree *p = ptrs.pop ();
+ tree n = copy_node (*p);
+ CONSTRUCTOR_ELTS (n) = vec_safe_copy (CONSTRUCTOR_ELTS (*p));
+ *p = n;
+ vec<constructor_elt, va_gc> *v = CONSTRUCTOR_ELTS (n);
+ constructor_elt *ce;
+ for (HOST_WIDE_INT i = 0; vec_safe_iterate (v, i, &ce); ++i)
+ if (TREE_CODE (ce->value) == CONSTRUCTOR)
+ ptrs.safe_push (&ce->value);
+ }
return t;
}
@@ -1898,7 +1903,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
clear_no_implicit_zero (result);
pop_cx_call_context ();
- return unshare_constructor (result);
+ return result;
}
/* FIXME speed this up, it's taking 16% of compile time on sieve testcase. */
commit 3faf66f3270c9f07ebd73376e66751105419322e
Author: Jason Merrill <ja...@redhat.com>
Date: Sat Jun 8 10:56:21 2019 -0400
* constexpr.c (free_constructor): New.
(cxx_eval_call_expression): Free parameter value CONSTRUCTORs.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index d7adb469b45..a2f29694462 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1350,6 +1350,30 @@ unshare_constructor (tree t)
return t;
}
+/* If T is a CONSTRUCTOR, ggc_free T and any sub-CONSTRUCTORs. */
+
+static void
+free_constructor (tree t)
+{
+ if (!t || TREE_CODE (t) != CONSTRUCTOR)
+ return;
+ releasing_vec ctors;
+ vec_safe_push (ctors, t);
+ while (!ctors->is_empty ())
+ {
+ tree c = ctors->pop ();
+ if (vec<constructor_elt, va_gc> *elts = CONSTRUCTOR_ELTS (c))
+ {
+ constructor_elt *ce;
+ for (HOST_WIDE_INT i = 0; vec_safe_iterate (elts, i, &ce); ++i)
+ if (TREE_CODE (ce->value) == CONSTRUCTOR)
+ vec_safe_push (ctors, ce->value);
+ ggc_free (elts);
+ }
+ ggc_free (c);
+ }
+}
+
/* Subroutine of cxx_eval_call_expression.
We are processing a call expression (either CALL_EXPR or
AGGR_INIT_EXPR) in the context of CTX. Evaluate
@@ -1398,7 +1422,8 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t,
if (!*non_constant_p)
{
- /* Don't share a CONSTRUCTOR that might be changed. */
+ /* Unsharing here isn't necessary for correctness, but it
+ significantly improves memory performance for some reason. */
arg = unshare_constructor (arg);
/* Make sure the binding has the same type as the parm. But
only for constant args. */
@@ -1733,7 +1758,11 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
void preserve () { do_free = false; }
~free_bindings () {
if (do_free)
- ggc_free (bindings);
+ {
+ for (int i = 0; i < TREE_VEC_LENGTH (bindings); ++i)
+ free_constructor (TREE_VEC_ELT (bindings, i));
+ ggc_free (bindings);
+ }
}
} fb (new_call.bindings);
@@ -1804,6 +1833,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
else
{
tree body, parms, res;
+ releasing_vec ctors;
/* Reuse or create a new unshared copy of this function's body. */
tree copy = get_fundef_copy (new_call.fundef);
@@ -1819,6 +1849,8 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
tree arg = TREE_VEC_ELT (bound, i);
/* Don't share a CONSTRUCTOR that might be changed. */
arg = unshare_constructor (arg);
+ if (TREE_CODE (arg) == CONSTRUCTOR)
+ vec_safe_push (ctors, arg);
ctx->values->put (remapped, arg);
remapped = DECL_CHAIN (remapped);
}
@@ -1884,6 +1916,14 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
ctx->values->remove (parm);
+ /* Free any parameter CONSTRUCTORs we aren't returning directly. */
+ while (!ctors->is_empty ())
+ {
+ tree c = ctors->pop ();
+ if (c != result)
+ free_constructor (c);
+ }
+
/* Make the unshared function copy we used available for re-use. */
save_fundef_copy (fun, copy);
}
