* [PATCH] Remove class cache_map and use ggc hash_maps instead (PR c++/70452)
@ 2016-04-04 17:08 Patrick Palka
2016-04-04 17:28 ` Patrick Palka
2016-04-04 23:59 ` Jason Merrill
0 siblings, 2 replies; 4+ messages in thread
From: Patrick Palka @ 2016-04-04 17:08 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Patrick Palka
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.
(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 | 17 +++++++++++------
gcc/cp/cp-tree.h | 36 ------------------------------------
3 files changed, 29 insertions(+), 51 deletions(-)
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index bcbf9bd..8f080ee 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,13 +4313,22 @@ 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);
- }
- return ret;
+ if (cv_cache == NULL)
+ cv_cache = hash_map<tree, tree>::create_ggc (101);
+
+ tree *slot = &cv_cache->get_or_insert (t, NULL);
+ if (*slot == NULL_TREE)
+ *slot = maybe_constant_value_1 (t, decl);
+ return *slot;
+}
+
+/* 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. */
@@ -4327,7 +4336,7 @@ maybe_constant_value (tree t, tree decl)
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..61ac187 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,12 @@ 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);
+
+ tree *slot = &fold_cache->get_or_insert (x, NULL);
+ if (*slot != NULL_TREE)
+ return *slot;
code = TREE_CODE (x);
switch (code)
@@ -2295,10 +2300,10 @@ cp_fold (tree x)
return org_x;
}
- fold_cache.put (org_x, x);
+ *slot = 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove class cache_map and use ggc hash_maps instead (PR c++/70452)
2016-04-04 17:08 [PATCH] Remove class cache_map and use ggc hash_maps instead (PR c++/70452) Patrick Palka
@ 2016-04-04 17:28 ` Patrick Palka
2016-04-04 23:59 ` Jason Merrill
2016-04-04 23:59 ` Jason Merrill
1 sibling, 1 reply; 4+ messages in thread
From: Patrick Palka @ 2016-04-04 17:28 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, jason
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove class cache_map and use ggc hash_maps instead (PR c++/70452)
2016-04-04 17:08 [PATCH] Remove class cache_map and use ggc hash_maps instead (PR c++/70452) Patrick Palka
2016-04-04 17:28 ` Patrick Palka
@ 2016-04-04 23:59 ` Jason Merrill
1 sibling, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2016-04-04 23:59 UTC (permalink / raw)
To: Patrick Palka, gcc-patches
On 04/04/2016 01:08 PM, Patrick Palka wrote:
> + tree *slot = &cv_cache->get_or_insert (t, NULL);
> + if (*slot == NULL_TREE)
> + *slot = maybe_constant_value_1 (t, decl);
...
> - fold_cache.put (org_x, x);
> + *slot = x;
This pattern isn't safe; the slot might move due to hash table resizing
between the get_or_insert and the store to *slot. Just use get/put.
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-04 23:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04 17:08 [PATCH] Remove class cache_map and use ggc hash_maps instead (PR c++/70452) Patrick Palka
2016-04-04 17:28 ` Patrick Palka
2016-04-04 23:59 ` Jason Merrill
2016-04-04 23:59 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).