Prune TYPE_NEXT_VARIANT lists in free_lang_data

Hi
while working on path to replace type variant by first compatible one I run
into issue that the first vairant was not seen by free_lang_data. I think this
is a bug becuase nothing prevents middle-end from looking up a variant there
and using it when it needs to do so.

I tried to walk the variant lists and free lang data on them but that crahses
because not all types in variant list passes verify_type (C++ puts there
incomplete type variants whose canonical type is complete and thus they are
considered bogus). So pruning those lists seems to be better variant.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* tree.c (free_lang_data_in_cgraph): Prune TYPE_NEXT_VARIANT lists.

On September 8, 2018 10:49:02 AM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote:
>Hi
>while working on path to replace type variant by first compatible one I
>run
>into issue that the first vairant was not seen by free_lang_data. I
>think this
>is a bug becuase nothing prevents middle-end from looking up a variant
>there
>and using it when it needs to do so.
>
>I tried to walk the variant lists and free lang data on them but that
>crahses
>because not all types in variant list passes verify_type (C++ puts
>there
>incomplete type variants whose canonical type is complete and thus they
>are
>considered bogus). So pruning those lists seems to be better variant.
>
>Bootstrapped/regtested x86_64-linux, OK?

But if they are still reachable by GC they are now bogously unlinked. We already do not stream unused variants, so what does this achieve? Maybe we should make the variant list GCable somehow? 

>Honza
>
>	* tree.c (free_lang_data_in_cgraph): Prune TYPE_NEXT_VARIANT lists.
>Index: tree.c
>===================================================================
>--- tree.c	(revision 263989)
>+++ tree.c	(working copy)
>@@ -5845,7 +5845,12 @@ free_lang_data_in_cgraph (void)
> 
>   /* Traverse every type found freeing its language data.  */
>   FOR_EACH_VEC_ELT (fld.types, i, t)
>-    free_lang_data_in_type (t);
>+    {
>+      free_lang_data_in_type (t);
>+      while (TYPE_NEXT_VARIANT (t)
>+	     && !fld.pset.contains (TYPE_NEXT_VARIANT (t)))
>+	TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (TYPE_NEXT_VARIANT (t));
>+    }
>   if (flag_checking)
>     {
>       FOR_EACH_VEC_ELT (fld.types, i, t)

> On September 8, 2018 10:49:02 AM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote:
> >Hi
> >while working on path to replace type variant by first compatible one I
> >run
> >into issue that the first vairant was not seen by free_lang_data. I
> >think this
> >is a bug becuase nothing prevents middle-end from looking up a variant
> >there
> >and using it when it needs to do so.
> >
> >I tried to walk the variant lists and free lang data on them but that
> >crahses
> >because not all types in variant list passes verify_type (C++ puts
> >there
> >incomplete type variants whose canonical type is complete and thus they
> >are
> >considered bogus). So pruning those lists seems to be better variant.
> >
> >Bootstrapped/regtested x86_64-linux, OK?
> 
> But if they are still reachable by GC they are now bogously unlinked. We already do not stream unused variants, so what does this achieve? Maybe we should make the variant list GCable somehow? 

Main goal is to prevent middle end to walk the TYPE_NEXT_VARIANT list (for example by get_qualified_variant)
and pick up a variant that was not seen by free_lang_data and put it into the IL.
I know that if we end up referring to some type variant in a way not seen
by free lang data then the type won't be reachable from the list, but why that
would be a problem?

Honza
> 
> >Honza
> >
> >	* tree.c (free_lang_data_in_cgraph): Prune TYPE_NEXT_VARIANT lists.
> >Index: tree.c
> >===================================================================
> >--- tree.c	(revision 263989)
> >+++ tree.c	(working copy)
> >@@ -5845,7 +5845,12 @@ free_lang_data_in_cgraph (void)
> > 
> >   /* Traverse every type found freeing its language data.  */
> >   FOR_EACH_VEC_ELT (fld.types, i, t)
> >-    free_lang_data_in_type (t);
> >+    {
> >+      free_lang_data_in_type (t);
> >+      while (TYPE_NEXT_VARIANT (t)
> >+	     && !fld.pset.contains (TYPE_NEXT_VARIANT (t)))
> >+	TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (TYPE_NEXT_VARIANT (t));
> >+    }
> >   if (flag_checking)
> >     {
> >       FOR_EACH_VEC_ELT (fld.types, i, t)
>

On Sun, 9 Sep 2018, Jan Hubicka wrote:

> > On September 8, 2018 10:49:02 AM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote:
> > >Hi
> > >while working on path to replace type variant by first compatible one I
> > >run
> > >into issue that the first vairant was not seen by free_lang_data. I
> > >think this
> > >is a bug becuase nothing prevents middle-end from looking up a variant
> > >there
> > >and using it when it needs to do so.
> > >
> > >I tried to walk the variant lists and free lang data on them but that
> > >crahses
> > >because not all types in variant list passes verify_type (C++ puts
> > >there
> > >incomplete type variants whose canonical type is complete and thus they
> > >are
> > >considered bogus). So pruning those lists seems to be better variant.
> > >
> > >Bootstrapped/regtested x86_64-linux, OK?
> > 
> > But if they are still reachable by GC they are now bogously unlinked. We already do not stream unused variants, so what does this achieve? Maybe we should make the variant list GCable somehow? 
> 
> Main goal is to prevent middle end to walk the TYPE_NEXT_VARIANT list (for example by get_qualified_variant)
> and pick up a variant that was not seen by free_lang_data and put it into the IL.
> I know that if we end up referring to some type variant in a way not seen
> by free lang data then the type won't be reachable from the list, but why that
> would be a problem?

Duplicate types?  Also the type verifier could assert that a type variant
is in the variant list of its main variant - it looks bogus to break that
invariant at least.

Maybe the real fix is to prune the list in lang specific free-lang-data
so free_lang_data _can_ walk the variant list?  That is, can we track
down those C++ issues and fix them in some way?

Richard.

> Honza
> > 
> > >Honza
> > >
> > >	* tree.c (free_lang_data_in_cgraph): Prune TYPE_NEXT_VARIANT lists.
> > >Index: tree.c
> > >===================================================================
> > >--- tree.c	(revision 263989)
> > >+++ tree.c	(working copy)
> > >@@ -5845,7 +5845,12 @@ free_lang_data_in_cgraph (void)
> > > 
> > >   /* Traverse every type found freeing its language data.  */
> > >   FOR_EACH_VEC_ELT (fld.types, i, t)
> > >-    free_lang_data_in_type (t);
> > >+    {
> > >+      free_lang_data_in_type (t);
> > >+      while (TYPE_NEXT_VARIANT (t)
> > >+	     && !fld.pset.contains (TYPE_NEXT_VARIANT (t)))
> > >+	TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (TYPE_NEXT_VARIANT (t));
> > >+    }
> > >   if (flag_checking)
> > >     {
> > >       FOR_EACH_VEC_ELT (fld.types, i, t)
> > 
> 
>