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

[Richard Guenther <richard dot guenther at gmail dot 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)
>