On Thu, Feb 2, 2017 at 4:21 AM, Richard Biener <rguent...@suse.de> wrote: > On Wed, 1 Feb 2017, Richard Biener wrote: > >> On Wed, 1 Feb 2017, Jakub Jelinek wrote: >> >> > On Wed, Feb 01, 2017 at 02:14:20PM +0100, Richard Biener wrote: >> > > >> > > Looks like we cache the answer to maybe_constant_value (INTEGER_CST) >> > > which results in (-fmem-report): >> > > >> > > cp/constexpr.c:4814 (maybe_constant_value) 67108816:100.0% >> > > 100663104 17: 0.0% ggc >> > > >> > > this can be improved trivially to >> > > >> > > cp/constexpr.c:4817 (maybe_constant_value) 2032: 13.6% >> > > 2144 2: 0.0% ggc >> > > >> > > with the following patch which I am testing right now. >> > > >> > > Ok for trunk? >> > > >> > > (just in case it causes some fallout because, err, some tcc_constant >> > > is not really constant, what's the subset we can cheaply check here? >> > > basically we want to avoid caching all INTEGER_CSTs we use for >> > > CONSTRUCTOR_INDEX in large initializers) >> > >> > I'm worried that we don't want to handle all the constants that way. >> > As I wrote on IRC, I see some problematic constants: >> > 1) not sure if constants can't be >> > potential_nondependent_constant_expression, then we don't want to return >> > them >> > 2) cxx_eval_outermost_constant_expr has some special handling of >> > trees with vector type (and array type) >> > 3) constants with TREE_OVERFLOW should go through maybe_constant_value_1 >> > 4) INTEGER_CSTs with POINTER_TYPE (if they aren't 0) likewise >> > >> > For 3) and 4) I believe maybe_constant_value is supposed to wrap the >> > constants into a NOP_EXPR or something. >> >> Just to mention, bootstrap & regtest completed successfully without >> regressions on x86_64-unknown-linux-gnu so we at least have zero >> testing coverage for the cases that break. >> >> I'll wait for Jason to suggest specific things to avoid, TREE_OVERFLOW >> and pointer types are easy (no need to special case zero, it's just >> one entry per pointer type). > > Oh, and just to mention the same issue of course plagues > maybe_constant_init which ends up allocating a hash_map 1630776 times > (fixing that doesn't fix any memory-hog but would avoid some needless > cycles spent on this). Similar "simple" patch would be > > * constexpr.c (maybe_constant_init): Bail out early > for CONSTANT_CLASS_P. > > Index: gcc/cp/constexpr.c > =================================================================== > --- gcc/cp/constexpr.c (revision 245119) > +++ gcc/cp/constexpr.c (working copy) > @@ -4916,6 +4919,8 @@ maybe_constant_init (tree t, tree decl) > t = TARGET_EXPR_INITIAL (t); > if (!potential_nondependent_static_init_expression (t)) > /* Don't try to evaluate it. */; > + else if (CONSTANT_CLASS_P (t)) > + return t; > else > t = cxx_eval_outermost_constant_expr (t, true, false, decl); > if (TREE_CODE (t) == TARGET_EXPR) > > which is even eventually safer because it's after > the !potential_nondependent_static_init_expression (if that can > be ever true for CONSTANT_CLASS_P t). > > Then the other issue noticed is that we always copy every > CONSTRUCTOR at least once via reshape_init_array. > > I think both maybe_constant_value and maybe_constant_init are > low-hanging fruit to fix at this point so waiting for some > guidance on Jakubs concerns (or just take it yourself from here).
Yeah, I think doing the early return after potential_nondependent_* is the way to go. Here's what I'm applying:
commit 7b3d88c4524ae080518f8cbe4afbd57ffe4fdc3c Author: Jason Merrill <ja...@redhat.com> Date: Thu Feb 2 17:24:41 2017 -0500 PR c++/12245 - excessive memory use * constexpr.c (maybe_constant_value): Fold maybe_constant_value_1 back in. Don't cache constants. (maybe_constant_init): Don't cache constants. diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 67d2428..f9bc5186 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -4777,8 +4777,10 @@ fold_simple (tree t) Otherwise, if T does not have TREE_CONSTANT set, returns T. Otherwise, returns a version of T without TREE_CONSTANT. */ -static tree -maybe_constant_value_1 (tree t, tree decl) +static GTY((deletable)) hash_map<tree, tree> *cv_cache; + +tree +maybe_constant_value (tree t, tree decl) { tree r; @@ -4791,6 +4793,14 @@ maybe_constant_value_1 (tree t, tree decl) } return t; } + else if (CONSTANT_CLASS_P (t)) + /* No caching or evaluation needed. */ + return t; + + if (cv_cache == NULL) + cv_cache = hash_map<tree, tree>::create_ggc (101); + if (tree *cached = cv_cache->get (t)) + return *cached; r = cxx_eval_outermost_constant_expr (t, true, true, decl); gcc_checking_assert (r == t @@ -4798,29 +4808,10 @@ maybe_constant_value_1 (tree t, tree decl) || TREE_CODE (t) == VIEW_CONVERT_EXPR || (TREE_CONSTANT (t) && !TREE_CONSTANT (r)) || !cp_tree_equal (r, t)); + cv_cache->put (t, r); return r; } -static GTY((deletable)) hash_map<tree, tree> *cv_cache; - -/* If T is a constant expression, returns its reduced value. - Otherwise, if T does not have TREE_CONSTANT set, returns T. - Otherwise, returns a version of T without TREE_CONSTANT. */ - -tree -maybe_constant_value (tree t, tree decl) -{ - if (cv_cache == NULL) - cv_cache = hash_map<tree, tree>::create_ggc (101); - - if (tree *cached = cv_cache->get (t)) - return *cached; - - tree ret = maybe_constant_value_1 (t, decl); - cv_cache->put (t, ret); - return ret; -} - /* Dispose of the whole CV_CACHE. */ static void @@ -4916,6 +4907,8 @@ maybe_constant_init (tree t, tree decl) t = TARGET_EXPR_INITIAL (t); if (!potential_nondependent_static_init_expression (t)) /* Don't try to evaluate it. */; + else if (CONSTANT_CLASS_P (t)) + /* No evaluation needed. */; else t = cxx_eval_outermost_constant_expr (t, true, false, decl); if (TREE_CODE (t) == TARGET_EXPR)