> On Sun, Oct 7, 2012 at 7:22 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
> >> On Sun, Oct 7, 2012 at 5:15 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
> >> > Hi,
> >> > I added a santy check that after fixup all types that lost in the
> >> > merging are
> >> > really dead. And it turns out we have some zombies around.
> >> >
> >> > INTEGER_CST needs special care because it is special cased by the
> >> > streamer. We also
> >> > do not want to do inplace modificaitons on it because that would corrupt
> >> > the hashtable
> >> > used by tree.c's sharing code
> >> >
> >> > Bootstrapped/regtested x86_64-linux, OK?
> >>
> >> No, I don't think we want to fixup INTEGER_CSTs this way. Instead we
> >> want to fixup
> >> them where they end up used unfixed.
> >
> > Erm, I think it is what the patch does?
>
> Ah, indeed.
>
> > It replaces pointers to integer_cst with type that did not survive by
> > pointer
> > to new integer_cst. (with the optimization that INTEGER_CST with overflow
> > is changed in place because it is allowed to do so).
>
> Btw ...
>
> >> > @@ -1526,6 +1549,11 @@ lto_ft_type (tree t)
> >> > LTO_FIXUP_TREE (t->type_non_common.binfo);
> >> >
> >> > LTO_FIXUP_TREE (TYPE_CONTEXT (t));
> >> > +
> >> > + if (TREE_CODE (t) == METHOD_TYPE)
> >> > + TYPE_METHOD_BASETYPE (t);
> >> > + if (TREE_CODE (t) == OFFSET_TYPE)
> >> > + TYPE_OFFSET_BASETYPE (t);
>
> that looks like a no-op to me ... (both are TYPE_MAXVAL which
> is already fixed up).
Ah, indeed. They were result of experimenting with the stale pointers to the
obsoletted types and field decls. I now understand where they come from. The
reason is twofold.
1) after merging records we replace field decls in the cache
by new ones. This however does not mean that they die, because
the existing pointers to them are not replaced.
I have WIP patch for that that however require one extra pass
over the list of all trees.
2) As we query the type_hash while we are rewritting the types,
we run into instability of the hashtable. This manifests itself
as an ICE when one adds sanity check that while merging function
types their arg types are equivalent, too.
This ICEs compiling i.e. sqlite but I did not really managed to
reduce this. I tracked it down to the argument type being inserted
into gimple_type_hash but at the time we query the new argument type,
the original is no longer found despite their hashes are equivalent.
The problem is hidden when things fit into the leader cache,
so one needs rather big testcase.
So I tried to register all gimple types first. Use TREE_VISITED within
the merging code to mark that type is not a leader and then TREE_CHAIN
to point to the leader. This avoids need to re-query the hashtable
from the later fixups. We only look for types with TREEE_VISITED
and replace them by TREE_CHAIN.
This has two passes. First we compute the main variants and mark
field_decls and type_decls for merging and in last pass we finally do
fixup on what remained in the table.
This allows me to poison pointers in the removed types in a way
so the GGC would ICE if they stayed reachable.
I however need the extra pass because
1) I can not update the type_decls/field_decls while registering
types or I run into the hash table problems
2) I can not merge the second two passes because at the time
I find type/field decls equialent there may be earlier pointers
to them.
Honza
