Re: Proposal: Make TYPE_HASH != TYPE_UID

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Feb 24, 2012 at 2:48 PM, Diego Novillo <dnovillo@google.com> wrote:
>
> Over in the pph branch we are having several failures due to TYPE_CANONICAL
> and the canonical types table.
>
> Entries of the canonical types table corresponding to user-generated types
> get saved on each pre-parsed header. ?We then read the table back in and
> register the hash codes corresponding to each type.
>
> However, we later run into an ICE in comptypes() because the compiler gets
> different hash values from structurally identical types.
>
> The problem here is that the computation of type hash codes use a non-stable
> seed (TYPE_UID). ?When a header file is compiled independently to generate a
> corresponding pre-parsed header, types will receive different UIDs than when
> the file is compiled within a translation unit.
>
> Initially, I was toying with the idea of not saving the hash code but the
> steps needed to re-compute the hash code. ?This is not trivial, since all
> the hash code computations are open coded and do not always depend on the
> type itself (e.g., build_type_attribute_qual_variant).
>
> So, what I think will be more feasible to do is to make sure that types get
> stable hash codes that only depend on structural attributes of the type. ?So
> far, the only unstable element I've seen is the use of TYPE_UID in the hash
> computations.
>
> My proposal is to:
>
> 1- Separate TYPE_HASH from TYPE_UID.

Isn't TYPE_HASH == TYPE_UID?  So how do you exactly want to
"separate" it?  You could re-use gimple_canonical_type_hash, but
that's certainly more expensive.

> 2- Once a type T is entered in the canonical table, make TYPE_HASH(T)
> ? be the hash code that was used to enter the type.

So, store it?  We don't have space for it, do we?

> Another thing I noticed is that type_hash_canon insists that we only enter
> main variants of types. ?However, after being entered, some types get their
> TYPE_MAIN_VARIANT changed (this produces failures when we try to re-enter
> the types out of a PPH header, but this is simpler to resolve).

I think that detail of type_hash_canon is because variants are created off the
main variant, and they are in the type variant list and thus can be looked up
efficiently and thus do not need to be entered in the hash table.
Having spurious
types (non-main-variants) in the hash would only be inefficient.

> Do you see any big problems with my plan?

No, but the proposal is not exactly clear - do you have a patch already?

Richard.

>
> Thanks. ?Diego.