From: Patrick Palka <patrick@parcs.ath.cx>
To: Patrick Palka <patrick@parcs.ath.cx>
Cc: gcc-patches@gcc.gnu.org, jason@redhat.com
Subject: Re: [PATCH] Remove class cache_map and use ggc hash_maps instead (PR c++/70452)
Date: Mon, 04 Apr 2016 17:28:00 -0000 [thread overview]
Message-ID: <alpine.DEB.2.20.11.1604041325040.2715@idea> (raw)
In-Reply-To: <1459789689-15305-1-git-send-email-patrick@parcs.ath.cx>
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
next prev parent reply other threads:[~2016-04-04 17:28 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-04 17:08 Patrick Palka
2016-04-04 17:28 ` Patrick Palka [this message]
2016-04-04 23:59 ` Jason Merrill
2016-04-04 23:59 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.20.11.1604041325040.2715@idea \
--to=patrick@parcs.ath.cx \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).