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
