Free TYPE_VALUES of enums

Message ID 20181107123426.s22o3tvd6dtnwedk@kam.mff.cuni.cz
State New
Headers show
Series
  • Free TYPE_VALUES of enums
Related show

Commit Message

Jan Hubicka Nov. 7, 2018, 12:34 p.m.
Hi,
this patch make free_lang_data_in_type to free TYPE_VALUE of enum
unless it is an main variant of ODR type (in that case we use them to
produce ODR warnings).  C++ represents enum values as CONST_DECLs that
are expensive to stream and unused, so I also updated free_lang_data to
replace them by C representation that only uses integer constants.

This needs little change to type verifier.

In addition to that ipa-devirt now frees the values after ODR warnings.
This reduces ltrans files from 450 to 370MB.
I have also updated duplicate printing to be more pareseable.

Bootstrapped/regtested x86_64-linux, will commit it after
lto-bootstrapping uneless there are complains.

Honza

	* ipa-devirt.c (odr_types_equivalent_p): Expect constants
	than const decls in TREE_VALUE of enum.
	(dump_type_inheritance_graph): Improve duplicate dumping.
	(free_enum_values): New.
	(build_type_inheritance_graph): Use it.
	* tree.c (free_lang_data_in_type): Free TYPE_VALUES of enums
	which are not main variants or not ODR types.
	(verify_type_variant): Expect variants to have no TYPE_VALUES.

Comments

Richard Biener Nov. 7, 2018, 1:09 p.m. | #1
On Wed, Nov 7, 2018 at 1:34 PM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> Hi,
> this patch make free_lang_data_in_type to free TYPE_VALUE of enum
> unless it is an main variant of ODR type (in that case we use them to
> produce ODR warnings).  C++ represents enum values as CONST_DECLs that
> are expensive to stream and unused, so I also updated free_lang_data to
> replace them by C representation that only uses integer constants.
>
> This needs little change to type verifier.
>
> In addition to that ipa-devirt now frees the values after ODR warnings.
> This reduces ltrans files from 450 to 370MB.
> I have also updated duplicate printing to be more pareseable.
>
> Bootstrapped/regtested x86_64-linux, will commit it after
> lto-bootstrapping uneless there are complains.

LGTM

> Honza
>
>         * ipa-devirt.c (odr_types_equivalent_p): Expect constants
>         than const decls in TREE_VALUE of enum.
>         (dump_type_inheritance_graph): Improve duplicate dumping.
>         (free_enum_values): New.
>         (build_type_inheritance_graph): Use it.
>         * tree.c (free_lang_data_in_type): Free TYPE_VALUES of enums
>         which are not main variants or not ODR types.
>         (verify_type_variant): Expect variants to have no TYPE_VALUES.
> Index: ipa-devirt.c
> ===================================================================
> --- ipa-devirt.c        (revision 265872)
> +++ ipa-devirt.c        (working copy)
> @@ -1328,9 +1328,7 @@ odr_types_equivalent_p (tree t1, tree t2
>                            " is defined in another translation unit"));
>               return false;
>             }
> -         if (TREE_VALUE (v1) != TREE_VALUE (v2)
> -             && !operand_equal_p (DECL_INITIAL (TREE_VALUE (v1)),
> -                                  DECL_INITIAL (TREE_VALUE (v2)), 0))
> +         if (TREE_VALUE (v1) != TREE_VALUE (v2))
>             {
>               warn_odr (t1, t2, NULL, NULL, warn, warned,
>                         G_("an enum with different values is defined"
> @@ -2191,6 +2189,7 @@ static void
>  dump_type_inheritance_graph (FILE *f)
>  {
>    unsigned int i;
> +  unsigned int num_all_types = 0, num_types = 0, num_duplicates = 0;
>    if (!odr_types_ptr)
>      return;
>    fprintf (f, "\n\nType inheritance graph:\n");
> @@ -2201,26 +2200,70 @@ dump_type_inheritance_graph (FILE *f)
>      }
>    for (i = 0; i < odr_types.length (); i++)
>      {
> -      if (odr_types[i] && odr_types[i]->types && odr_types[i]->types->length ())
> -       {
> -         unsigned int j;
> -         fprintf (f, "Duplicate tree types for odr type %i\n", i);
> -         print_node (f, "", odr_types[i]->type, 0);
> -         for (j = 0; j < odr_types[i]->types->length (); j++)
> -           {
> -             tree t;
> -             fprintf (f, "duplicate #%i\n", j);
> -             print_node (f, "", (*odr_types[i]->types)[j], 0);
> -             t = (*odr_types[i]->types)[j];
> -             while (TYPE_P (t) && TYPE_CONTEXT (t))
> -               {
> -                 t = TYPE_CONTEXT (t);
> -                 print_node (f, "", t, 0);
> -               }
> -             putc ('\n',f);
> +      if (!odr_types[i])
> +       continue;
> +
> +      num_all_types++;
> +      if (!odr_types[i]->types || !odr_types[i]->types->length ())
> +       continue;
> +
> +      /* To aid ODR warnings we also mangle integer constants but do
> +        not consinder duplicates there.  */
> +      if (TREE_CODE (odr_types[i]->type) == INTEGER_TYPE)
> +       continue;
> +
> +      /* It is normal to have one duplicate and one normal variant.  */
> +      if (odr_types[i]->types->length () == 1
> +         && COMPLETE_TYPE_P (odr_types[i]->type)
> +         && !COMPLETE_TYPE_P ((*odr_types[i]->types)[0]))
> +       continue;
> +
> +      num_types ++;
> +
> +      unsigned int j;
> +      fprintf (f, "Duplicate tree types for odr type %i\n", i);
> +      print_node (f, "", odr_types[i]->type, 0);
> +      print_node (f, "", TYPE_NAME (odr_types[i]->type), 0);
> +      putc ('\n',f);
> +      for (j = 0; j < odr_types[i]->types->length (); j++)
> +       {
> +         tree t;
> +         num_duplicates ++;
> +         fprintf (f, "duplicate #%i\n", j);
> +         print_node (f, "", (*odr_types[i]->types)[j], 0);
> +         t = (*odr_types[i]->types)[j];
> +         while (TYPE_P (t) && TYPE_CONTEXT (t))
> +           {
> +             t = TYPE_CONTEXT (t);
> +             print_node (f, "", t, 0);
>             }
> +         print_node (f, "", TYPE_NAME ((*odr_types[i]->types)[j]), 0);
> +         putc ('\n',f);
>         }
>      }
> +  fprintf (f, "Out of %i types there are %i types with duplicates; "
> +          "%i duplicates overall\n", num_all_types, num_types, num_duplicates);
> +}
> +
> +/* Save some WPA->ltrans streaming by freeing enum values.  */
> +
> +static void
> +free_enum_values ()
> +{
> +  static bool enum_values_freed = false;
> +  if (enum_values_freed || !flag_wpa || !odr_types_ptr)
> +    return;
> +  enum_values_freed = true;
> +  unsigned int i;
> +  for (i = 0; i < odr_types.length (); i++)
> +    if (odr_types[i] && TREE_CODE (odr_types[i]->type) == ENUMERAL_TYPE)
> +      {
> +       TYPE_VALUES (odr_types[i]->type) = NULL;
> +       if (odr_types[i]->types)
> +          for (unsigned int j = 0; j < odr_types[i]->types->length (); j++)
> +           TYPE_VALUES ((*odr_types[i]->types)[j]) = NULL;
> +      }
> +  enum_values_freed = true;
>  }
>
>  /* Initialize IPA devirt and build inheritance tree graph.  */
> @@ -2233,7 +2276,10 @@ build_type_inheritance_graph (void)
>    dump_flags_t flags;
>
>    if (odr_hash)
> -    return;
> +    {
> +      free_enum_values ();
> +      return;
> +    }
>    timevar_push (TV_IPA_INHERITANCE);
>    inheritance_dump_file = dump_begin (TDI_inheritance, &flags);
>    odr_hash = new odr_hash_type (23);
> @@ -2278,6 +2324,7 @@ build_type_inheritance_graph (void)
>        dump_type_inheritance_graph (inheritance_dump_file);
>        dump_end (TDI_inheritance, inheritance_dump_file);
>      }
> +  free_enum_values ();
>    timevar_pop (TV_IPA_INHERITANCE);
>  }
>
> Index: tree.c
> ===================================================================
> --- tree.c      (revision 265872)
> +++ tree.c      (working copy)
> @@ -5345,6 +5348,20 @@ free_lang_data_in_type (tree type, struc
>            || SCALAR_FLOAT_TYPE_P (type)
>            || FIXED_POINT_TYPE_P (type))
>      {
> +      if (TREE_CODE (type) == ENUMERAL_TYPE)
> +       {
> +         /* Type values are used only for C++ ODR checking.  Drop them
> +            for all type variants and non-ODR types.  */
> +         if (TYPE_MAIN_VARIANT (type) != type
> +             || !type_with_linkage_p (type))
> +           TYPE_VALUES (type) = NULL;
> +         else
> +         /* Simplify representation by recording only values rather
> +            than const decls.  */
> +           for (tree e = TYPE_VALUES (type); e; e = TREE_CHAIN (e))
> +             if (TREE_CODE (TREE_VALUE (e)) == CONST_DECL)
> +               TREE_VALUE (e) = DECL_INITIAL (TREE_VALUE (e));
> +       }
>        free_lang_data_in_one_sizepos (&TYPE_MIN_VALUE (type));
>        free_lang_data_in_one_sizepos (&TYPE_MAX_VALUE (type));
>      }
> @@ -13525,7 +13556,8 @@ verify_type_variant (const_tree t, tree
>      }
>
>    /* Check various uses of TYPE_VALUES_RAW.  */
> -  if (TREE_CODE (t) == ENUMERAL_TYPE)
> +  if (TREE_CODE (t) == ENUMERAL_TYPE
> +      && TYPE_VALUES (t))
>      verify_variant_match (TYPE_VALUES);
>    else if (TREE_CODE (t) == ARRAY_TYPE)
>      verify_variant_match (TYPE_DOMAIN);
Bernhard Reutner-Fischer Nov. 7, 2018, 7:34 p.m. | #2
On Wed, 7 Nov 2018 14:09:24 +0100
Richard Biener <richard.guenther@gmail.com> wrote:

> On Wed, Nov 7, 2018 at 1:34 PM Jan Hubicka <hubicka@ucw.cz> wrote:

> > Bootstrapped/regtested x86_64-linux, will commit it after
> > lto-bootstrapping uneless there are complains.  

> > +/* Save some WPA->ltrans streaming by freeing enum values.  */
> > +
> > +static void
> > +free_enum_values ()
> > +{
> > +  static bool enum_values_freed = false;
> > +  if (enum_values_freed || !flag_wpa || !odr_types_ptr)
> > +    return;
> > +  enum_values_freed = true;
[]
> > +  enum_values_freed = true;
> >  }

Short of a "verytrue" choice for boolean, i think setting it to true
once should be enough though.

cheers,

Patch

Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 265872)
+++ ipa-devirt.c	(working copy)
@@ -1328,9 +1328,7 @@  odr_types_equivalent_p (tree t1, tree t2
 			   " is defined in another translation unit"));
 	      return false;
 	    }
-	  if (TREE_VALUE (v1) != TREE_VALUE (v2)
-	      && !operand_equal_p (DECL_INITIAL (TREE_VALUE (v1)),
-				   DECL_INITIAL (TREE_VALUE (v2)), 0))
+	  if (TREE_VALUE (v1) != TREE_VALUE (v2))
 	    {
 	      warn_odr (t1, t2, NULL, NULL, warn, warned,
 			G_("an enum with different values is defined"
@@ -2191,6 +2189,7 @@  static void
 dump_type_inheritance_graph (FILE *f)
 {
   unsigned int i;
+  unsigned int num_all_types = 0, num_types = 0, num_duplicates = 0;
   if (!odr_types_ptr)
     return;
   fprintf (f, "\n\nType inheritance graph:\n");
@@ -2201,26 +2200,70 @@  dump_type_inheritance_graph (FILE *f)
     }
   for (i = 0; i < odr_types.length (); i++)
     {
-      if (odr_types[i] && odr_types[i]->types && odr_types[i]->types->length ())
-	{
-	  unsigned int j;
-	  fprintf (f, "Duplicate tree types for odr type %i\n", i);
-	  print_node (f, "", odr_types[i]->type, 0);
-	  for (j = 0; j < odr_types[i]->types->length (); j++)
-	    {
-	      tree t;
-	      fprintf (f, "duplicate #%i\n", j);
-	      print_node (f, "", (*odr_types[i]->types)[j], 0);
-	      t = (*odr_types[i]->types)[j];
-	      while (TYPE_P (t) && TYPE_CONTEXT (t))
-		{
-		  t = TYPE_CONTEXT (t);
-	          print_node (f, "", t, 0);
-		}
-	      putc ('\n',f);
+      if (!odr_types[i])
+	continue;
+
+      num_all_types++;
+      if (!odr_types[i]->types || !odr_types[i]->types->length ())
+	continue;
+
+      /* To aid ODR warnings we also mangle integer constants but do
+	 not consinder duplicates there.  */
+      if (TREE_CODE (odr_types[i]->type) == INTEGER_TYPE)
+	continue;
+
+      /* It is normal to have one duplicate and one normal variant.  */
+      if (odr_types[i]->types->length () == 1
+	  && COMPLETE_TYPE_P (odr_types[i]->type)
+	  && !COMPLETE_TYPE_P ((*odr_types[i]->types)[0]))
+	continue;
+
+      num_types ++;
+
+      unsigned int j;
+      fprintf (f, "Duplicate tree types for odr type %i\n", i);
+      print_node (f, "", odr_types[i]->type, 0);
+      print_node (f, "", TYPE_NAME (odr_types[i]->type), 0);
+      putc ('\n',f);
+      for (j = 0; j < odr_types[i]->types->length (); j++)
+	{
+	  tree t;
+	  num_duplicates ++;
+	  fprintf (f, "duplicate #%i\n", j);
+	  print_node (f, "", (*odr_types[i]->types)[j], 0);
+	  t = (*odr_types[i]->types)[j];
+	  while (TYPE_P (t) && TYPE_CONTEXT (t))
+	    {
+	      t = TYPE_CONTEXT (t);
+	      print_node (f, "", t, 0);
 	    }
+	  print_node (f, "", TYPE_NAME ((*odr_types[i]->types)[j]), 0);
+	  putc ('\n',f);
 	}
     }
+  fprintf (f, "Out of %i types there are %i types with duplicates; "
+	   "%i duplicates overall\n", num_all_types, num_types, num_duplicates);
+}
+
+/* Save some WPA->ltrans streaming by freeing enum values.  */
+
+static void
+free_enum_values ()
+{
+  static bool enum_values_freed = false;
+  if (enum_values_freed || !flag_wpa || !odr_types_ptr)
+    return;
+  enum_values_freed = true;
+  unsigned int i;
+  for (i = 0; i < odr_types.length (); i++)
+    if (odr_types[i] && TREE_CODE (odr_types[i]->type) == ENUMERAL_TYPE)
+      {
+	TYPE_VALUES (odr_types[i]->type) = NULL;
+	if (odr_types[i]->types)
+          for (unsigned int j = 0; j < odr_types[i]->types->length (); j++)
+	    TYPE_VALUES ((*odr_types[i]->types)[j]) = NULL;
+      }
+  enum_values_freed = true;
 }
 
 /* Initialize IPA devirt and build inheritance tree graph.  */
@@ -2233,7 +2276,10 @@  build_type_inheritance_graph (void)
   dump_flags_t flags;
 
   if (odr_hash)
-    return;
+    {
+      free_enum_values ();
+      return;
+    }
   timevar_push (TV_IPA_INHERITANCE);
   inheritance_dump_file = dump_begin (TDI_inheritance, &flags);
   odr_hash = new odr_hash_type (23);
@@ -2278,6 +2324,7 @@  build_type_inheritance_graph (void)
       dump_type_inheritance_graph (inheritance_dump_file);
       dump_end (TDI_inheritance, inheritance_dump_file);
     }
+  free_enum_values ();
   timevar_pop (TV_IPA_INHERITANCE);
 }
 
Index: tree.c
===================================================================
--- tree.c	(revision 265872)
+++ tree.c	(working copy)
@@ -5345,6 +5348,20 @@  free_lang_data_in_type (tree type, struc
 	   || SCALAR_FLOAT_TYPE_P (type)
 	   || FIXED_POINT_TYPE_P (type))
     {
+      if (TREE_CODE (type) == ENUMERAL_TYPE)
+	{
+	  /* Type values are used only for C++ ODR checking.  Drop them
+	     for all type variants and non-ODR types.  */
+	  if (TYPE_MAIN_VARIANT (type) != type
+	      || !type_with_linkage_p (type))
+	    TYPE_VALUES (type) = NULL;
+	  else
+	  /* Simplify representation by recording only values rather
+	     than const decls.  */
+	    for (tree e = TYPE_VALUES (type); e; e = TREE_CHAIN (e))
+	      if (TREE_CODE (TREE_VALUE (e)) == CONST_DECL)
+		TREE_VALUE (e) = DECL_INITIAL (TREE_VALUE (e));
+	}
       free_lang_data_in_one_sizepos (&TYPE_MIN_VALUE (type));
       free_lang_data_in_one_sizepos (&TYPE_MAX_VALUE (type));
     }
@@ -13525,7 +13556,8 @@  verify_type_variant (const_tree t, tree
     }
 
   /* Check various uses of TYPE_VALUES_RAW.  */
-  if (TREE_CODE (t) == ENUMERAL_TYPE)
+  if (TREE_CODE (t) == ENUMERAL_TYPE
+      && TYPE_VALUES (t))
     verify_variant_match (TYPE_VALUES);
   else if (TREE_CODE (t) == ARRAY_TYPE)
     verify_variant_match (TYPE_DOMAIN);