Mikhail Maltsev <malts...@gmail.com> writes: > On 23.06.2015 17:49, Richard Sandiford wrote: >> Index: gcc/config/alpha/alpha.c >> =================================================================== >> --- gcc/config/alpha/alpha.c 2015-06-23 15:48:30.751788389 +0100 >> +++ gcc/config/alpha/alpha.c 2015-06-23 15:48:30.747788453 +0100 >> @@ -4808,13 +4808,7 @@ alpha_multipass_dfa_lookahead (void) >> >> struct GTY(()) alpha_links; >> >> -struct string_traits : default_hashmap_traits >> -{ >> - static bool equal_keys (const char *const &a, const char *const &b) >> - { >> - return strcmp (a, b) == 0; >> - } >> -}; >> +typedef simple_hashmap_traits <nofree_string_hash> string_traits; >> > > I remember that when we briefly discussed unification of string traits, > a looked through GCC code and this one seemed weird to me: it does not > reimplement the hash function. I.e. the pointer value is used as hash. I > wonder, is it intentional or not? This could actually work if strings > are interned (but in that case there is no need to compare them, because > comparing pointers would be enough).
I think it was accidental. The code originally used splay trees and so didn't need to provide a hash. SYMBOL_REF names are unique, like you say, so pointer equality should be enough. Even then though, htab_hash_string ought to give a better hash than the pointer value (as well as giving a stable order, although that isn't important here). So IMO the patch as it stands is still an improvement: we're keeping the existing comparison function but adding a better hasher. If the series goes anywhere I might look at adding a dedicated "interned string hasher" that sits inbetween pointer_hash and string_hash. Thanks, Richard