| Message ID | 20190930183035.7347-1-jason@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add some hash_map_safe_* functions like vec_safe_*. | expand |
My comments accidentally got lost. Several places in the front-end (and elsewhere) use the same lazy allocation pattern for hash_maps, and this patch replaces them with hash_map_safe_* functions like vec_safe_*. They don't provide a way to specify an initial size, but I don't think that's a significant issue. Tested x86_64-pc-linux-gnu. OK for trunk? On Mon, Sep 30, 2019 at 2:30 PM Jason Merrill <jason@redhat.com> wrote: > > gcc/ > * hash-map.h (default_size): Put in member variable. > (create_ggc): Use it as default argument. > (hash_map_maybe_create, hash_map_safe_get) > (hash_map_safe_get_or_insert, hash_map_safe_put): New fns. > gcc/cp/ > * constexpr.c (maybe_initialize_fundef_copies_table): Remove. > (get_fundef_copy): Use hash_map_safe_get_or_insert. > * cp-objcp-common.c (cp_get_debug_type): Use hash_map_safe_*. > * decl.c (store_decomp_type): Remove. > (cp_finish_decomp): Use hash_map_safe_put. > * init.c (get_nsdmi): Use hash_map_safe_*. > * pt.c (store_defaulted_ttp, lookup_defaulted_ttp): Remove. > (add_defaults_to_ttp): Use hash_map_safe_*. > --- > gcc/hash-map.h | 38 ++++++++++++++++++++++++++++++++++++-- > gcc/cp/constexpr.c | 14 ++------------ > gcc/cp/cp-objcp-common.c | 6 ++---- > gcc/cp/decl.c | 9 +-------- > gcc/cp/init.c | 9 ++------- > gcc/cp/pt.c | 21 +++------------------ > gcc/hash-table.c | 2 +- > 7 files changed, 47 insertions(+), 52 deletions(-) > > diff --git a/gcc/hash-map.h b/gcc/hash-map.h > index ba20fe79f23..e638f761465 100644 > --- a/gcc/hash-map.h > +++ b/gcc/hash-map.h > @@ -128,8 +128,9 @@ class GTY((user)) hash_map > } > }; > > + static const size_t default_size = 13; > public: > - explicit hash_map (size_t n = 13, bool ggc = false, > + explicit hash_map (size_t n = default_size, bool ggc = false, > bool sanitize_eq_and_hash = true, > bool gather_mem_stats = GATHER_STATISTICS > CXX_MEM_STAT_INFO) > @@ -146,7 +147,7 @@ public: > HASH_MAP_ORIGIN PASS_MEM_STAT) {} > > /* Create a hash_map in ggc memory. */ > - static hash_map *create_ggc (size_t size, > + static hash_map *create_ggc (size_t size = default_size, > bool gather_mem_stats = GATHER_STATISTICS > CXX_MEM_STAT_INFO) > { > @@ -326,4 +327,37 @@ gt_pch_nx (hash_map<K, V, H> *h, gt_pointer_operator op, void *cookie) > op (&h->m_table.m_entries, cookie); > } > > +template<typename K, typename V, typename H> > +inline hash_map<K,V,H> * > +hash_map_maybe_create (hash_map<K,V,H> *&h) > +{ > + if (!h) > + h = h->create_ggc (); > + return h; > +} > + > +/* Like h->get, but handles null h. */ > +template<typename K, typename V, typename H> > +inline V* > +hash_map_safe_get (hash_map<K,V,H> *h, const K& k) > +{ > + return h ? h->get (k) : NULL; > +} > + > +/* Like h->get, but handles null h. */ > +template<typename K, typename V, typename H> > +inline V& > +hash_map_safe_get_or_insert (hash_map<K,V,H> *&h, const K& k, bool *e = NULL) > +{ > + return hash_map_maybe_create (h)->get_or_insert (k, e); > +} > + > +/* Like h->put, but handles null h. */ > +template<typename K, typename V, typename H> > +inline bool > +hash_map_safe_put (hash_map<K,V,H> *&h, const K& k, const V& v) > +{ > + return hash_map_maybe_create (h)->put (k, v); > +} > + > #endif > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index cb5484f4b72..904b70a9c99 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -1098,15 +1098,6 @@ maybe_initialize_constexpr_call_table (void) > > static GTY(()) hash_map<tree, tree> *fundef_copies_table; > > -/* Initialize FUNDEF_COPIES_TABLE if it's not initialized. */ > - > -static void > -maybe_initialize_fundef_copies_table () > -{ > - if (fundef_copies_table == NULL) > - fundef_copies_table = hash_map<tree,tree>::create_ggc (101); > -} > - > /* Reuse a copy or create a new unshared copy of the function FUN. > Return this copy. We use a TREE_LIST whose PURPOSE is body, VALUE > is parms, TYPE is result. */ > @@ -1114,11 +1105,10 @@ maybe_initialize_fundef_copies_table () > static tree > get_fundef_copy (constexpr_fundef *fundef) > { > - maybe_initialize_fundef_copies_table (); > - > tree copy; > bool existed; > - tree *slot = &fundef_copies_table->get_or_insert (fundef->decl, &existed); > + tree *slot = &hash_map_safe_get_or_insert (fundef_copies_table, > + fundef->decl, &existed); > > if (!existed) > { > diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c > index 4369a5b5570..652b94230ad 100644 > --- a/gcc/cp/cp-objcp-common.c > +++ b/gcc/cp/cp-objcp-common.c > @@ -145,11 +145,9 @@ cp_get_debug_type (const_tree type) > if (dtype) > { > tree ktype = CONST_CAST_TREE (type); > - if (debug_type_map == NULL) > - debug_type_map = tree_cache_map::create_ggc (512); > - else if (tree *slot = debug_type_map->get (ktype)) > + if (tree *slot = hash_map_safe_get (debug_type_map, ktype)) > return *slot; > - debug_type_map->put (ktype, dtype); > + hash_map_safe_put (debug_type_map, ktype, dtype); > } > > return dtype; > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > index 67c4521e98c..562fe10bd96 100644 > --- a/gcc/cp/decl.c > +++ b/gcc/cp/decl.c > @@ -7704,13 +7704,6 @@ get_tuple_decomp_init (tree decl, unsigned i) > based on the actual type of the variable, so store it in a hash table. */ > > static GTY((cache)) tree_cache_map *decomp_type_table; > -static void > -store_decomp_type (tree v, tree t) > -{ > - if (!decomp_type_table) > - decomp_type_table = tree_cache_map::create_ggc (13); > - decomp_type_table->put (v, t); > -} > > tree > lookup_decomp_type (tree v) > @@ -7946,7 +7939,7 @@ cp_finish_decomp (tree decl, tree first, unsigned int count) > goto error_out; > } > /* Save the decltype away before reference collapse. */ > - store_decomp_type (v[i], eltype); > + hash_map_safe_put (decomp_type_table, v[i], eltype); > eltype = cp_build_reference_type (eltype, !lvalue_p (init)); > TREE_TYPE (v[i]) = eltype; > layout_decl (v[i], 0); > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > index 67e06568b2f..4bd60859bf8 100644 > --- a/gcc/cp/init.c > +++ b/gcc/cp/init.c > @@ -563,10 +563,9 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) > init = DECL_INITIAL (DECL_TI_TEMPLATE (member)); > location_t expr_loc > = cp_expr_loc_or_loc (init, DECL_SOURCE_LOCATION (member)); > - tree *slot; > if (TREE_CODE (init) == DEFERRED_PARSE) > /* Unparsed. */; > - else if (nsdmi_inst && (slot = nsdmi_inst->get (member))) > + else if (tree *slot = hash_map_safe_get (nsdmi_inst, member)) > init = *slot; > /* Check recursive instantiation. */ > else if (DECL_INSTANTIATING_NSDMI_P (member)) > @@ -611,11 +610,7 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) > DECL_INSTANTIATING_NSDMI_P (member) = 0; > > if (init != error_mark_node) > - { > - if (!nsdmi_inst) > - nsdmi_inst = tree_cache_map::create_ggc (37); > - nsdmi_inst->put (member, init); > - } > + hash_map_safe_put (nsdmi_inst, member, init); > > if (pushed) > { > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index 44b36183304..d2aeb1ccba6 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -7357,21 +7357,6 @@ coerce_template_args_for_ttp (tree templ, tree arglist, > /* A cache of template template parameters with match-all default > arguments. */ > static GTY((deletable)) hash_map<tree,tree> *defaulted_ttp_cache; > -static void > -store_defaulted_ttp (tree v, tree t) > -{ > - if (!defaulted_ttp_cache) > - defaulted_ttp_cache = hash_map<tree,tree>::create_ggc (13); > - defaulted_ttp_cache->put (v, t); > -} > -static tree > -lookup_defaulted_ttp (tree v) > -{ > - if (defaulted_ttp_cache) > - if (tree *p = defaulted_ttp_cache->get (v)) > - return *p; > - return NULL_TREE; > -} > > /* T is a bound template template-parameter. Copy its arguments into default > arguments of the template template-parameter's template parameters. */ > @@ -7379,8 +7364,8 @@ lookup_defaulted_ttp (tree v) > static tree > add_defaults_to_ttp (tree otmpl) > { > - if (tree c = lookup_defaulted_ttp (otmpl)) > - return c; > + if (tree *c = hash_map_safe_get (defaulted_ttp_cache, otmpl)) > + return *c; > > tree ntmpl = copy_node (otmpl); > > @@ -7410,7 +7395,7 @@ add_defaults_to_ttp (tree otmpl) > } > } > > - store_defaulted_ttp (otmpl, ntmpl); > + hash_map_safe_put (defaulted_ttp_cache, otmpl, ntmpl); > return ntmpl; > } > > diff --git a/gcc/hash-table.c b/gcc/hash-table.c > index e3b5d3da09e..3520c3bb596 100644 > --- a/gcc/hash-table.c > +++ b/gcc/hash-table.c > @@ -78,7 +78,7 @@ struct prime_ent const prime_tab[] = { > unsigned int hash_table_sanitize_eq_limit; > > /* The following function returns an index into the above table of the > - nearest prime number which is greater than N, and near a power of two. */ > + nearest prime number which is at least N, and near a power of two. */ > > unsigned int > hash_table_higher_prime_index (unsigned long n) > > base-commit: c6fd1c4cf03b10c7ab34dea84f70c618c824599e > -- > 2.21.0 >
On Mon, Sep 30, 2019 at 8:33 PM Jason Merrill <jason@redhat.com> wrote: > > My comments accidentally got lost. > > Several places in the front-end (and elsewhere) use the same lazy > allocation pattern for hash_maps, and this patch replaces them with > hash_map_safe_* functions like vec_safe_*. They don't provide a way > to specify an initial size, but I don't think that's a significant > issue. > > Tested x86_64-pc-linux-gnu. OK for trunk? You are using create_ggc but the new functions do not indicate that ggc allocation is done. It's then also incomplete with not having a non-ggc variant of them? Maybe I'm missing something. Thanks, Richard. > On Mon, Sep 30, 2019 at 2:30 PM Jason Merrill <jason@redhat.com> wrote: > > > > gcc/ > > * hash-map.h (default_size): Put in member variable. > > (create_ggc): Use it as default argument. > > (hash_map_maybe_create, hash_map_safe_get) > > (hash_map_safe_get_or_insert, hash_map_safe_put): New fns. > > gcc/cp/ > > * constexpr.c (maybe_initialize_fundef_copies_table): Remove. > > (get_fundef_copy): Use hash_map_safe_get_or_insert. > > * cp-objcp-common.c (cp_get_debug_type): Use hash_map_safe_*. > > * decl.c (store_decomp_type): Remove. > > (cp_finish_decomp): Use hash_map_safe_put. > > * init.c (get_nsdmi): Use hash_map_safe_*. > > * pt.c (store_defaulted_ttp, lookup_defaulted_ttp): Remove. > > (add_defaults_to_ttp): Use hash_map_safe_*. > > --- > > gcc/hash-map.h | 38 ++++++++++++++++++++++++++++++++++++-- > > gcc/cp/constexpr.c | 14 ++------------ > > gcc/cp/cp-objcp-common.c | 6 ++---- > > gcc/cp/decl.c | 9 +-------- > > gcc/cp/init.c | 9 ++------- > > gcc/cp/pt.c | 21 +++------------------ > > gcc/hash-table.c | 2 +- > > 7 files changed, 47 insertions(+), 52 deletions(-) > > > > diff --git a/gcc/hash-map.h b/gcc/hash-map.h > > index ba20fe79f23..e638f761465 100644 > > --- a/gcc/hash-map.h > > +++ b/gcc/hash-map.h > > @@ -128,8 +128,9 @@ class GTY((user)) hash_map > > } > > }; > > > > + static const size_t default_size = 13; > > public: > > - explicit hash_map (size_t n = 13, bool ggc = false, > > + explicit hash_map (size_t n = default_size, bool ggc = false, > > bool sanitize_eq_and_hash = true, > > bool gather_mem_stats = GATHER_STATISTICS > > CXX_MEM_STAT_INFO) > > @@ -146,7 +147,7 @@ public: > > HASH_MAP_ORIGIN PASS_MEM_STAT) {} > > > > /* Create a hash_map in ggc memory. */ > > - static hash_map *create_ggc (size_t size, > > + static hash_map *create_ggc (size_t size = default_size, > > bool gather_mem_stats = GATHER_STATISTICS > > CXX_MEM_STAT_INFO) > > { > > @@ -326,4 +327,37 @@ gt_pch_nx (hash_map<K, V, H> *h, gt_pointer_operator op, void *cookie) > > op (&h->m_table.m_entries, cookie); > > } > > > > +template<typename K, typename V, typename H> > > +inline hash_map<K,V,H> * > > +hash_map_maybe_create (hash_map<K,V,H> *&h) > > +{ > > + if (!h) > > + h = h->create_ggc (); > > + return h; > > +} > > + > > +/* Like h->get, but handles null h. */ > > +template<typename K, typename V, typename H> > > +inline V* > > +hash_map_safe_get (hash_map<K,V,H> *h, const K& k) > > +{ > > + return h ? h->get (k) : NULL; > > +} > > + > > +/* Like h->get, but handles null h. */ > > +template<typename K, typename V, typename H> > > +inline V& > > +hash_map_safe_get_or_insert (hash_map<K,V,H> *&h, const K& k, bool *e = NULL) > > +{ > > + return hash_map_maybe_create (h)->get_or_insert (k, e); > > +} > > + > > +/* Like h->put, but handles null h. */ > > +template<typename K, typename V, typename H> > > +inline bool > > +hash_map_safe_put (hash_map<K,V,H> *&h, const K& k, const V& v) > > +{ > > + return hash_map_maybe_create (h)->put (k, v); > > +} > > + > > #endif > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > index cb5484f4b72..904b70a9c99 100644 > > --- a/gcc/cp/constexpr.c > > +++ b/gcc/cp/constexpr.c > > @@ -1098,15 +1098,6 @@ maybe_initialize_constexpr_call_table (void) > > > > static GTY(()) hash_map<tree, tree> *fundef_copies_table; > > > > -/* Initialize FUNDEF_COPIES_TABLE if it's not initialized. */ > > - > > -static void > > -maybe_initialize_fundef_copies_table () > > -{ > > - if (fundef_copies_table == NULL) > > - fundef_copies_table = hash_map<tree,tree>::create_ggc (101); > > -} > > - > > /* Reuse a copy or create a new unshared copy of the function FUN. > > Return this copy. We use a TREE_LIST whose PURPOSE is body, VALUE > > is parms, TYPE is result. */ > > @@ -1114,11 +1105,10 @@ maybe_initialize_fundef_copies_table () > > static tree > > get_fundef_copy (constexpr_fundef *fundef) > > { > > - maybe_initialize_fundef_copies_table (); > > - > > tree copy; > > bool existed; > > - tree *slot = &fundef_copies_table->get_or_insert (fundef->decl, &existed); > > + tree *slot = &hash_map_safe_get_or_insert (fundef_copies_table, > > + fundef->decl, &existed); > > > > if (!existed) > > { > > diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c > > index 4369a5b5570..652b94230ad 100644 > > --- a/gcc/cp/cp-objcp-common.c > > +++ b/gcc/cp/cp-objcp-common.c > > @@ -145,11 +145,9 @@ cp_get_debug_type (const_tree type) > > if (dtype) > > { > > tree ktype = CONST_CAST_TREE (type); > > - if (debug_type_map == NULL) > > - debug_type_map = tree_cache_map::create_ggc (512); > > - else if (tree *slot = debug_type_map->get (ktype)) > > + if (tree *slot = hash_map_safe_get (debug_type_map, ktype)) > > return *slot; > > - debug_type_map->put (ktype, dtype); > > + hash_map_safe_put (debug_type_map, ktype, dtype); > > } > > > > return dtype; > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > > index 67c4521e98c..562fe10bd96 100644 > > --- a/gcc/cp/decl.c > > +++ b/gcc/cp/decl.c > > @@ -7704,13 +7704,6 @@ get_tuple_decomp_init (tree decl, unsigned i) > > based on the actual type of the variable, so store it in a hash table. */ > > > > static GTY((cache)) tree_cache_map *decomp_type_table; > > -static void > > -store_decomp_type (tree v, tree t) > > -{ > > - if (!decomp_type_table) > > - decomp_type_table = tree_cache_map::create_ggc (13); > > - decomp_type_table->put (v, t); > > -} > > > > tree > > lookup_decomp_type (tree v) > > @@ -7946,7 +7939,7 @@ cp_finish_decomp (tree decl, tree first, unsigned int count) > > goto error_out; > > } > > /* Save the decltype away before reference collapse. */ > > - store_decomp_type (v[i], eltype); > > + hash_map_safe_put (decomp_type_table, v[i], eltype); > > eltype = cp_build_reference_type (eltype, !lvalue_p (init)); > > TREE_TYPE (v[i]) = eltype; > > layout_decl (v[i], 0); > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > > index 67e06568b2f..4bd60859bf8 100644 > > --- a/gcc/cp/init.c > > +++ b/gcc/cp/init.c > > @@ -563,10 +563,9 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) > > init = DECL_INITIAL (DECL_TI_TEMPLATE (member)); > > location_t expr_loc > > = cp_expr_loc_or_loc (init, DECL_SOURCE_LOCATION (member)); > > - tree *slot; > > if (TREE_CODE (init) == DEFERRED_PARSE) > > /* Unparsed. */; > > - else if (nsdmi_inst && (slot = nsdmi_inst->get (member))) > > + else if (tree *slot = hash_map_safe_get (nsdmi_inst, member)) > > init = *slot; > > /* Check recursive instantiation. */ > > else if (DECL_INSTANTIATING_NSDMI_P (member)) > > @@ -611,11 +610,7 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) > > DECL_INSTANTIATING_NSDMI_P (member) = 0; > > > > if (init != error_mark_node) > > - { > > - if (!nsdmi_inst) > > - nsdmi_inst = tree_cache_map::create_ggc (37); > > - nsdmi_inst->put (member, init); > > - } > > + hash_map_safe_put (nsdmi_inst, member, init); > > > > if (pushed) > > { > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > index 44b36183304..d2aeb1ccba6 100644 > > --- a/gcc/cp/pt.c > > +++ b/gcc/cp/pt.c > > @@ -7357,21 +7357,6 @@ coerce_template_args_for_ttp (tree templ, tree arglist, > > /* A cache of template template parameters with match-all default > > arguments. */ > > static GTY((deletable)) hash_map<tree,tree> *defaulted_ttp_cache; > > -static void > > -store_defaulted_ttp (tree v, tree t) > > -{ > > - if (!defaulted_ttp_cache) > > - defaulted_ttp_cache = hash_map<tree,tree>::create_ggc (13); > > - defaulted_ttp_cache->put (v, t); > > -} > > -static tree > > -lookup_defaulted_ttp (tree v) > > -{ > > - if (defaulted_ttp_cache) > > - if (tree *p = defaulted_ttp_cache->get (v)) > > - return *p; > > - return NULL_TREE; > > -} > > > > /* T is a bound template template-parameter. Copy its arguments into default > > arguments of the template template-parameter's template parameters. */ > > @@ -7379,8 +7364,8 @@ lookup_defaulted_ttp (tree v) > > static tree > > add_defaults_to_ttp (tree otmpl) > > { > > - if (tree c = lookup_defaulted_ttp (otmpl)) > > - return c; > > + if (tree *c = hash_map_safe_get (defaulted_ttp_cache, otmpl)) > > + return *c; > > > > tree ntmpl = copy_node (otmpl); > > > > @@ -7410,7 +7395,7 @@ add_defaults_to_ttp (tree otmpl) > > } > > } > > > > - store_defaulted_ttp (otmpl, ntmpl); > > + hash_map_safe_put (defaulted_ttp_cache, otmpl, ntmpl); > > return ntmpl; > > } > > > > diff --git a/gcc/hash-table.c b/gcc/hash-table.c > > index e3b5d3da09e..3520c3bb596 100644 > > --- a/gcc/hash-table.c > > +++ b/gcc/hash-table.c > > @@ -78,7 +78,7 @@ struct prime_ent const prime_tab[] = { > > unsigned int hash_table_sanitize_eq_limit; > > > > /* The following function returns an index into the above table of the > > - nearest prime number which is greater than N, and near a power of two. */ > > + nearest prime number which is at least N, and near a power of two. */ > > > > unsigned int > > hash_table_higher_prime_index (unsigned long n) > > > > base-commit: c6fd1c4cf03b10c7ab34dea84f70c618c824599e > > -- > > 2.21.0 > > >
On 10/1/19 3:34 AM, Richard Biener wrote: > On Mon, Sep 30, 2019 at 8:33 PM Jason Merrill <jason@redhat.com> wrote: >> >> My comments accidentally got lost. >> >> Several places in the front-end (and elsewhere) use the same lazy >> allocation pattern for hash_maps, and this patch replaces them with >> hash_map_safe_* functions like vec_safe_*. They don't provide a way >> to specify an initial size, but I don't think that's a significant >> issue. >> >> Tested x86_64-pc-linux-gnu. OK for trunk? > > You are using create_ggc but the new functions do not indicate that ggc > allocation is done. > It's then also incomplete with not having a non-ggc variant > of them? Maybe I'm missing something. Ah, I had been thinking that this lazy pattern would only be used with ggc, but I see that I was wrong. How's this? Incidentally, now I see another C++11 feature I'd like to be able to use: default template arguments for function templates.
On Tue, Oct 1, 2019 at 8:50 PM Jason Merrill <jason@redhat.com> wrote: > > On 10/1/19 3:34 AM, Richard Biener wrote: > > On Mon, Sep 30, 2019 at 8:33 PM Jason Merrill <jason@redhat.com> wrote: > >> > >> My comments accidentally got lost. > >> > >> Several places in the front-end (and elsewhere) use the same lazy > >> allocation pattern for hash_maps, and this patch replaces them with > >> hash_map_safe_* functions like vec_safe_*. They don't provide a way > >> to specify an initial size, but I don't think that's a significant > >> issue. > >> > >> Tested x86_64-pc-linux-gnu. OK for trunk? > > > > You are using create_ggc but the new functions do not indicate that ggc > > allocation is done. > > It's then also incomplete with not having a non-ggc variant > > of them? Maybe I'm missing something. > > Ah, I had been thinking that this lazy pattern would only be used with > ggc, but I see that I was wrong. How's this? > > Incidentally, now I see another C++11 feature I'd like to be able to > use: default template arguments for function templates. I presume template<bool ggc, typename K, typename V, typename H> inline bool hash_map_safe_put (hash_map<K,V,H> *&h, const K& k, const V& v, size_t size = default_hash_map_size) { return hash_map_maybe_create<ggc> (h, size)->put (k, v); } was deemed to ugly? IMHO instantiating the templates for different sizes is unwanted compile-time overhead (plus not being able to use a default value makes non-default values creep into the code-base?). I'd have OKed a variant like above, so - would that work for you (change hash_map_maybe_create as well then, of course). Thanks, Richard.
On 10/2/19 4:28 AM, Richard Biener wrote: > On Tue, Oct 1, 2019 at 8:50 PM Jason Merrill <jason@redhat.com> wrote: >> >> On 10/1/19 3:34 AM, Richard Biener wrote: >>> On Mon, Sep 30, 2019 at 8:33 PM Jason Merrill <jason@redhat.com> wrote: >>>> >>>> My comments accidentally got lost. >>>> >>>> Several places in the front-end (and elsewhere) use the same lazy >>>> allocation pattern for hash_maps, and this patch replaces them with >>>> hash_map_safe_* functions like vec_safe_*. They don't provide a way >>>> to specify an initial size, but I don't think that's a significant >>>> issue. >>>> >>>> Tested x86_64-pc-linux-gnu. OK for trunk? >>> >>> You are using create_ggc but the new functions do not indicate that ggc >>> allocation is done. >>> It's then also incomplete with not having a non-ggc variant >>> of them? Maybe I'm missing something. >> >> Ah, I had been thinking that this lazy pattern would only be used with >> ggc, but I see that I was wrong. How's this? >> >> Incidentally, now I see another C++11 feature I'd like to be able to >> use: default template arguments for function templates. > > I presume > > template<bool ggc, typename K, typename V, typename H> > inline bool > hash_map_safe_put (hash_map<K,V,H> *&h, const K& k, const V& v, size_t > size = default_hash_map_size) > { > return hash_map_maybe_create<ggc> (h, size)->put (k, v); > } > > was deemed to ugly? IMHO instantiating the templates for different sizes > is unwanted compile-time overhead (plus not being able to use > a default value makes non-default values creep into the code-base?). > > I'd have OKed a variant like above, so - would that work for you > (change hash_map_maybe_create as well then, of course). Sure. Applying this version:
diff --git a/gcc/hash-map.h b/gcc/hash-map.h index ba20fe79f23..e638f761465 100644 --- a/gcc/hash-map.h +++ b/gcc/hash-map.h @@ -128,8 +128,9 @@ class GTY((user)) hash_map } }; + static const size_t default_size = 13; public: - explicit hash_map (size_t n = 13, bool ggc = false, + explicit hash_map (size_t n = default_size, bool ggc = false, bool sanitize_eq_and_hash = true, bool gather_mem_stats = GATHER_STATISTICS CXX_MEM_STAT_INFO) @@ -146,7 +147,7 @@ public: HASH_MAP_ORIGIN PASS_MEM_STAT) {} /* Create a hash_map in ggc memory. */ - static hash_map *create_ggc (size_t size, + static hash_map *create_ggc (size_t size = default_size, bool gather_mem_stats = GATHER_STATISTICS CXX_MEM_STAT_INFO) { @@ -326,4 +327,37 @@ gt_pch_nx (hash_map<K, V, H> *h, gt_pointer_operator op, void *cookie) op (&h->m_table.m_entries, cookie); } +template<typename K, typename V, typename H> +inline hash_map<K,V,H> * +hash_map_maybe_create (hash_map<K,V,H> *&h) +{ + if (!h) + h = h->create_ggc (); + return h; +} + +/* Like h->get, but handles null h. */ +template<typename K, typename V, typename H> +inline V* +hash_map_safe_get (hash_map<K,V,H> *h, const K& k) +{ + return h ? h->get (k) : NULL; +} + +/* Like h->get, but handles null h. */ +template<typename K, typename V, typename H> +inline V& +hash_map_safe_get_or_insert (hash_map<K,V,H> *&h, const K& k, bool *e = NULL) +{ + return hash_map_maybe_create (h)->get_or_insert (k, e); +} + +/* Like h->put, but handles null h. */ +template<typename K, typename V, typename H> +inline bool +hash_map_safe_put (hash_map<K,V,H> *&h, const K& k, const V& v) +{ + return hash_map_maybe_create (h)->put (k, v); +} + #endif diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index cb5484f4b72..904b70a9c99 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1098,15 +1098,6 @@ maybe_initialize_constexpr_call_table (void) static GTY(()) hash_map<tree, tree> *fundef_copies_table; -/* Initialize FUNDEF_COPIES_TABLE if it's not initialized. */ - -static void -maybe_initialize_fundef_copies_table () -{ - if (fundef_copies_table == NULL) - fundef_copies_table = hash_map<tree,tree>::create_ggc (101); -} - /* Reuse a copy or create a new unshared copy of the function FUN. Return this copy. We use a TREE_LIST whose PURPOSE is body, VALUE is parms, TYPE is result. */ @@ -1114,11 +1105,10 @@ maybe_initialize_fundef_copies_table () static tree get_fundef_copy (constexpr_fundef *fundef) { - maybe_initialize_fundef_copies_table (); - tree copy; bool existed; - tree *slot = &fundef_copies_table->get_or_insert (fundef->decl, &existed); + tree *slot = &hash_map_safe_get_or_insert (fundef_copies_table, + fundef->decl, &existed); if (!existed) { diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c index 4369a5b5570..652b94230ad 100644 --- a/gcc/cp/cp-objcp-common.c +++ b/gcc/cp/cp-objcp-common.c @@ -145,11 +145,9 @@ cp_get_debug_type (const_tree type) if (dtype) { tree ktype = CONST_CAST_TREE (type); - if (debug_type_map == NULL) - debug_type_map = tree_cache_map::create_ggc (512); - else if (tree *slot = debug_type_map->get (ktype)) + if (tree *slot = hash_map_safe_get (debug_type_map, ktype)) return *slot; - debug_type_map->put (ktype, dtype); + hash_map_safe_put (debug_type_map, ktype, dtype); } return dtype; diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 67c4521e98c..562fe10bd96 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -7704,13 +7704,6 @@ get_tuple_decomp_init (tree decl, unsigned i) based on the actual type of the variable, so store it in a hash table. */ static GTY((cache)) tree_cache_map *decomp_type_table; -static void -store_decomp_type (tree v, tree t) -{ - if (!decomp_type_table) - decomp_type_table = tree_cache_map::create_ggc (13); - decomp_type_table->put (v, t); -} tree lookup_decomp_type (tree v) @@ -7946,7 +7939,7 @@ cp_finish_decomp (tree decl, tree first, unsigned int count) goto error_out; } /* Save the decltype away before reference collapse. */ - store_decomp_type (v[i], eltype); + hash_map_safe_put (decomp_type_table, v[i], eltype); eltype = cp_build_reference_type (eltype, !lvalue_p (init)); TREE_TYPE (v[i]) = eltype; layout_decl (v[i], 0); diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 67e06568b2f..4bd60859bf8 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -563,10 +563,9 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) init = DECL_INITIAL (DECL_TI_TEMPLATE (member)); location_t expr_loc = cp_expr_loc_or_loc (init, DECL_SOURCE_LOCATION (member)); - tree *slot; if (TREE_CODE (init) == DEFERRED_PARSE) /* Unparsed. */; - else if (nsdmi_inst && (slot = nsdmi_inst->get (member))) + else if (tree *slot = hash_map_safe_get (nsdmi_inst, member)) init = *slot; /* Check recursive instantiation. */ else if (DECL_INSTANTIATING_NSDMI_P (member)) @@ -611,11 +610,7 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) DECL_INSTANTIATING_NSDMI_P (member) = 0; if (init != error_mark_node) - { - if (!nsdmi_inst) - nsdmi_inst = tree_cache_map::create_ggc (37); - nsdmi_inst->put (member, init); - } + hash_map_safe_put (nsdmi_inst, member, init); if (pushed) { diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 44b36183304..d2aeb1ccba6 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -7357,21 +7357,6 @@ coerce_template_args_for_ttp (tree templ, tree arglist, /* A cache of template template parameters with match-all default arguments. */ static GTY((deletable)) hash_map<tree,tree> *defaulted_ttp_cache; -static void -store_defaulted_ttp (tree v, tree t) -{ - if (!defaulted_ttp_cache) - defaulted_ttp_cache = hash_map<tree,tree>::create_ggc (13); - defaulted_ttp_cache->put (v, t); -} -static tree -lookup_defaulted_ttp (tree v) -{ - if (defaulted_ttp_cache) - if (tree *p = defaulted_ttp_cache->get (v)) - return *p; - return NULL_TREE; -} /* T is a bound template template-parameter. Copy its arguments into default arguments of the template template-parameter's template parameters. */ @@ -7379,8 +7364,8 @@ lookup_defaulted_ttp (tree v) static tree add_defaults_to_ttp (tree otmpl) { - if (tree c = lookup_defaulted_ttp (otmpl)) - return c; + if (tree *c = hash_map_safe_get (defaulted_ttp_cache, otmpl)) + return *c; tree ntmpl = copy_node (otmpl); @@ -7410,7 +7395,7 @@ add_defaults_to_ttp (tree otmpl) } } - store_defaulted_ttp (otmpl, ntmpl); + hash_map_safe_put (defaulted_ttp_cache, otmpl, ntmpl); return ntmpl; } diff --git a/gcc/hash-table.c b/gcc/hash-table.c index e3b5d3da09e..3520c3bb596 100644 --- a/gcc/hash-table.c +++ b/gcc/hash-table.c @@ -78,7 +78,7 @@ struct prime_ent const prime_tab[] = { unsigned int hash_table_sanitize_eq_limit; /* The following function returns an index into the above table of the - nearest prime number which is greater than N, and near a power of two. */ + nearest prime number which is at least N, and near a power of two. */ unsigned int hash_table_higher_prime_index (unsigned long n)