Turn complete to incomplete types in free_lang_data

Hi,
this is minimal variant of the patch turning complete to incomplete pointers in
fields.  We can do more - in particular it would be very function to do same
for functions types and decls (because they often end up being streamed to
symtab) and we should also turn pointers to arrays and enums to incomplete
variants.

I do that in my local tree but i would like to get it into mainline one by
one and check benefits of each change independently. 

Patch bootstraped&regtests x86-64 and I am now re-testing it on firefox.  I
checked on small testcases that types indeed do get merged.

OK if it survives more testing on firefox and lto bootstrap?

Honza

	* tree.c (free_lang_data_in_type): Declare.
	(types_equal_p): New function.
	(free_lang_data_type_variant): New function.
	(incomplete_type_of): New function.
	(simplified_type): New function.

On Fri, Oct 26, 2018 at 1:29 PM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> Hi,
> this is minimal variant of the patch turning complete to incomplete pointers in
> fields.  We can do more - in particular it would be very function to do same
> for functions types and decls (because they often end up being streamed to
> symtab) and we should also turn pointers to arrays and enums to incomplete
> variants.
>
> I do that in my local tree but i would like to get it into mainline one by
> one and check benefits of each change independently.
>
> Patch bootstraped&regtests x86-64 and I am now re-testing it on firefox.  I
> checked on small testcases that types indeed do get merged.
>
> OK if it survives more testing on firefox and lto bootstrap?

It looks like a hack to do free_lang_data_in_type from free_lang_data_in_decl
walk - I remember you wanted to unify find_* and free_*?  If not doing that
why would first doing the type walk and only then the decl walk not work
to avoid this ugliness?

That we need to have variants of the incomplete types at all for the place
you substitute them (FIELD_DECLs) has what reason?  See also comments below...

We are getting more and more "interesting" in things we free.  _Please_ work on
enabling free-lang-data (portions) for all compilations (with
-fchecking?).  It's disturbing to see
so much differences creep in in the supposedly "shared" part of regular and
LTO compilation.

> Honza
>
>         * tree.c (free_lang_data_in_type): Declare.
>         (types_equal_p): New function.
>         (free_lang_data_type_variant): New function.
>         (incomplete_type_of): New function.
>         (simplified_type): New function.
> Index: tree.c
> ===================================================================
> --- tree.c      (revision 265522)
> +++ tree.c      (working copy)
> @@ -265,6 +265,8 @@ static void print_type_hash_statistics (
>  static void print_debug_expr_statistics (void);
>  static void print_value_expr_statistics (void);
>
> +static void free_lang_data_in_type (tree type);
> +
>  tree global_trees[TI_MAX];
>  tree integer_types[itk_none];
>
> @@ -5038,6 +5041,140 @@ protected_set_expr_location (tree t, loc
>      SET_EXPR_LOCATION (t, loc);
>  }
>
> +/* Do same comparsion as check_qualified_type skipping lang part of type
> +   and be more permissive about type names: we only care that names are
> +   same (for diagnostics) and that ODR names are the same.  */
> +
> +static bool
> +types_equal_p (tree t, tree v)

The function name is of course totally misleading.  Please use sth like
fld_type_variants_equal_p.

Note we already split check_qualified_type - can't you somehow re-use
check_base_type (only)?

> +{
> +  if (t==v)
> +    return true;
> +
> +  if (TYPE_QUALS (t) != TYPE_QUALS (v))
> +    return false;
> +
> +  if (TYPE_NAME (t) != TYPE_NAME (v)
> +      && (!TYPE_NAME (t) || !TYPE_NAME (v)
> +         || TREE_CODE (TYPE_NAME (t)) != TYPE_DECL
> +         || TREE_CODE (TYPE_NAME (v)) != TYPE_DECL
> +         || DECL_ASSEMBLER_NAME_RAW (TYPE_NAME (t))
> +            != DECL_ASSEMBLER_NAME_RAW (TYPE_NAME (v))

I wonder what this is about...

> +         || DECL_NAME (TYPE_NAME (t)) != DECL_NAME (TYPE_NAME (v))))

...or this, given we may end up turning DECL_NAMEs to INDENTIFIER_NODEs so
this will crash.

> +     return false;
> +
> +  if (TYPE_ALIGN (t) != TYPE_ALIGN (v))
> +    return false;
> +
> +  if (!attribute_list_equal (TYPE_ATTRIBUTES (t),
> +                            TYPE_ATTRIBUTES (v)))
> +     return false;
> +
> +  /* Do not replace complete type by incomplete.  */
> +  if ((TREE_CODE (t) == QUAL_UNION_TYPE
> +       || TREE_CODE (t) == UNION_TYPE || TREE_CODE (t) == RECORD_TYPE)
> +      && COMPLETE_TYPE_P (t) != COMPLETE_TYPE_P (v))
> +    return false;

?  It looks like this function is a left-over from another patch and
the incomplete
type building could use sth leaner and more clearer?


> +
> +  gcc_assert (TREE_CODE (t) == TREE_CODE (v));
> +
> +  /* For pointer types and array types we also care about the type they
> +     reffer to.  */
> +  if (TREE_TYPE (t))
> +    return types_equal_p (TREE_TYPE (t), TREE_TYPE (v));
> +
> +  return true;
> +}
> +
> +/* Find variant of FIRST that match T and create new one if necessary.  */
> +
> +static tree
> +free_lang_data_type_variant (tree first, tree t)
> +{
> +  if (first == TYPE_MAIN_VARIANT (t))
> +    return t;
> +  for (tree v = first; v; v = TYPE_NEXT_VARIANT (v))
> +    if (types_equal_p (t, v))
> +      return v;
> +  tree v = build_variant_type_copy (first);
> +  TYPE_READONLY (v) = TYPE_READONLY (t);
> +  TYPE_VOLATILE (v) = TYPE_VOLATILE (t);
> +  TYPE_ATOMIC (v) = TYPE_ATOMIC (t);
> +  TYPE_RESTRICT (v) = TYPE_RESTRICT (t);
> +  TYPE_ADDR_SPACE (v) = TYPE_ADDR_SPACE (t);
> +  TYPE_NAME (v) = TYPE_NAME (t);
> +  TYPE_ATTRIBUTES (v) = TYPE_ATTRIBUTES (t);
> +  return v;
> +}
> +
> +/* Map complete types to incomplete types.  */
> +static hash_map<tree, tree> *incomplete_types;
> +
> +/* See if type T can be turned into incopmlete variant.  */
> +
> +static tree
> +incomplete_type_of (tree t)
> +{
> +  if (!RECORD_OR_UNION_TYPE_P (t))
> +    return t;
> +  if (!COMPLETE_TYPE_P (t))
> +    return t;
> +  if (TYPE_MAIN_VARIANT (t) == t)
> +    {
> +      bool existed;
> +      tree &copy
> +        = incomplete_types->get_or_insert (TYPE_MAIN_VARIANT (t), &existed);
> +
> +      if (!existed)
> +       {
> +         copy = build_distinct_type_copy (t);
> +
> +         /* It is possible type was not seen by free_lang_data yet.  */
> +         free_lang_data_in_type (copy);
> +         TYPE_SIZE (copy) = NULL;
> +         SET_TYPE_MODE (copy, VOIDmode);
> +         SET_TYPE_ALIGN (copy, BITS_PER_UNIT);
> +         TYPE_SIZE_UNIT (copy) = NULL;
> +         if (AGGREGATE_TYPE_P (t))
> +           {
> +             TYPE_FIELDS (copy) = NULL;
> +             TYPE_BINFO (copy) = NULL;
> +           }
> +         else
> +           TYPE_VALUES (copy) = NULL;
> +       }
> +      return copy;
> +   }
> +  return (free_lang_data_type_variant
> +           (incomplete_type_of (TYPE_MAIN_VARIANT (t)), t));
> +}
> +
> +/* Simplify type T for scenarios where we do not need complete pointer
> +   types.  */
> +
> +static tree
> +simplified_type (tree t)
> +{
> +  if (POINTER_TYPE_P (t))
> +    {
> +      tree t2 = POINTER_TYPE_P (TREE_TYPE (t))
> +               ? simplified_type (TREE_TYPE (t))
> +               : incomplete_type_of (TREE_TYPE (t));
> +      if (t2 != TREE_TYPE (t))
> +       {
> +         tree first;
> +         if (TREE_CODE (t) == POINTER_TYPE)
> +           first = build_pointer_type_for_mode (t2, TYPE_MODE (t),
> +                                               TYPE_REF_CAN_ALIAS_ALL (t));
> +         else
> +           first = build_reference_type_for_mode (t2, TYPE_MODE (t),
> +                                               TYPE_REF_CAN_ALIAS_ALL (t));
> +         return free_lang_data_type_variant (first, t);
> +       }
> +    }
> +  return t;
> +}
> +
>  /* Reset the expression *EXPR_P, a size or position.
>
>     ??? We could reset all non-constant sizes or positions.  But it's cheap
> @@ -5354,9 +5492,11 @@ free_lang_data_in_decl (tree decl)
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
>        DECL_INITIAL (decl) = NULL_TREE;
>        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
>      }
>    else if (TREE_CODE (decl) == FIELD_DECL)
> -    DECL_INITIAL (decl) = NULL_TREE;
> +    {
> +      TREE_TYPE (decl) = simplified_type (TREE_TYPE (decl));
> +      DECL_INITIAL (decl) = NULL_TREE;
> +    }
>    else if (TREE_CODE (decl) == TRANSLATION_UNIT_DECL
>             && DECL_INITIAL (decl)
>             && TREE_CODE (DECL_INITIAL (decl)) == BLOCK)
> @@ -5866,6 +6011,8 @@ free_lang_data (void)
>        || (!flag_generate_lto && !flag_generate_offload))
>      return 0;
>
> +  incomplete_types = new hash_map<tree, tree>;
> +
>    /* Provide a dummy TRANSLATION_UNIT_DECL if the FE failed to provide one.  */
>    if (vec_safe_is_empty (all_translation_units))
>      build_translation_unit_decl (NULL_TREE);

> > OK if it survives more testing on firefox and lto bootstrap?
> 
> It looks like a hack to do free_lang_data_in_type from free_lang_data_in_decl
> walk - I remember you wanted to unify find_* and free_*?  If not doing that

I did try it :) There is a catch - free lang data calls langhooks to produce mangled
assembler names. For that the trees must be non-freed yet.
So you can't do freeing as you discover what trees to follow.

We can save one walk by computing assembler names during the discovery, but we
need to know all trees we want to do langhooks on before we start putting NULL
pointers around.

> why would first doing the type walk and only then the decl walk not work
> to avoid this ugliness?

Hmm, currently we first walk decl and then types,so swapping them woudl work.
But since I want to also simplify types in function types, it would break next.

> 
> That we need to have variants of the incomplete types at all for the place
> you substitute them (FIELD_DECLs) has what reason?  See also comments below...
> 
> We are getting more and more "interesting" in things we free.  _Please_ work on
> enabling free-lang-data (portions) for all compilations (with
> -fchecking?).  It's disturbing to see
> so much differences creep in in the supposedly "shared" part of regular and
> LTO compilation.

I wonder what is the plan to make late warnings to work reliably in this case?

> > +/* Do same comparsion as check_qualified_type skipping lang part of type
> > +   and be more permissive about type names: we only care that names are
> > +   same (for diagnostics) and that ODR names are the same.  */
> > +
> > +static bool
> > +types_equal_p (tree t, tree v)
> 
> The function name is of course totally misleading.  Please use sth like
> fld_type_variants_equal_p.
> 
> Note we already split check_qualified_type - can't you somehow re-use
> check_base_type (only)?

Hmm, you are right. I can re-unify those since this function basically cared
about ...
> 
> > +{
> > +  if (t==v)
> > +    return true;
> > +
> > +  if (TYPE_QUALS (t) != TYPE_QUALS (v))
> > +    return false;
> > +
> > +  if (TYPE_NAME (t) != TYPE_NAME (v)
> > +      && (!TYPE_NAME (t) || !TYPE_NAME (v)
> > +         || TREE_CODE (TYPE_NAME (t)) != TYPE_DECL
> > +         || TREE_CODE (TYPE_NAME (v)) != TYPE_DECL
> > +         || DECL_ASSEMBLER_NAME_RAW (TYPE_NAME (t))
> > +            != DECL_ASSEMBLER_NAME_RAW (TYPE_NAME (v))
> 
> I wonder what this is about...

... unmerged TYPE_NAMEs which happens only on WPA state. It is leftover of
my merging during streaming experiment.

I will clean this up and send updated patch.  I was bit in hurry leaving today
and wanted to send at least initial patch for discussion.

Honza

Hi,
this is cleaner version of the patch.  During weekend I did some tests with
firefox, libreoffice and gcc builds and it seems to work well. For firefox
it reaches almost linear scalability of the ltrans files (they are 1.9GB,
after increasing number of partitions to 128 they grow to 2GB that looks
quite acceptable. Resulting .s and libxul binary are actually bigger than
that when debug info is enabled).

So if that works in other cases, I will increas lto-partitions and probably
declare it good enough for this stage1 and will try to move to other things.

Concerning two things we have discussed, I am keeping recursion to
free_lang_data_in_type for now as reordering seems just temporary solution
until we do more freeing from both types and decl (eventually I want to free
subtypes of function types that also brings a lot of context. On Firefox that
acount another 5% of stream data volume).

One option would be to change the walking order for this stage1 and worry
about it next stage1.  Other option would be to simply push the newly created
types to the fld queues.

I also did not share fld_type_variant_equal_p with check_base_type since
that one checks also context that we do not want to do. Probably could be
cleaned up incrementally - I wonder why Objective-C needs it.

I looked into enabling free_lang_data by default.  My main problem there is
what to do with late langhooks, say one for variably_modified_type_p.
I suppose we could just declare middle-end self contained wrt analyzing
IL and try to disable them one by one after free lang data was run?

What we however want to do about late warnings? Do we have some idea how
many are those and what kind of % modifiers needs to be printed correctly?
Say late warning wants to print a type, how we are going to do that?

Boostrapped/regtested x86_64-linux.

Honza
	* tree.c (free_lang_data_in_type): Forward declare.
	(fld_type_variant_equal_p): New function.
	(fld_type_variant): New function
	(fld_incomplete_types): New hash.
	(fld_incomplete_type_of): New function
	(fld_simplfied-type): New function.
	(free_lang_data_in_decl): New.
Index: tree.c
===================================================================
--- tree.c	(revision 265573)
+++ tree.c	(working copy)
@@ -265,6 +265,8 @@
 static void print_debug_expr_statistics (void);
 static void print_value_expr_statistics (void);
 
+static void free_lang_data_in_type (tree type);
+
 tree global_trees[TI_MAX];
 tree integer_types[itk_none];
 
@@ -5038,6 +5041,115 @@
     SET_EXPR_LOCATION (t, loc);
 }
 
+/* Do same comparsion as check_qualified_type skipping lang part of type
+   and be more permissive about type names: we only care that names are
+   same (for diagnostics) and that ODR names are the same.  */
+
+static bool
+fld_type_variant_equal_p (tree t, tree v)
+{
+  if (TYPE_QUALS (t) != TYPE_QUALS (v)
+      || TYPE_NAME (t) != TYPE_NAME (v)
+      || TYPE_ALIGN (t) != TYPE_ALIGN (v)
+      || !attribute_list_equal (TYPE_ATTRIBUTES (t),
+			        TYPE_ATTRIBUTES (v)))
+    return false;
+
+  return true;
+}
+
+/* Find variant of FIRST that match T and create new one if necessary.  */
+
+static tree
+fld_type_variant (tree first, tree t)
+{
+  if (first == TYPE_MAIN_VARIANT (t))
+    return t;
+  for (tree v = first; v; v = TYPE_NEXT_VARIANT (v))
+    if (fld_type_variant_equal_p (t, v))
+      return v;
+  tree v = build_variant_type_copy (first);
+  TYPE_READONLY (v) = TYPE_READONLY (t);
+  TYPE_VOLATILE (v) = TYPE_VOLATILE (t);
+  TYPE_ATOMIC (v) = TYPE_ATOMIC (t);
+  TYPE_RESTRICT (v) = TYPE_RESTRICT (t);
+  TYPE_ADDR_SPACE (v) = TYPE_ADDR_SPACE (t);
+  TYPE_NAME (v) = TYPE_NAME (t);
+  TYPE_ATTRIBUTES (v) = TYPE_ATTRIBUTES (t);
+  return v;
+}
+
+/* Map complete types to incomplete types.  */
+
+static hash_map<tree, tree> *fld_incomplete_types;
+
+/* For T being aggregate type try to turn it into a incomplete variant.
+   Return T if no simplification is possible.  */
+
+static tree
+fld_incomplete_type_of (tree t)
+{
+  if (!t)
+    return NULL;
+  if (POINTER_TYPE_P (t))
+    {
+      tree t2 = fld_incomplete_type_of (TREE_TYPE (t));
+      if (t2 != TREE_TYPE (t))
+	{
+	  tree first;
+	  if (TREE_CODE (t) == POINTER_TYPE)
+	    first = build_pointer_type_for_mode (t2, TYPE_MODE (t),
+						TYPE_REF_CAN_ALIAS_ALL (t));
+	  else
+	    first = build_reference_type_for_mode (t2, TYPE_MODE (t),
+						TYPE_REF_CAN_ALIAS_ALL (t));
+	  return fld_type_variant (first, t);
+	}
+      return t;
+    }
+  if (!RECORD_OR_UNION_TYPE_P (t) || !COMPLETE_TYPE_P (t))
+    return t;
+  if (TYPE_MAIN_VARIANT (t) == t)
+    {
+      bool existed;
+      tree &copy
+	 = fld_incomplete_types->get_or_insert (t, &existed);
+
+      if (!existed)
+	{
+	  copy = build_distinct_type_copy (t);
+
+	  /* It is possible type was not seen by free_lang_data yet.  */
+	  free_lang_data_in_type (copy);
+	  TYPE_SIZE (copy) = NULL;
+	  SET_TYPE_MODE (copy, VOIDmode);
+	  SET_TYPE_ALIGN (copy, BITS_PER_UNIT);
+	  TYPE_SIZE_UNIT (copy) = NULL;
+	  if (AGGREGATE_TYPE_P (t))
+	    {
+	      TYPE_FIELDS (copy) = NULL;
+	      TYPE_BINFO (copy) = NULL;
+	    }
+	  else
+	    TYPE_VALUES (copy) = NULL;
+	}
+      return copy;
+   }
+  return (fld_type_variant
+	    (fld_incomplete_type_of (TYPE_MAIN_VARIANT (t)), t));
+}
+
+/* Simplify type T for scenarios where we do not need complete pointer
+   types.  */
+
+static tree
+fld_simplified_type (tree t)
+{
+  if (t && POINTER_TYPE_P (t))
+    return fld_incomplete_type_of (t);
+  return t;
+}
+
 /* Reset the expression *EXPR_P, a size or position.
 
    ??? We could reset all non-constant sizes or positions.  But it's cheap
@@ -5354,9 +5470,12 @@
       DECL_VISIBILITY_SPECIFIED (decl) = 0;
       DECL_INITIAL (decl) = NULL_TREE;
       DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
     }
   else if (TREE_CODE (decl) == FIELD_DECL)
-    DECL_INITIAL (decl) = NULL_TREE;
+    {
+      TREE_TYPE (decl) = fld_simplified_type (TREE_TYPE (decl));
+      DECL_INITIAL (decl) = NULL_TREE;
+    }
   else if (TREE_CODE (decl) == TRANSLATION_UNIT_DECL
            && DECL_INITIAL (decl)
            && TREE_CODE (DECL_INITIAL (decl)) == BLOCK)
@@ -5866,6 +5989,8 @@
       || (!flag_generate_lto && !flag_generate_offload))
     return 0;
 
+  fld_incomplete_types = new hash_map<tree, tree>;
+
   /* Provide a dummy TRANSLATION_UNIT_DECL if the FE failed to provide one.  */
   if (vec_safe_is_empty (all_translation_units))
     build_translation_unit_decl (NULL_TREE);

On Fri, 26 Oct 2018, Jan Hubicka wrote:

> > > OK if it survives more testing on firefox and lto bootstrap?
> > 
> > It looks like a hack to do free_lang_data_in_type from free_lang_data_in_decl
> > walk - I remember you wanted to unify find_* and free_*?  If not doing that
> 
> I did try it :) There is a catch - free lang data calls langhooks to produce mangled
> assembler names. For that the trees must be non-freed yet.
> So you can't do freeing as you discover what trees to follow.
> 
> We can save one walk by computing assembler names during the discovery, but we
> need to know all trees we want to do langhooks on before we start putting NULL
> pointers around.
> 
> > why would first doing the type walk and only then the decl walk not work
> > to avoid this ugliness?
> 
> Hmm, currently we first walk decl and then types,so swapping them woudl work.
> But since I want to also simplify types in function types, it would break next.
> 
> > 
> > That we need to have variants of the incomplete types at all for the place
> > you substitute them (FIELD_DECLs) has what reason?  See also comments below...
> > 
> > We are getting more and more "interesting" in things we free.  _Please_ work on
> > enabling free-lang-data (portions) for all compilations (with
> > -fchecking?).  It's disturbing to see
> > so much differences creep in in the supposedly "shared" part of regular and
> > LTO compilation.
> 
> I wonder what is the plan to make late warnings to work reliably in this case?

Well - late warnings are a red herring.  And we have caret locations 
citing source.  And we could look into the DWARF (well, not from lto1,
but there it's "broken" already anyways)

> > > +/* Do same comparsion as check_qualified_type skipping lang part of type
> > > +   and be more permissive about type names: we only care that names are
> > > +   same (for diagnostics) and that ODR names are the same.  */
> > > +
> > > +static bool
> > > +types_equal_p (tree t, tree v)
> > 
> > The function name is of course totally misleading.  Please use sth like
> > fld_type_variants_equal_p.
> > 
> > Note we already split check_qualified_type - can't you somehow re-use
> > check_base_type (only)?
> 
> Hmm, you are right. I can re-unify those since this function basically cared
> about ...
> > 
> > > +{
> > > +  if (t==v)
> > > +    return true;
> > > +
> > > +  if (TYPE_QUALS (t) != TYPE_QUALS (v))
> > > +    return false;
> > > +
> > > +  if (TYPE_NAME (t) != TYPE_NAME (v)
> > > +      && (!TYPE_NAME (t) || !TYPE_NAME (v)
> > > +         || TREE_CODE (TYPE_NAME (t)) != TYPE_DECL
> > > +         || TREE_CODE (TYPE_NAME (v)) != TYPE_DECL
> > > +         || DECL_ASSEMBLER_NAME_RAW (TYPE_NAME (t))
> > > +            != DECL_ASSEMBLER_NAME_RAW (TYPE_NAME (v))
> > 
> > I wonder what this is about...
> 
> ... unmerged TYPE_NAMEs which happens only on WPA state. It is leftover of
> my merging during streaming experiment.
> 
> I will clean this up and send updated patch.  I was bit in hurry leaving today
> and wanted to send at least initial patch for discussion.

OK, btw I noticed that we simply do

      DECL_ORIGINAL_TYPE (decl) = NULL_TREE;

on TYPE_DECLs.  But the bigger benefit would be dropping the typedef
variants completely by doing

  if (TREE_CODE (TYPE_NAME (TREE_TYPE (t))) == TYPE_DECL
      && DECL_ORIGINAL_TYPE (TYPE_NAME (TREE_TYPE (t))))
    TREE_TYPE (t) = DECL_ORIGINAL_TYPE (TYPE_NAME (TREE_TYPE (t)));

for all type references.  That should get rid of most TYPE_DECLs
(the DECL_ORIGINAL_TYPE has no name IIRC).

Simply NULLing DECL_ORIGINAL_TYPE on TYPE_DECLs will make this
impossible.

Richard.

> > ... unmerged TYPE_NAMEs which happens only on WPA state. It is leftover of
> > my merging during streaming experiment.
> > 
> > I will clean this up and send updated patch.  I was bit in hurry leaving today
> > and wanted to send at least initial patch for discussion.
> 
> OK, btw I noticed that we simply do
> 
>       DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> 
> on TYPE_DECLs.  But the bigger benefit would be dropping the typedef
> variants completely by doing
> 
>   if (TREE_CODE (TYPE_NAME (TREE_TYPE (t))) == TYPE_DECL
>       && DECL_ORIGINAL_TYPE (TYPE_NAME (TREE_TYPE (t))))
>     TREE_TYPE (t) = DECL_ORIGINAL_TYPE (TYPE_NAME (TREE_TYPE (t)));
> 
> for all type references.  That should get rid of most TYPE_DECLs
> (the DECL_ORIGINAL_TYPE has no name IIRC).
> 
> Simply NULLing DECL_ORIGINAL_TYPE on TYPE_DECLs will make this
> impossible.

I was playing with that some time ago, too. Even after freeing original
type we could go via variant list to find first compatible variant and
zap unnecessary variants at the same time.

This confuses warning that can no longer name typedefs and speak
of original types instead.  This is why I am keeping the test for
DECL_NAME to match but translate them to pure IDENTIFIER_NODE.

It was sub 2% change whether or not to save different typedef names
with my oriignal type merging experiment.

Honza

On Mon, 29 Oct 2018, Jan Hubicka wrote:

> Hi,
> this is cleaner version of the patch.  During weekend I did some tests with
> firefox, libreoffice and gcc builds and it seems to work well. For firefox
> it reaches almost linear scalability of the ltrans files (they are 1.9GB,
> after increasing number of partitions to 128 they grow to 2GB that looks
> quite acceptable. Resulting .s and libxul binary are actually bigger than
> that when debug info is enabled).
> 
> So if that works in other cases, I will increas lto-partitions and probably
> declare it good enough for this stage1 and will try to move to other things.
> 
> Concerning two things we have discussed, I am keeping recursion to
> free_lang_data_in_type for now as reordering seems just temporary solution
> until we do more freeing from both types and decl (eventually I want to free
> subtypes of function types that also brings a lot of context. On Firefox that
> acount another 5% of stream data volume).
> 
> One option would be to change the walking order for this stage1 and worry
> about it next stage1.  Other option would be to simply push the newly created
> types to the fld queues.

If that works this sounds best.

> I also did not share fld_type_variant_equal_p with check_base_type since
> that one checks also context that we do not want to do. Probably could be
> cleaned up incrementally - I wonder why Objective-C needs it.
> 
> I looked into enabling free_lang_data by default.  My main problem there is
> what to do with late langhooks, say one for variably_modified_type_p.
> I suppose we could just declare middle-end self contained wrt analyzing
> IL and try to disable them one by one after free lang data was run?

Yes, that was the original idea.  IIRC enabling free-lang-data 
unconditionally mostly works fine apart from some testcases issues
with late diagnostics and dump scanning.

> What we however want to do about late warnings? Do we have some idea how
> many are those and what kind of % modifiers needs to be printed correctly?
> Say late warning wants to print a type, how we are going to do that?

Well, we print the type in the middle-end way - basically 
install the LTO variant of the langhooks.

You fail to free fld_incomplete_types btw.  The patch looks sensible
with that change - possibly with removing the recursion and pushing
to the worklist instead.

Thanks,
Richard.

> Boostrapped/regtested x86_64-linux.
> 
> Honza
> 	* tree.c (free_lang_data_in_type): Forward declare.
> 	(fld_type_variant_equal_p): New function.
> 	(fld_type_variant): New function
> 	(fld_incomplete_types): New hash.
> 	(fld_incomplete_type_of): New function
> 	(fld_simplfied-type): New function.
> 	(free_lang_data_in_decl): New.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 265573)
> +++ tree.c	(working copy)
> @@ -265,6 +265,8 @@
>  static void print_debug_expr_statistics (void);
>  static void print_value_expr_statistics (void);
>  
> +static void free_lang_data_in_type (tree type);
> +
>  tree global_trees[TI_MAX];
>  tree integer_types[itk_none];
>  
> @@ -5038,6 +5041,115 @@
>      SET_EXPR_LOCATION (t, loc);
>  }
>  
> +/* Do same comparsion as check_qualified_type skipping lang part of type
> +   and be more permissive about type names: we only care that names are
> +   same (for diagnostics) and that ODR names are the same.  */
> +
> +static bool
> +fld_type_variant_equal_p (tree t, tree v)
> +{
> +  if (TYPE_QUALS (t) != TYPE_QUALS (v)
> +      || TYPE_NAME (t) != TYPE_NAME (v)
> +      || TYPE_ALIGN (t) != TYPE_ALIGN (v)
> +      || !attribute_list_equal (TYPE_ATTRIBUTES (t),
> +			        TYPE_ATTRIBUTES (v)))
> +    return false;
> +
> +  return true;
> +}
> +
> +/* Find variant of FIRST that match T and create new one if necessary.  */
> +
> +static tree
> +fld_type_variant (tree first, tree t)
> +{
> +  if (first == TYPE_MAIN_VARIANT (t))
> +    return t;
> +  for (tree v = first; v; v = TYPE_NEXT_VARIANT (v))
> +    if (fld_type_variant_equal_p (t, v))
> +      return v;
> +  tree v = build_variant_type_copy (first);
> +  TYPE_READONLY (v) = TYPE_READONLY (t);
> +  TYPE_VOLATILE (v) = TYPE_VOLATILE (t);
> +  TYPE_ATOMIC (v) = TYPE_ATOMIC (t);
> +  TYPE_RESTRICT (v) = TYPE_RESTRICT (t);
> +  TYPE_ADDR_SPACE (v) = TYPE_ADDR_SPACE (t);
> +  TYPE_NAME (v) = TYPE_NAME (t);
> +  TYPE_ATTRIBUTES (v) = TYPE_ATTRIBUTES (t);
> +  return v;
> +}
> +
> +/* Map complete types to incomplete types.  */
> +
> +static hash_map<tree, tree> *fld_incomplete_types;
> +
> +/* For T being aggregate type try to turn it into a incomplete variant.
> +   Return T if no simplification is possible.  */
> +
> +static tree
> +fld_incomplete_type_of (tree t)
> +{
> +  if (!t)
> +    return NULL;
> +  if (POINTER_TYPE_P (t))
> +    {
> +      tree t2 = fld_incomplete_type_of (TREE_TYPE (t));
> +      if (t2 != TREE_TYPE (t))
> +	{
> +	  tree first;
> +	  if (TREE_CODE (t) == POINTER_TYPE)
> +	    first = build_pointer_type_for_mode (t2, TYPE_MODE (t),
> +						TYPE_REF_CAN_ALIAS_ALL (t));
> +	  else
> +	    first = build_reference_type_for_mode (t2, TYPE_MODE (t),
> +						TYPE_REF_CAN_ALIAS_ALL (t));
> +	  return fld_type_variant (first, t);
> +	}
> +      return t;
> +    }
> +  if (!RECORD_OR_UNION_TYPE_P (t) || !COMPLETE_TYPE_P (t))
> +    return t;
> +  if (TYPE_MAIN_VARIANT (t) == t)
> +    {
> +      bool existed;
> +      tree &copy
> +	 = fld_incomplete_types->get_or_insert (t, &existed);
> +
> +      if (!existed)
> +	{
> +	  copy = build_distinct_type_copy (t);
> +
> +	  /* It is possible type was not seen by free_lang_data yet.  */
> +	  free_lang_data_in_type (copy);
> +	  TYPE_SIZE (copy) = NULL;
> +	  SET_TYPE_MODE (copy, VOIDmode);
> +	  SET_TYPE_ALIGN (copy, BITS_PER_UNIT);
> +	  TYPE_SIZE_UNIT (copy) = NULL;
> +	  if (AGGREGATE_TYPE_P (t))
> +	    {
> +	      TYPE_FIELDS (copy) = NULL;
> +	      TYPE_BINFO (copy) = NULL;
> +	    }
> +	  else
> +	    TYPE_VALUES (copy) = NULL;
> +	}
> +      return copy;
> +   }
> +  return (fld_type_variant
> +	    (fld_incomplete_type_of (TYPE_MAIN_VARIANT (t)), t));
> +}
> +
> +/* Simplify type T for scenarios where we do not need complete pointer
> +   types.  */
> +
> +static tree
> +fld_simplified_type (tree t)
> +{
> +  if (t && POINTER_TYPE_P (t))
> +    return fld_incomplete_type_of (t);
> +  return t;
> +}
> +
>  /* Reset the expression *EXPR_P, a size or position.
>  
>     ??? We could reset all non-constant sizes or positions.  But it's cheap
> @@ -5354,9 +5470,12 @@
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
>        DECL_INITIAL (decl) = NULL_TREE;
>        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
>      }
>    else if (TREE_CODE (decl) == FIELD_DECL)
> -    DECL_INITIAL (decl) = NULL_TREE;
> +    {
> +      TREE_TYPE (decl) = fld_simplified_type (TREE_TYPE (decl));
> +      DECL_INITIAL (decl) = NULL_TREE;
> +    }
>    else if (TREE_CODE (decl) == TRANSLATION_UNIT_DECL
>             && DECL_INITIAL (decl)
>             && TREE_CODE (DECL_INITIAL (decl)) == BLOCK)
> @@ -5866,6 +5989,8 @@
>        || (!flag_generate_lto && !flag_generate_offload))
>      return 0;
>  
> +  fld_incomplete_types = new hash_map<tree, tree>;
> +
>    /* Provide a dummy TRANSLATION_UNIT_DECL if the FE failed to provide one.  */
>    if (vec_safe_is_empty (all_translation_units))
>      build_translation_unit_decl (NULL_TREE);
> 
>

Hi,
this is variant I re-tested and comitted.  It goes by adding the type to worklist - 
it is very hard to debug ordering issues in free lang data so I think it is more robust
to avoid introducing more surprises and we will definitly want to also simplify function
types eventually.  I am now re-benchmarking builds and will also try firefox/libreoffice
without unified build that stresses merging a lot more.

lto-boottrapped/regtested x86_64-linux.

Honza

	* tree.c
	(free_lang_data_d, add_tree_to_fld_list, fld_worklist_push): Move
	head in file.
	(free_lang_data_in_type): Forward declare.
	(fld_type_variant_equal_p): New function.
	(fld_type_variant): New function
	(fld_incomplete_types): New hash.
	(fld_incomplete_type_of): New function
	(fld_simplfied-type): New function.
	(free_lang_data_in_decl): Add fld parameter; simplify type of FIELD_DECL
	(free_lang_data): Allocate and free fld_incomplete_type; update call
	of free_lang_data_in_decl.

Index: tree.c
===================================================================
--- tree.c	(revision 265573)
+++ tree.c	(working copy)
@@ -5037,7 +5037,163 @@ protected_set_expr_location (tree t, loc
   if (CAN_HAVE_LOCATION_P (t))
     SET_EXPR_LOCATION (t, loc);
 }
+
+/* Data used when collecting DECLs and TYPEs for language data removal.  */
+
+struct free_lang_data_d
+{
+  free_lang_data_d () : decls (100), types (100) {}
+
+  /* Worklist to avoid excessive recursion.  */
+  auto_vec<tree> worklist;
+
+  /* Set of traversed objects.  Used to avoid duplicate visits.  */
+  hash_set<tree> pset;
+
+  /* Array of symbols to process with free_lang_data_in_decl.  */
+  auto_vec<tree> decls;
+
+  /* Array of types to process with free_lang_data_in_type.  */
+  auto_vec<tree> types;
+};
+
+
+/* Add type or decl T to one of the list of tree nodes that need their
+   language data removed.  The lists are held inside FLD.  */
+
+static void
+add_tree_to_fld_list (tree t, struct free_lang_data_d *fld)
+{
+  if (DECL_P (t))
+    fld->decls.safe_push (t);
+  else if (TYPE_P (t))
+    fld->types.safe_push (t);
+  else
+    gcc_unreachable ();
+}
+
+/* Push tree node T into FLD->WORKLIST.  */
+
+static inline void
+fld_worklist_push (tree t, struct free_lang_data_d *fld)
+{
+  if (t && !is_lang_specific (t) && !fld->pset.contains (t))
+    fld->worklist.safe_push ((t));
+}
+
+
 
+/* Do same comparsion as check_qualified_type skipping lang part of type
+   and be more permissive about type names: we only care that names are
+   same (for diagnostics) and that ODR names are the same.  */
+
+static bool
+fld_type_variant_equal_p (tree t, tree v)
+{
+  if (TYPE_QUALS (t) != TYPE_QUALS (v)
+      || TYPE_NAME (t) != TYPE_NAME (v)
+      || TYPE_ALIGN (t) != TYPE_ALIGN (v)
+      || !attribute_list_equal (TYPE_ATTRIBUTES (t),
+			        TYPE_ATTRIBUTES (v)))
+    return false;
+
+  return true;
+}
+
+/* Find variant of FIRST that match T and create new one if necessary.  */
+
+static tree
+fld_type_variant (tree first, tree t, struct free_lang_data_d *fld)
+{
+  if (first == TYPE_MAIN_VARIANT (t))
+    return t;
+  for (tree v = first; v; v = TYPE_NEXT_VARIANT (v))
+    if (fld_type_variant_equal_p (t, v))
+      return v;
+  tree v = build_variant_type_copy (first);
+  TYPE_READONLY (v) = TYPE_READONLY (t);
+  TYPE_VOLATILE (v) = TYPE_VOLATILE (t);
+  TYPE_ATOMIC (v) = TYPE_ATOMIC (t);
+  TYPE_RESTRICT (v) = TYPE_RESTRICT (t);
+  TYPE_ADDR_SPACE (v) = TYPE_ADDR_SPACE (t);
+  TYPE_NAME (v) = TYPE_NAME (t);
+  TYPE_ATTRIBUTES (v) = TYPE_ATTRIBUTES (t);
+  add_tree_to_fld_list (v, fld);
+  return v;
+}
+
+/* Map complete types to incomplete types.  */
+
+static hash_map<tree, tree> *fld_incomplete_types;
+
+/* For T being aggregate type try to turn it into a incomplete variant.
+   Return T if no simplification is possible.  */
+
+static tree
+fld_incomplete_type_of (tree t, struct free_lang_data_d *fld)
+{
+  if (!t)
+    return NULL;
+  if (POINTER_TYPE_P (t))
+    {
+      tree t2 = fld_incomplete_type_of (TREE_TYPE (t), fld);
+      if (t2 != TREE_TYPE (t))
+	{
+	  tree first;
+	  if (TREE_CODE (t) == POINTER_TYPE)
+	    first = build_pointer_type_for_mode (t2, TYPE_MODE (t),
+						TYPE_REF_CAN_ALIAS_ALL (t));
+	  else
+	    first = build_reference_type_for_mode (t2, TYPE_MODE (t),
+						TYPE_REF_CAN_ALIAS_ALL (t));
+	  add_tree_to_fld_list (first, fld);
+	  return fld_type_variant (first, t, fld);
+	}
+      return t;
+    }
+  if (!RECORD_OR_UNION_TYPE_P (t) || !COMPLETE_TYPE_P (t))
+    return t;
+  if (TYPE_MAIN_VARIANT (t) == t)
+    {
+      bool existed;
+      tree &copy
+	 = fld_incomplete_types->get_or_insert (t, &existed);
+
+      if (!existed)
+	{
+	  copy = build_distinct_type_copy (t);
+
+	  /* It is possible type was not seen by free_lang_data yet.  */
+	  add_tree_to_fld_list (copy, fld);
+	  TYPE_SIZE (copy) = NULL;
+	  SET_TYPE_MODE (copy, VOIDmode);
+	  SET_TYPE_ALIGN (copy, BITS_PER_UNIT);
+	  TYPE_SIZE_UNIT (copy) = NULL;
+	  if (AGGREGATE_TYPE_P (t))
+	    {
+	      TYPE_FIELDS (copy) = NULL;
+	      TYPE_BINFO (copy) = NULL;
+	    }
+	  else
+	    TYPE_VALUES (copy) = NULL;
+	}
+      return copy;
+   }
+  return (fld_type_variant
+	    (fld_incomplete_type_of (TYPE_MAIN_VARIANT (t), fld), t, fld));
+}
+
+/* Simplify type T for scenarios where we do not need complete pointer
+   types.  */
+
+static tree
+fld_simplified_type (tree t, struct free_lang_data_d *fld)
+{
+  if (t && POINTER_TYPE_P (t))
+    return fld_incomplete_type_of (t, fld);
+  return t;
+}
+
 /* Reset the expression *EXPR_P, a size or position.
 
    ??? We could reset all non-constant sizes or positions.  But it's cheap
@@ -5261,7 +5417,7 @@ need_assembler_name_p (tree decl)
    DECL.  */
 
 static void
-free_lang_data_in_decl (tree decl)
+free_lang_data_in_decl (tree decl, struct free_lang_data_d *fld)
 {
   gcc_assert (DECL_P (decl));
 
@@ -5356,7 +5512,10 @@ free_lang_data_in_decl (tree decl)
       DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
     }
   else if (TREE_CODE (decl) == FIELD_DECL)
-    DECL_INITIAL (decl) = NULL_TREE;
+    {
+      TREE_TYPE (decl) = fld_simplified_type (TREE_TYPE (decl), fld);
+      DECL_INITIAL (decl) = NULL_TREE;
+    }
   else if (TREE_CODE (decl) == TRANSLATION_UNIT_DECL
            && DECL_INITIAL (decl)
            && TREE_CODE (DECL_INITIAL (decl)) == BLOCK)
@@ -5400,50 +5559,6 @@ free_lang_data_in_decl (tree decl)
 }
 
 
-/* Data used when collecting DECLs and TYPEs for language data removal.  */
-
-struct free_lang_data_d
-{
-  free_lang_data_d () : decls (100), types (100) {}
-
-  /* Worklist to avoid excessive recursion.  */
-  auto_vec<tree> worklist;
-
-  /* Set of traversed objects.  Used to avoid duplicate visits.  */
-  hash_set<tree> pset;
-
-  /* Array of symbols to process with free_lang_data_in_decl.  */
-  auto_vec<tree> decls;
-
-  /* Array of types to process with free_lang_data_in_type.  */
-  auto_vec<tree> types;
-};
-
-
-/* Add type or decl T to one of the list of tree nodes that need their
-   language data removed.  The lists are held inside FLD.  */
-
-static void
-add_tree_to_fld_list (tree t, struct free_lang_data_d *fld)
-{
-  if (DECL_P (t))
-    fld->decls.safe_push (t);
-  else if (TYPE_P (t))
-    fld->types.safe_push (t);
-  else
-    gcc_unreachable ();
-}
-
-/* Push tree node T into FLD->WORKLIST.  */
-
-static inline void
-fld_worklist_push (tree t, struct free_lang_data_d *fld)
-{
-  if (t && !is_lang_specific (t) && !fld->pset.contains (t))
-    fld->worklist.safe_push ((t));
-}
-
-
 /* Operand callback helper for free_lang_data_in_node.  *TP is the
    subtree operand being considered.  */
 
@@ -5841,7 +5956,7 @@ free_lang_data_in_cgraph (void)
 
   /* Traverse every decl found freeing its language data.  */
   FOR_EACH_VEC_ELT (fld.decls, i, t)
-    free_lang_data_in_decl (t);
+    free_lang_data_in_decl (t, &fld);
 
   /* Traverse every type found freeing its language data.  */
   FOR_EACH_VEC_ELT (fld.types, i, t)
@@ -5866,6 +5981,8 @@ free_lang_data (void)
       || (!flag_generate_lto && !flag_generate_offload))
     return 0;
 
+  fld_incomplete_types = new hash_map<tree, tree>;
+
   /* Provide a dummy TRANSLATION_UNIT_DECL if the FE failed to provide one.  */
   if (vec_safe_is_empty (all_translation_units))
     build_translation_unit_decl (NULL_TREE);
@@ -5904,6 +6021,8 @@ free_lang_data (void)
 
   rebuild_type_inheritance_graph ();
 
+  delete fld_incomplete_types;
+
   return 0;
 }