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

Reply via email to