On Mon, 4 Apr 2016, Patrick Palka wrote: > On Mon, 4 Apr 2016, Jason Merrill wrote: > > > Hmm, I thought I remembered hitting the breakpoint in gt_cleare_cache and it > > being non-null. But I guess we can get rid of the cache_map class and use > > the > > approach you have here, of a deletable gc-allocated hash_map pointer; I'd > > still use ->empty() for dumping the cache outside of GC, though. > > Is this what you had in mind? > > gcc/cp/ChangeLog: > > PR c++/70452 > * cp-tree.h (class cache_map): Remove. > * constexpr.c (cv_cache): Change type to > GTY((deletable)) hash_map<tree, tree> *. > (maybe_constant_value): Adjust following the change to cv_cache.
Actually, using get_or_insert here is unsafe because maybe_constant_value could be called recursively. > (clear_cv_cache): New static function. > (clear_cv_and_fold_caches): Use it. > * cp-gimplify.c (fold_cache): Change type to > GTY((deletable)) hash_map<tree, tree> *. > (clear_fold_cache): Adjust following the change to fold_cache. > (cp_fold): Likewise. Same issue in cp_fold probably. So here's a patch that just uses get() and put(): -- >8 -- gcc/cp/ChangeLog: PR c++/70452 * cp-tree.h (class cache_map): Remove. * constexpr.c (cv_cache): Change type to GTY((deletable)) hash_map<tree, tree> *. (maybe_constant_value): Adjust following the change to cv_cache. (clear_cv_cache): New static function. (clear_cv_and_fold_caches): Use it. * cp-gimplify.c (fold_cache): Change type to GTY((deletable)) hash_map<tree, tree> *. (clear_fold_cache): Adjust following the change to fold_cache. (cp_fold): Likewise. --- gcc/cp/constexpr.c | 27 +++++++++++++++++++-------- gcc/cp/cp-gimplify.c | 16 ++++++++++------ gcc/cp/cp-tree.h | 36 ------------------------------------ 3 files changed, 29 insertions(+), 50 deletions(-) diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index bcbf9bd..1c2701b 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -4304,7 +4304,7 @@ maybe_constant_value_1 (tree t, tree decl) return r; } -static GTY((cache, deletable)) cache_map cv_cache; +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. @@ -4313,21 +4313,32 @@ static GTY((cache, deletable)) cache_map cv_cache; tree maybe_constant_value (tree t, tree decl) { - tree ret = cv_cache.get (t); - if (!ret) - { - ret = maybe_constant_value_1 (t, decl); - cv_cache.put (t, ret); - } + 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 +clear_cv_cache (void) +{ + if (cv_cache != NULL) + cv_cache->empty (); +} + /* Dispose of the whole CV_CACHE and FOLD_CACHE. */ void clear_cv_and_fold_caches (void) { - gt_cleare_cache (cv_cache); + clear_cv_cache (); clear_fold_cache (); } diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index b6e1d27..c96b666 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -1902,14 +1902,15 @@ c_fully_fold (tree x, bool /*in_init*/, bool */*maybe_const*/) return cp_fold_rvalue (x); } -static GTY((cache, deletable)) cache_map fold_cache; +static GTY((deletable)) hash_map<tree, tree> *fold_cache; /* Dispose of the whole FOLD_CACHE. */ void clear_fold_cache (void) { - gt_cleare_cache (fold_cache); + if (fold_cache != NULL) + fold_cache->empty (); } /* This function tries to fold an expression X. @@ -1939,8 +1940,11 @@ cp_fold (tree x) if (DECL_P (x) || CONSTANT_CLASS_P (x)) return x; - if (tree cached = fold_cache.get (x)) - return cached; + if (fold_cache == NULL) + fold_cache = hash_map<tree, tree>::create_ggc (101); + + if (tree *cached = fold_cache->get (x)) + return *cached; code = TREE_CODE (x); switch (code) @@ -2295,10 +2299,10 @@ cp_fold (tree x) return org_x; } - fold_cache.put (org_x, x); + fold_cache->put (org_x, x); /* Prevent that we try to fold an already folded result again. */ if (x != org_x) - fold_cache.put (x, x); + fold_cache->put (x, x); return x; } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index b1d2bfa..0f7e08f 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5527,42 +5527,6 @@ extern cp_parameter_declarator *no_parameters; /* True if we saw "#pragma GCC java_exceptions". */ extern bool pragma_java_exceptions; -/* Data structure for a mapping from tree to tree that's only used as a cache; - we don't GC-mark trees in the map, and we clear the map when collecting - garbage. Global variables of this type must be marked - GTY((cache,deletable)) so that the gt_cleare_cache function is called by - ggc_collect but we don't try to load the map pointer from a PCH. - - FIXME improve to use keep_cache_entry. */ -class cache_map -{ - /* Use a lazily initialized pointer rather than a map member since a - hash_map can't be constructed in a static initializer. */ - hash_map<tree, tree> *map; - -public: - tree get (tree key) - { - if (map) - if (tree *slot = map->get (key)) - return *slot; - return NULL_TREE; - } - - bool put (tree key, tree val) - { - if (!map) - map = new hash_map<tree, tree>; - return map->put (key, val); - } - - friend inline void gt_cleare_cache (cache_map &cm) - { - if (cm.map) - cm.map->empty(); - } -}; - /* in call.c */ extern bool check_dtor_name (tree, tree); int magic_varargs_p (tree); -- 2.8.0.rc3.27.gade0865