| Message ID | 20191122140848.GA2466@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Fix concepts vs. PCH (PR c++/92458, take 2) | expand |
On 11/22/19 2:08 PM, Jakub Jelinek wrote: > On Wed, Nov 20, 2019 at 07:46:13PM -0500, Jason Merrill wrote: >>> If decl_tree_cache_map will be needed in more than one spot, I'll probably >>> need to move it to some generic header. >> >> Most of them probably need it, for code that uses the relevant features. >> Except debug_type_map, which probably needs to use TYPE_UID. >> >> Or we might make default_hash_traits<tree> use DECL_UID for decls and >> TYPE_UID for types even if it doesn't do the more complex analysis of >> tree_operand_hash. > > Finding out what is a type and what is decl (and what to do with rest?) > at runtime would be more costly than necessary. > > The following updated patch moves the decl_tree_cache_map to tree.h and > adds there type_tree_cache_map too and uses it in all the tree_cache_map > spots in the C++ FE. > tree-hash-traits.h can't be included from tree.h, because operand_equal_p is > declared elsewhere, so instead this patch moves some of the > tree-hash-traits.h traits from that header to tree.h. I could move there > just tree_decl_hash if moving the other two doesn't look right to you. > > Bootstrapped/regtested on x86_64-linux and i686-linux, additionally tested > by compiling stdc++.h as PCH with -std=c++2a and using it. Ok for trunk? > 2019-11-22 Jakub Jelinek <jakub@redhat.com> > > PR c++/92458 > * tree-hash-traits.h (tree_decl_hash, tree_ssa_name_hash, > tree_hash): Move to ... > * tree.h (tree_decl_hash, tree_ssa_name_hash, tree_hash): ... here. > (struct decl_tree_cache_traits, struct type_tree_cache_traits): New > types. > (decl_tree_cache_map, tree_tree_cache_map): New typedefs. > > * init.c (nsdmi_inst): Change type to > decl_tree_cache_map * from tree_cache_map *. > * constraint.cc (decl_constraints): Likewise. > * decl.c (get_tuple_decomp_init): Likewise. > * pt.c (defarg_inst, explicit_specifier_map): Likewise. > (tsubst_default_argument, store_explicit_specifier): Use > decl_tree_cache_map::create_ggc rather than > tree_cache_map::create_ggc. > * cp-objcp-common.c (debug_type_map): Change type to > type_tree_cache_map * from tree_cache_map *. > > * g++.dg/pch/pr92458.C: New test. > * g++.dg/pch/pr92458.Hs: New test. > > --- gcc/tree-hash-traits.h.jj 2019-01-01 12:37:16.902979134 +0100 > +++ gcc/tree-hash-traits.h 2019-11-22 12:00:01.538725844 +0100 > @@ -41,44 +41,4 @@ tree_operand_hash::equal (const value_ty > return operand_equal_p (t1, t2, 0); > } > > -/* Hasher for tree decls. Pointer equality is enough here, but the DECL_UID > - is a better hash than the pointer value and gives a predictable traversal > - order. */ > -struct tree_decl_hash : ggc_ptr_hash <tree_node> > -{ > - static inline hashval_t hash (tree); > -}; > - > -inline hashval_t > -tree_decl_hash::hash (tree t) > -{ > - return DECL_UID (t); > -} > - > -/* Hash for SSA_NAMEs in the same function. Pointer equality is enough > - here, but the SSA_NAME_VERSION is a better hash than the pointer > - value and gives a predictable traversal order. */ > -struct tree_ssa_name_hash : ggc_ptr_hash <tree_node> > -{ > - static inline hashval_t hash (tree); > -}; > - > -inline hashval_t > -tree_ssa_name_hash::hash (tree t) > -{ > - return SSA_NAME_VERSION (t); > -} > - > -/* Hasher for general trees, based on their TREE_HASH. */ > -struct tree_hash : ggc_ptr_hash <tree_node> > -{ > - static hashval_t hash (tree); > -}; > - > -inline hashval_t > -tree_hash::hash (tree t) > -{ > - return TREE_HASH (t); > -} > - > #endif > --- gcc/tree.h.jj 2019-11-15 00:37:26.293070143 +0100 > +++ gcc/tree.h 2019-11-22 12:00:17.479478830 +0100 > @@ -5351,6 +5351,58 @@ struct tree_decl_map_cache_hasher : ggc_ > #define tree_vec_map_hash tree_decl_map_hash > #define tree_vec_map_marked_p tree_map_base_marked_p > > +/* Hasher for tree decls. Pointer equality is enough here, but the DECL_UID > + is a better hash than the pointer value and gives a predictable traversal > + order. Additionally it can be used across PCH save/restore. */ > +struct tree_decl_hash : ggc_ptr_hash <tree_node> > +{ > + static inline hashval_t hash (tree); > +}; > + > +inline hashval_t > +tree_decl_hash::hash (tree t) > +{ > + return DECL_UID (t); > +} > + > +/* Similarly for types. Uses TYPE_UID as hash function. */ > +struct tree_type_hash : ggc_ptr_hash <tree_node> > +{ > + static inline hashval_t hash (tree); > +}; > + > +inline hashval_t > +tree_type_hash::hash (tree t) > +{ > + return TYPE_UID (t); > +} > + > +/* Hash for SSA_NAMEs in the same function. Pointer equality is enough > + here, but the SSA_NAME_VERSION is a better hash than the pointer > + value and gives a predictable traversal order. */ > +struct tree_ssa_name_hash : ggc_ptr_hash <tree_node> > +{ > + static inline hashval_t hash (tree); > +}; > + > +inline hashval_t > +tree_ssa_name_hash::hash (tree t) > +{ > + return SSA_NAME_VERSION (t); > +} > + > +/* Hasher for general trees, based on their TREE_HASH. */ > +struct tree_hash : ggc_ptr_hash <tree_node> > +{ > + static hashval_t hash (tree); > +}; > + > +inline hashval_t > +tree_hash::hash (tree t) > +{ > + return TREE_HASH (t); > +} > + > /* A hash_map of two trees for use with GTY((cache)). Garbage collection for > such a map will not mark keys, and will mark values if the key is already > marked. */ > @@ -5358,6 +5410,18 @@ struct tree_cache_traits > : simple_cache_map_traits<default_hash_traits<tree>, tree> { }; We should probably use tree_hash instead of default_hash_traits here. OK with or without that change. > typedef hash_map<tree,tree,tree_cache_traits> tree_cache_map; > > +/* Similarly, but use DECL_UID as hash function rather than pointer hashing. > + This is for hash_maps from decls to trees that need to work across PCH. */ > +struct decl_tree_cache_traits > + : simple_cache_map_traits<tree_decl_hash, tree> { }; > +typedef hash_map<tree,tree,decl_tree_cache_traits> decl_tree_cache_map; > + > +/* Similarly, but use TYPE_UID as hash function rather than pointer hashing. > + This is for hash_maps from types to trees that need to work across PCH. */ > +struct type_tree_cache_traits > + : simple_cache_map_traits<tree_type_hash, tree> { }; > +typedef hash_map<tree,tree,type_tree_cache_traits> type_tree_cache_map; > + > /* Initialize the abstract argument list iterator object ITER with the > arguments from CALL_EXPR node EXP. */ > static inline void > --- gcc/cp/init.c.jj 2019-11-08 00:22:12.042424097 +0100 > +++ gcc/cp/init.c 2019-11-22 11:50:11.285872353 +0100 > @@ -548,7 +548,7 @@ perform_target_ctor (tree init) > > /* Return the non-static data initializer for FIELD_DECL MEMBER. */ > > -static GTY((cache)) tree_cache_map *nsdmi_inst; > +static GTY((cache)) decl_tree_cache_map *nsdmi_inst; > > tree > get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) > --- gcc/cp/constraint.cc.jj 2019-11-11 21:04:02.104306246 +0100 > +++ gcc/cp/constraint.cc 2019-11-22 11:47:05.621749384 +0100 > @@ -1113,7 +1113,7 @@ build_constraints (tree tr, tree dr) > > /* A mapping from declarations to constraint information. */ > > -static GTY ((cache)) tree_cache_map *decl_constraints; > +static GTY ((cache)) decl_tree_cache_map *decl_constraints; > > /* Returns the template constraints of declaration T. If T is not > constrained, return NULL_TREE. Note that T must be non-null. */ > --- gcc/cp/decl.c.jj 2019-11-19 22:26:46.289293903 +0100 > +++ gcc/cp/decl.c 2019-11-22 11:49:01.954946691 +0100 > @@ -7998,7 +7998,7 @@ get_tuple_decomp_init (tree decl, unsign > /* It's impossible to recover the decltype of a tuple decomposition variable > 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 GTY((cache)) decl_tree_cache_map *decomp_type_table; > > tree > lookup_decomp_type (tree v) > --- gcc/cp/pt.c.jj 2019-11-19 22:27:00.737077646 +0100 > +++ gcc/cp/pt.c 2019-11-22 11:52:37.590605226 +0100 > @@ -13272,7 +13272,7 @@ tsubst_aggr_type (tree t, > } > } > > -static GTY((cache)) tree_cache_map *defarg_inst; > +static GTY((cache)) decl_tree_cache_map *defarg_inst; > > /* Substitute into the default argument ARG (a default argument for > FN), which has the indicated TYPE. */ > @@ -13346,7 +13346,7 @@ tsubst_default_argument (tree fn, int pa > if (arg != error_mark_node && !cp_unevaluated_operand) > { > if (!defarg_inst) > - defarg_inst = tree_cache_map::create_ggc (37); > + defarg_inst = decl_tree_cache_map::create_ggc (37); > defarg_inst->put (parm, arg); > } > > @@ -13383,7 +13383,7 @@ tsubst_default_arguments (tree fn, tsubs > } > > /* Hash table mapping a FUNCTION_DECL to its dependent explicit-specifier. */ > -static GTY((cache)) tree_cache_map *explicit_specifier_map; > +static GTY((cache)) decl_tree_cache_map *explicit_specifier_map; > > /* Store a pair to EXPLICIT_SPECIFIER_MAP. */ > > @@ -13391,7 +13391,7 @@ void > store_explicit_specifier (tree v, tree t) > { > if (!explicit_specifier_map) > - explicit_specifier_map = tree_cache_map::create_ggc (37); > + explicit_specifier_map = decl_tree_cache_map::create_ggc (37); > DECL_HAS_DEPENDENT_EXPLICIT_SPEC_P (v) = true; > explicit_specifier_map->put (v, t); > } > --- gcc/cp/cp-objcp-common.c.jj 2019-11-06 08:58:38.189471399 +0100 > +++ gcc/cp/cp-objcp-common.c 2019-11-22 11:51:28.090682190 +0100 > @@ -123,7 +123,7 @@ cxx_types_compatible_p (tree x, tree y) > return same_type_ignoring_top_level_qualifiers_p (x, y); > } > > -static GTY((cache)) tree_cache_map *debug_type_map; > +static GTY((cache)) type_tree_cache_map *debug_type_map; > > /* Return a type to use in the debug info instead of TYPE, or NULL_TREE to > keep TYPE. */ > --- gcc/testsuite/g++.dg/pch/pr92458.C.jj 2019-11-22 11:46:31.465278668 +0100 > +++ gcc/testsuite/g++.dg/pch/pr92458.C 2019-11-22 11:46:31.465278668 +0100 > @@ -0,0 +1,5 @@ > +// PR c++/92458 > +// { dg-options "-std=c++2a" } > + > +#include "pr92458.H" > +S<int> s; > --- gcc/testsuite/g++.dg/pch/pr92458.Hs.jj 2019-11-22 11:46:31.466278653 +0100 > +++ gcc/testsuite/g++.dg/pch/pr92458.Hs 2019-11-22 11:46:31.466278653 +0100 > @@ -0,0 +1,7 @@ > +// PR c++/92458 > +// { dg-options "-std=c++2a" } > + > +template<typename T> concept C = sizeof(T) > 1; > +template<typename T> struct S { }; > +template<typename T> requires C<T> struct S<T> { }; > +template<typename T> requires (!C<T>) struct S<T> { }; > > > Jakub >
On Fri, Nov 22, 2019 at 02:43:25PM -0500, Jason Merrill wrote: > > @@ -5358,6 +5410,18 @@ struct tree_cache_traits > > : simple_cache_map_traits<default_hash_traits<tree>, tree> { }; > > We should probably use tree_hash instead of default_hash_traits here. OK > with or without that change. Dunno, I'd say pointer_hash <Type>::hash which is return (hashval_t) ((intptr_t)candidate >> 3); might be actually better hash function than #define TREE_HASH(NODE) ((size_t) (NODE) & 0777777) at least on 64-bit hosts, because union tree_node is 8-byte aligned there, so the low 3 bits are all zero and we use all of the 32 bits above that. For 32-bit hosts, it might be better to just use candidate without shifting I guess, there aren't any extra bits we get in from above. TREE_HASH to me unnecessarily uses the low 3 bits known to be zero and masks with 0x3ffff, so throws away also 14 bits that could hold useful info, leaving for 64-bit hosts only 15 bits. Jakub
On 11/22/19 3:08 PM, Jakub Jelinek wrote: > On Fri, Nov 22, 2019 at 02:43:25PM -0500, Jason Merrill wrote: >>> @@ -5358,6 +5410,18 @@ struct tree_cache_traits >>> : simple_cache_map_traits<default_hash_traits<tree>, tree> { }; >> >> We should probably use tree_hash instead of default_hash_traits here. OK >> with or without that change. > > Dunno, I'd say pointer_hash <Type>::hash which is > return (hashval_t) ((intptr_t)candidate >> 3); > might be actually better hash function than > #define TREE_HASH(NODE) ((size_t) (NODE) & 0777777) > at least on 64-bit hosts, because union tree_node is 8-byte aligned there, > so the low 3 bits are all zero and we use all of the 32 bits above that. > For 32-bit hosts, it might be better to just use candidate without shifting > I guess, there aren't any extra bits we get in from above. > TREE_HASH to me unnecessarily uses the low 3 bits known to be zero and masks > with 0x3ffff, so throws away also 14 bits that could hold useful info, > leaving for 64-bit hosts only 15 bits. True, but that suggests that TREE_HASH should change. Jason
--- gcc/tree-hash-traits.h.jj 2019-01-01 12:37:16.902979134 +0100 +++ gcc/tree-hash-traits.h 2019-11-22 12:00:01.538725844 +0100 @@ -41,44 +41,4 @@ tree_operand_hash::equal (const value_ty return operand_equal_p (t1, t2, 0); } -/* Hasher for tree decls. Pointer equality is enough here, but the DECL_UID - is a better hash than the pointer value and gives a predictable traversal - order. */ -struct tree_decl_hash : ggc_ptr_hash <tree_node> -{ - static inline hashval_t hash (tree); -}; - -inline hashval_t -tree_decl_hash::hash (tree t) -{ - return DECL_UID (t); -} - -/* Hash for SSA_NAMEs in the same function. Pointer equality is enough - here, but the SSA_NAME_VERSION is a better hash than the pointer - value and gives a predictable traversal order. */ -struct tree_ssa_name_hash : ggc_ptr_hash <tree_node> -{ - static inline hashval_t hash (tree); -}; - -inline hashval_t -tree_ssa_name_hash::hash (tree t) -{ - return SSA_NAME_VERSION (t); -} - -/* Hasher for general trees, based on their TREE_HASH. */ -struct tree_hash : ggc_ptr_hash <tree_node> -{ - static hashval_t hash (tree); -}; - -inline hashval_t -tree_hash::hash (tree t) -{ - return TREE_HASH (t); -} - #endif --- gcc/tree.h.jj 2019-11-15 00:37:26.293070143 +0100 +++ gcc/tree.h 2019-11-22 12:00:17.479478830 +0100 @@ -5351,6 +5351,58 @@ struct tree_decl_map_cache_hasher : ggc_ #define tree_vec_map_hash tree_decl_map_hash #define tree_vec_map_marked_p tree_map_base_marked_p +/* Hasher for tree decls. Pointer equality is enough here, but the DECL_UID + is a better hash than the pointer value and gives a predictable traversal + order. Additionally it can be used across PCH save/restore. */ +struct tree_decl_hash : ggc_ptr_hash <tree_node> +{ + static inline hashval_t hash (tree); +}; + +inline hashval_t +tree_decl_hash::hash (tree t) +{ + return DECL_UID (t); +} + +/* Similarly for types. Uses TYPE_UID as hash function. */ +struct tree_type_hash : ggc_ptr_hash <tree_node> +{ + static inline hashval_t hash (tree); +}; + +inline hashval_t +tree_type_hash::hash (tree t) +{ + return TYPE_UID (t); +} + +/* Hash for SSA_NAMEs in the same function. Pointer equality is enough + here, but the SSA_NAME_VERSION is a better hash than the pointer + value and gives a predictable traversal order. */ +struct tree_ssa_name_hash : ggc_ptr_hash <tree_node> +{ + static inline hashval_t hash (tree); +}; + +inline hashval_t +tree_ssa_name_hash::hash (tree t) +{ + return SSA_NAME_VERSION (t); +} + +/* Hasher for general trees, based on their TREE_HASH. */ +struct tree_hash : ggc_ptr_hash <tree_node> +{ + static hashval_t hash (tree); +}; + +inline hashval_t +tree_hash::hash (tree t) +{ + return TREE_HASH (t); +} + /* A hash_map of two trees for use with GTY((cache)). Garbage collection for such a map will not mark keys, and will mark values if the key is already marked. */ @@ -5358,6 +5410,18 @@ struct tree_cache_traits : simple_cache_map_traits<default_hash_traits<tree>, tree> { }; typedef hash_map<tree,tree,tree_cache_traits> tree_cache_map; +/* Similarly, but use DECL_UID as hash function rather than pointer hashing. + This is for hash_maps from decls to trees that need to work across PCH. */ +struct decl_tree_cache_traits + : simple_cache_map_traits<tree_decl_hash, tree> { }; +typedef hash_map<tree,tree,decl_tree_cache_traits> decl_tree_cache_map; + +/* Similarly, but use TYPE_UID as hash function rather than pointer hashing. + This is for hash_maps from types to trees that need to work across PCH. */ +struct type_tree_cache_traits + : simple_cache_map_traits<tree_type_hash, tree> { }; +typedef hash_map<tree,tree,type_tree_cache_traits> type_tree_cache_map; + /* Initialize the abstract argument list iterator object ITER with the arguments from CALL_EXPR node EXP. */ static inline void --- gcc/cp/init.c.jj 2019-11-08 00:22:12.042424097 +0100 +++ gcc/cp/init.c 2019-11-22 11:50:11.285872353 +0100 @@ -548,7 +548,7 @@ perform_target_ctor (tree init) /* Return the non-static data initializer for FIELD_DECL MEMBER. */ -static GTY((cache)) tree_cache_map *nsdmi_inst; +static GTY((cache)) decl_tree_cache_map *nsdmi_inst; tree get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain) --- gcc/cp/constraint.cc.jj 2019-11-11 21:04:02.104306246 +0100 +++ gcc/cp/constraint.cc 2019-11-22 11:47:05.621749384 +0100 @@ -1113,7 +1113,7 @@ build_constraints (tree tr, tree dr) /* A mapping from declarations to constraint information. */ -static GTY ((cache)) tree_cache_map *decl_constraints; +static GTY ((cache)) decl_tree_cache_map *decl_constraints; /* Returns the template constraints of declaration T. If T is not constrained, return NULL_TREE. Note that T must be non-null. */ --- gcc/cp/decl.c.jj 2019-11-19 22:26:46.289293903 +0100 +++ gcc/cp/decl.c 2019-11-22 11:49:01.954946691 +0100 @@ -7998,7 +7998,7 @@ get_tuple_decomp_init (tree decl, unsign /* It's impossible to recover the decltype of a tuple decomposition variable 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 GTY((cache)) decl_tree_cache_map *decomp_type_table; tree lookup_decomp_type (tree v) --- gcc/cp/pt.c.jj 2019-11-19 22:27:00.737077646 +0100 +++ gcc/cp/pt.c 2019-11-22 11:52:37.590605226 +0100 @@ -13272,7 +13272,7 @@ tsubst_aggr_type (tree t, } } -static GTY((cache)) tree_cache_map *defarg_inst; +static GTY((cache)) decl_tree_cache_map *defarg_inst; /* Substitute into the default argument ARG (a default argument for FN), which has the indicated TYPE. */ @@ -13346,7 +13346,7 @@ tsubst_default_argument (tree fn, int pa if (arg != error_mark_node && !cp_unevaluated_operand) { if (!defarg_inst) - defarg_inst = tree_cache_map::create_ggc (37); + defarg_inst = decl_tree_cache_map::create_ggc (37); defarg_inst->put (parm, arg); } @@ -13383,7 +13383,7 @@ tsubst_default_arguments (tree fn, tsubs } /* Hash table mapping a FUNCTION_DECL to its dependent explicit-specifier. */ -static GTY((cache)) tree_cache_map *explicit_specifier_map; +static GTY((cache)) decl_tree_cache_map *explicit_specifier_map; /* Store a pair to EXPLICIT_SPECIFIER_MAP. */ @@ -13391,7 +13391,7 @@ void store_explicit_specifier (tree v, tree t) { if (!explicit_specifier_map) - explicit_specifier_map = tree_cache_map::create_ggc (37); + explicit_specifier_map = decl_tree_cache_map::create_ggc (37); DECL_HAS_DEPENDENT_EXPLICIT_SPEC_P (v) = true; explicit_specifier_map->put (v, t); } --- gcc/cp/cp-objcp-common.c.jj 2019-11-06 08:58:38.189471399 +0100 +++ gcc/cp/cp-objcp-common.c 2019-11-22 11:51:28.090682190 +0100 @@ -123,7 +123,7 @@ cxx_types_compatible_p (tree x, tree y) return same_type_ignoring_top_level_qualifiers_p (x, y); } -static GTY((cache)) tree_cache_map *debug_type_map; +static GTY((cache)) type_tree_cache_map *debug_type_map; /* Return a type to use in the debug info instead of TYPE, or NULL_TREE to keep TYPE. */ --- gcc/testsuite/g++.dg/pch/pr92458.C.jj 2019-11-22 11:46:31.465278668 +0100 +++ gcc/testsuite/g++.dg/pch/pr92458.C 2019-11-22 11:46:31.465278668 +0100 @@ -0,0 +1,5 @@ +// PR c++/92458 +// { dg-options "-std=c++2a" } + +#include "pr92458.H" +S<int> s; --- gcc/testsuite/g++.dg/pch/pr92458.Hs.jj 2019-11-22 11:46:31.466278653 +0100 +++ gcc/testsuite/g++.dg/pch/pr92458.Hs 2019-11-22 11:46:31.466278653 +0100 @@ -0,0 +1,7 @@ +// PR c++/92458 +// { dg-options "-std=c++2a" } + +template<typename T> concept C = sizeof(T) > 1; +template<typename T> struct S { }; +template<typename T> requires C<T> struct S<T> { }; +template<typename T> requires (!C<T>) struct S<T> { };