Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)

[Richard Guenther <richard.guenther@gmail.com>]

On Mon, Nov 7, 2011 at 6:55 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
>
>> False.  You get the equivalent of bootstrap comparison mismatches.
>> If we actually used tm during the bootstrap.
>>
>> The simplest thing to do is to change the hash this table uses.
>> E.g. use the DECL_UID right from the start, rather than the pointer.
>
> Woah!  Can it be that easy?  That's as easy as changing the hash, no
> conversion necessary.
>
> OK for branch?

This won't work - DECL_UIDs are not stable -g vs. -g0 - only their
_order_ is stable - thus you won't get comparison fails with code generated
dependent on DECL_UID order, but you will if you depend on the DECL_UID
value (which you do by using it as a hash).

And we will still generate different object files based on garbage collector
settings this way - GC can shrink hashtables, causing re-hashing,
which changes the order of the elements.  It also causes re-ordering
with slight unrelated code changes (but if you say at runtime we always
sort the thing that might not be an issue).

Thus, the patch isn't a fix to get a stable order (you can't get
that for hashtable walks).  A quick fix is to collect the elements
into a VEC and qsort that after some stable key (like the DECL_UID).

Thanks,
Richard.

>        * varasm.c (record_tm_clone_pair): Use DECL_UID as hash.
>        (get_tm_clone_pair): Same.
>
> Index: varasm.c
> ===================================================================
> --- varasm.c    (revision 181067)
> +++ varasm.c    (working copy)
> @@ -5875,7 +5875,7 @@ record_tm_clone_pair (tree o, tree n)
>     tm_clone_pairs = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);
>
>   h = ggc_alloc_tree_map ();
> -  h->hash = htab_hash_pointer (o);
> +  h->hash = DECL_UID (o);
>   h->base.from = o;
>   h->to = n;
>
> @@ -5892,7 +5892,7 @@ get_tm_clone_pair (tree o)
>       struct tree_map *h, in;
>
>       in.base.from = o;
> -      in.hash = htab_hash_pointer (o);
> +      in.hash = DECL_UID (o);
>       h = (struct tree_map *) htab_find_with_hash (tm_clone_pairs,
>                                                   &in, in.hash);
>       if (h)
>