Free more of type decls

Hi,
this patch frees TYPE_DECL and alignment from TYPE_DECL and also preserves
only those TYPE_DECL pointers that are actually used to build ODR type tree.

It reduces number of TYPE_DECLs streamed from WPA to ltrans to about 20%
and is important for the patch turning types to incomplete types.  Without
this change the TREE_TYPE of TYPE_DECL would still point back to complete type
and duplicating TYPE_DECLs as well is somewhat laborious.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* tree-inline.c (remap_decl): Be ready that TREE_TYPE of TYPE_DECL
	may be NULL.
	* tree-pretty-print.c (dump_generic_node): Likewise.
	* tree.c (free_lang_data_in_type): Free more type decls.
	(free_lang_data_in_decl): Free type and alignment of TYPE_DECL.

On Fri, Oct 26, 2018 at 9:12 AM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> Hi,
> this patch frees TYPE_DECL and alignment from TYPE_DECL and also preserves
> only those TYPE_DECL pointers that are actually used to build ODR type tree.
>
> It reduces number of TYPE_DECLs streamed from WPA to ltrans to about 20%
> and is important for the patch turning types to incomplete types.  Without
> this change the TREE_TYPE of TYPE_DECL would still point back to complete type
> and duplicating TYPE_DECLs as well is somewhat laborious.

So the following is the really important hunk, correct?

> @@ -5174,7 +5174,7 @@ free_lang_data_in_type (tree type)
>
>    /* Drop TYPE_DECLs in TYPE_NAME in favor of the identifier in the
>       TYPE_DECL if the type doesn't have linkage.  */
> -  if (! type_with_linkage_p (type))
> +  if (type != TYPE_MAIN_VARIANT (type) || ! type_with_linkage_p (type))
>      {
>        TYPE_NAME (type) = TYPE_IDENTIFIER (type);
>        TYPE_STUB_DECL (type) = NULL;

Can you explain why you "free" alignment of TYPE_DECLs?  It's just some
bits...  does the FE somehow re-use those for sth else?  I wouldn't have
expected those to be set to anything meaningful.

I'm not too comfortable with setting TREE_TYPE of a TYPE_DECL to NULL.
Can we instead use void_type_node?  The tree-inline.c hunk should
probably test whether DECL_ORIGINAL_TYPE is non-null instead.

Richard.

> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
>         * tree-inline.c (remap_decl): Be ready that TREE_TYPE of TYPE_DECL
>         may be NULL.
>         * tree-pretty-print.c (dump_generic_node): Likewise.
>         * tree.c (free_lang_data_in_type): Free more type decls.
>         (free_lang_data_in_decl): Free type and alignment of TYPE_DECL.
> Index: tree-inline.c
> ===================================================================
> --- tree-inline.c       (revision 265492)
> +++ tree-inline.c       (working copy)
> @@ -382,7 +382,7 @@ remap_decl (tree decl, copy_body_data *i
>           /* Preserve the invariant that DECL_ORIGINAL_TYPE != TREE_TYPE,
>              which is enforced in gen_typedef_die when DECL_ABSTRACT_ORIGIN
>              is not set on the TYPE_DECL, for example in LTO mode.  */
> -         if (DECL_ORIGINAL_TYPE (t) == TREE_TYPE (t))
> +         if (TREE_TYPE (t) && DECL_ORIGINAL_TYPE (t) == TREE_TYPE (t))
>             {
>               tree x = build_variant_type_copy (TREE_TYPE (t));
>               TYPE_STUB_DECL (x) = TYPE_STUB_DECL (TREE_TYPE (t));
> Index: tree-pretty-print.c
> ===================================================================
> --- tree-pretty-print.c (revision 265492)
> +++ tree-pretty-print.c (working copy)
> @@ -1896,7 +1896,7 @@ dump_generic_node (pretty_printer *pp, t
>         }
>        if (DECL_NAME (node))
>         dump_decl_name (pp, node, flags);
> -      else if (TYPE_NAME (TREE_TYPE (node)) != node)
> +      else if (TREE_TYPE (node) && TYPE_NAME (TREE_TYPE (node)) != node)
>         {
>           pp_string (pp, (TREE_CODE (TREE_TYPE (node)) == UNION_TYPE
>                           ? "union" : "struct "));
> Index: tree.c
> ===================================================================
> --- tree.c      (revision 265492)
> +++ tree.c      (working copy)
> @@ -5174,7 +5174,7 @@ free_lang_data_in_type (tree type)
>
>    /* Drop TYPE_DECLs in TYPE_NAME in favor of the identifier in the
>       TYPE_DECL if the type doesn't have linkage.  */
> -  if (! type_with_linkage_p (type))
> +  if (type != TYPE_MAIN_VARIANT (type) || ! type_with_linkage_p (type))
>      {
>        TYPE_NAME (type) = TYPE_IDENTIFIER (type);
>        TYPE_STUB_DECL (type) = NULL;
> @@ -5354,6 +5354,8 @@ free_lang_data_in_decl (tree decl)
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
>        DECL_INITIAL (decl) = NULL_TREE;
>        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> +      TREE_TYPE (decl) = NULL_TREE;
> +      SET_DECL_ALIGN (decl, 0);
>      }
>    else if (TREE_CODE (decl) == FIELD_DECL)
>      DECL_INITIAL (decl) = NULL_TREE;

> On Fri, Oct 26, 2018 at 9:12 AM Jan Hubicka <hubicka@ucw.cz> wrote:
> >
> > Hi,
> > this patch frees TYPE_DECL and alignment from TYPE_DECL and also preserves
> > only those TYPE_DECL pointers that are actually used to build ODR type tree.
> >
> > It reduces number of TYPE_DECLs streamed from WPA to ltrans to about 20%
> > and is important for the patch turning types to incomplete types.  Without
> > this change the TREE_TYPE of TYPE_DECL would still point back to complete type
> > and duplicating TYPE_DECLs as well is somewhat laborious.
> 
> So the following is the really important hunk, correct?
> 
> > @@ -5174,7 +5174,7 @@ free_lang_data_in_type (tree type)
> >
> >    /* Drop TYPE_DECLs in TYPE_NAME in favor of the identifier in the
> >       TYPE_DECL if the type doesn't have linkage.  */
> > -  if (! type_with_linkage_p (type))
> > +  if (type != TYPE_MAIN_VARIANT (type) || ! type_with_linkage_p (type))
> >      {
> >        TYPE_NAME (type) = TYPE_IDENTIFIER (type);
> >        TYPE_STUB_DECL (type) = NULL;
> 
> Can you explain why you "free" alignment of TYPE_DECLs?  It's just some
> bits...  does the FE somehow re-use those for sth else?  I wouldn't have
> expected those to be set to anything meaningful.

It is set to 1 for forward declarations and 8 for fully defined types.
Once I start to turn complete types into incomplete they would not match
becaue of this difference.
> 
> I'm not too comfortable with setting TREE_TYPE of a TYPE_DECL to NULL.
> Can we instead use void_type_node?  The tree-inline.c hunk should
> probably test whether DECL_ORIGINAL_TYPE is non-null instead.

void_type_node works too.  I wanted to put in something that will make any
code that relies on it to crash (so I am sure there is none).

Does the following variant look OK?
I am re-testing it.

	* tree.c (free_lang_data_in_decl): Clear alignment and TREE_TYPE
	of TYPE_DECL.
Index: tree.c
===================================================================
--- tree.c	(revision 265522)
+++ tree.c	(working copy)
@@ -5354,6 +5354,10 @@ free_lang_data_in_decl (tree decl)
       DECL_VISIBILITY_SPECIFIED (decl) = 0;
       DECL_INITIAL (decl) = NULL_TREE;
       DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
+      /* Make sure that complete and incomplete types have same TYPE_DECL.
+	 C++ produces different DECL_ALIGN for them.  */
+      SET_DECL_ALIGN (decl, 0);
+      TREE_TYPE (decl) = void_type_node;
     }
   else if (TREE_CODE (decl) == FIELD_DECL)
     DECL_INITIAL (decl) = NULL_TREE;

On Fri, 26 Oct 2018, Jan Hubicka wrote:

> > On Fri, Oct 26, 2018 at 9:12 AM Jan Hubicka <hubicka@ucw.cz> wrote:
> > >
> > > Hi,
> > > this patch frees TYPE_DECL and alignment from TYPE_DECL and also preserves
> > > only those TYPE_DECL pointers that are actually used to build ODR type tree.
> > >
> > > It reduces number of TYPE_DECLs streamed from WPA to ltrans to about 20%
> > > and is important for the patch turning types to incomplete types.  Without
> > > this change the TREE_TYPE of TYPE_DECL would still point back to complete type
> > > and duplicating TYPE_DECLs as well is somewhat laborious.
> > 
> > So the following is the really important hunk, correct?
> > 
> > > @@ -5174,7 +5174,7 @@ free_lang_data_in_type (tree type)
> > >
> > >    /* Drop TYPE_DECLs in TYPE_NAME in favor of the identifier in the
> > >       TYPE_DECL if the type doesn't have linkage.  */
> > > -  if (! type_with_linkage_p (type))
> > > +  if (type != TYPE_MAIN_VARIANT (type) || ! type_with_linkage_p (type))
> > >      {
> > >        TYPE_NAME (type) = TYPE_IDENTIFIER (type);
> > >        TYPE_STUB_DECL (type) = NULL;
> > 
> > Can you explain why you "free" alignment of TYPE_DECLs?  It's just some
> > bits...  does the FE somehow re-use those for sth else?  I wouldn't have
> > expected those to be set to anything meaningful.
> 
> It is set to 1 for forward declarations and 8 for fully defined types.
> Once I start to turn complete types into incomplete they would not match
> becaue of this difference.

Bah ;)

> > 
> > I'm not too comfortable with setting TREE_TYPE of a TYPE_DECL to NULL.
> > Can we instead use void_type_node?  The tree-inline.c hunk should
> > probably test whether DECL_ORIGINAL_TYPE is non-null instead.
> 
> void_type_node works too.  I wanted to put in something that will make any
> code that relies on it to crash (so I am sure there is none).
>
> Does the following variant look OK?
> I am re-testing it.

Yes.
 
Thanks,
Richard.

> 	* tree.c (free_lang_data_in_decl): Clear alignment and TREE_TYPE
> 	of TYPE_DECL.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 265522)
> +++ tree.c	(working copy)
> @@ -5354,6 +5354,10 @@ free_lang_data_in_decl (tree decl)
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
>        DECL_INITIAL (decl) = NULL_TREE;
>        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> +      /* Make sure that complete and incomplete types have same TYPE_DECL.
> +	 C++ produces different DECL_ALIGN for them.  */
> +      SET_DECL_ALIGN (decl, 0);
> +      TREE_TYPE (decl) = void_type_node;
>      }
>    else if (TREE_CODE (decl) == FIELD_DECL)
>      DECL_INITIAL (decl) = NULL_TREE;
> 
>