Trivial free-lang-data adjustments for early LTO debug

Message ID alpine.LSU.2.20.1801120924430.32271@zhemvz.fhfr.qr
State New
Headers show
Series
  • Trivial free-lang-data adjustments for early LTO debug
Related show

Commit Message

Richard Biener Jan. 12, 2018, 8:33 a.m.
It's now no longer necessary to preserve type related stuff for debug
reasons so the following forgos with streaming TYPE_DECLs in TYPE_FIELDS,
TYPE_DECLs for TYPE_NAME (where not necessary) and also BINFOs when
not needed for devirtualization.

I noticed the unpatched code

      if (TYPE_BINFO (type))
        {
          free_lang_data_in_binfo (TYPE_BINFO (type));
          /* We need to preserve link to bases and virtual table for all
             polymorphic types to make devirtualization machinery working.
             Debug output cares only about bases, but output also
             virtual table pointers so merging of -fdevirtualize and
             -fno-devirtualize units is easier.  */
          if ((!BINFO_VTABLE (TYPE_BINFO (type))
               || !flag_devirtualize)
              && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type))
                   && !BINFO_VTABLE (TYPE_BINFO (type)))
                  || debug_info_level != DINFO_LEVEL_NONE))
            TYPE_BINFO (type) = NULL;
        }

where the condition

              && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type))
                   && !BINFO_VTABLE (TYPE_BINFO (type)))
                  || debug_info_level != DINFO_LEVEL_NONE))

makes no sense to me -- it implies that when -g is enabled we'll
clear TYPE_BINFO but not with -g0 unless
(!BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) && !BINFO_VTABLE (TYPE_BINFO 
(type)).

I concluded this second condition is basically dead code (and at most
prevented BINFO related debug for LTO), so I dropped it as well as the
comment talking about the merging of -fdevirtualize vs. -fno-devirtualize.
If I'd just remove the debug_info_level condition we'd end up with

          if (!BINFO_VTABLE (TYPE_BINFO (type))
              && !BINFO_N_BASE_BINFOS (TYPE_BINFO (type)))
            TYPE_BINFO (type) = NULL;

which is not -fdevirtualize conditional anymore.

Honza - can you shed light on the original intent?

LTO bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Thanks,
Richard.

2018-01-11  Richard Biener  <rguenther@suse.de>

	* tree.c (free_lang_data_in_type): Always unlink TYPE_DECLs
	from TYPE_FIELDS.  Free TYPE_BINFO if not used by devirtualization.
	Reset type names to their identifier if their TYPE_DECL doesn't
	have linkage (and thus is used for ODR and devirt).
	(save_debug_info_for_decl): Remove.
	(save_debug_info_for_type): Likewise.
	(add_tree_to_fld_list): Adjust.

Comments

Jan Hubicka Jan. 12, 2018, 8:52 a.m. | #1
> 
> It's now no longer necessary to preserve type related stuff for debug
> reasons so the following forgos with streaming TYPE_DECLs in TYPE_FIELDS,
> TYPE_DECLs for TYPE_NAME (where not necessary) and also BINFOs when
> not needed for devirtualization.
> 
> I noticed the unpatched code
> 
>       if (TYPE_BINFO (type))
>         {
>           free_lang_data_in_binfo (TYPE_BINFO (type));
>           /* We need to preserve link to bases and virtual table for all
>              polymorphic types to make devirtualization machinery working.
>              Debug output cares only about bases, but output also
>              virtual table pointers so merging of -fdevirtualize and
>              -fno-devirtualize units is easier.  */
>           if ((!BINFO_VTABLE (TYPE_BINFO (type))
>                || !flag_devirtualize)
>               && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type))
>                    && !BINFO_VTABLE (TYPE_BINFO (type)))
>                   || debug_info_level != DINFO_LEVEL_NONE))
>             TYPE_BINFO (type) = NULL;
>         }
> 
> where the condition
> 
>               && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type))
>                    && !BINFO_VTABLE (TYPE_BINFO (type)))
>                   || debug_info_level != DINFO_LEVEL_NONE))
> 
> makes no sense to me -- it implies that when -g is enabled we'll
> clear TYPE_BINFO but not with -g0 unless
> (!BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) && !BINFO_VTABLE (TYPE_BINFO 
> (type)).

The first part of conditional is testing when flag_devirtualize may need binfo
(when there is vtable). Second part is about when late dwarf2out was
accessing binfos (when there was bases) but indeed debug_info_level test
seems to be backwards.

I think the change is OK.
Honza
> 
> I concluded this second condition is basically dead code (and at most
> prevented BINFO related debug for LTO), so I dropped it as well as the
> comment talking about the merging of -fdevirtualize vs. -fno-devirtualize.
> If I'd just remove the debug_info_level condition we'd end up with
> 
>           if (!BINFO_VTABLE (TYPE_BINFO (type))
>               && !BINFO_N_BASE_BINFOS (TYPE_BINFO (type)))
>             TYPE_BINFO (type) = NULL;
> 
> which is not -fdevirtualize conditional anymore.
> 
> Honza - can you shed light on the original intent?
> 
> LTO bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Thanks,
> Richard.
> 
> 2018-01-11  Richard Biener  <rguenther@suse.de>
> 
> 	* tree.c (free_lang_data_in_type): Always unlink TYPE_DECLs
> 	from TYPE_FIELDS.  Free TYPE_BINFO if not used by devirtualization.
> 	Reset type names to their identifier if their TYPE_DECL doesn't
> 	have linkage (and thus is used for ODR and devirt).
> 	(save_debug_info_for_decl): Remove.
> 	(save_debug_info_for_type): Likewise.
> 	(add_tree_to_fld_list): Adjust.
> 
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c	(revision 256562)
> +++ gcc/tree.c	(working copy)
> @@ -5127,15 +5127,10 @@ free_lang_data_in_type (tree type)
>        TREE_PURPOSE (p) = NULL;
>    else if (RECORD_OR_UNION_TYPE_P (type))
>      {
> -      /* Remove members that are not FIELD_DECLs (and maybe
> -	 TYPE_DECLs) from the field list of an aggregate.  These occur
> -	 in C++.  */
> +      /* Remove members that are not FIELD_DECLs from the field list
> +	 of an aggregate.  These occur in C++.  */
>        for (tree *prev = &TYPE_FIELDS (type), member; (member = *prev);)
> -	if (TREE_CODE (member) == FIELD_DECL
> -	    || (TREE_CODE (member) == TYPE_DECL
> -		&& !DECL_IGNORED_P (member)
> -		&& debug_info_level > DINFO_LEVEL_TERSE
> -		&& !is_redundant_typedef (member)))
> +	if (TREE_CODE (member) == FIELD_DECL)
>  	  prev = &DECL_CHAIN (member);
>  	else
>  	  *prev = DECL_CHAIN (member);
> @@ -5149,15 +5144,9 @@ free_lang_data_in_type (tree type)
>  	{
>  	  free_lang_data_in_binfo (TYPE_BINFO (type));
>  	  /* We need to preserve link to bases and virtual table for all
> -	     polymorphic types to make devirtualization machinery working.
> -	     Debug output cares only about bases, but output also
> -	     virtual table pointers so merging of -fdevirtualize and
> -	     -fno-devirtualize units is easier.  */
> -	  if ((!BINFO_VTABLE (TYPE_BINFO (type))
> -	       || !flag_devirtualize)
> -	      && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type))
> -		   && !BINFO_VTABLE (TYPE_BINFO (type)))
> -		  || debug_info_level != DINFO_LEVEL_NONE))
> +	     polymorphic types to make devirtualization machinery working.  */
> +	  if (!BINFO_VTABLE (TYPE_BINFO (type))
> +	      || !flag_devirtualize)
>  	    TYPE_BINFO (type) = NULL;
>  	}
>      }
> @@ -5185,6 +5174,11 @@ free_lang_data_in_type (tree type)
>        while (ctx && TREE_CODE (ctx) == BLOCK);
>        TYPE_CONTEXT (type) = ctx;
>      }
> +
> +  /* 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))
> +    TYPE_NAME (type) = TYPE_IDENTIFIER (type);
>  }
>  
>  
> @@ -5407,34 +5401,6 @@ struct free_lang_data_d
>  };
>  
>  
> -/* Save all language fields needed to generate proper debug information
> -   for DECL.  This saves most fields cleared out by free_lang_data_in_decl.  */
> -
> -static void
> -save_debug_info_for_decl (tree t)
> -{
> -  /*struct saved_debug_info_d *sdi;*/
> -
> -  gcc_assert (debug_info_level > DINFO_LEVEL_TERSE && t && DECL_P (t));
> -
> -  /* FIXME.  Partial implementation for saving debug info removed.  */
> -}
> -
> -
> -/* Save all language fields needed to generate proper debug information
> -   for TYPE.  This saves most fields cleared out by free_lang_data_in_type.  */
> -
> -static void
> -save_debug_info_for_type (tree t)
> -{
> -  /*struct saved_debug_info_d *sdi;*/
> -
> -  gcc_assert (debug_info_level > DINFO_LEVEL_TERSE && t && TYPE_P (t));
> -
> -  /* FIXME.  Partial implementation for saving debug info removed.  */
> -}
> -
> -
>  /* 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.  */
>  
> @@ -5442,17 +5408,9 @@ static void
>  add_tree_to_fld_list (tree t, struct free_lang_data_d *fld)
>  {
>    if (DECL_P (t))
> -    {
> -      fld->decls.safe_push (t);
> -      if (debug_info_level > DINFO_LEVEL_TERSE)
> -	save_debug_info_for_decl (t);
> -    }
> +    fld->decls.safe_push (t);
>    else if (TYPE_P (t))
> -    {
> -      fld->types.safe_push (t);
> -      if (debug_info_level > DINFO_LEVEL_TERSE)
> -	save_debug_info_for_type (t);
> -    }
> +    fld->types.safe_push (t);
>    else
>      gcc_unreachable ();
>  }

Patch

Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 256562)
+++ gcc/tree.c	(working copy)
@@ -5127,15 +5127,10 @@  free_lang_data_in_type (tree type)
       TREE_PURPOSE (p) = NULL;
   else if (RECORD_OR_UNION_TYPE_P (type))
     {
-      /* Remove members that are not FIELD_DECLs (and maybe
-	 TYPE_DECLs) from the field list of an aggregate.  These occur
-	 in C++.  */
+      /* Remove members that are not FIELD_DECLs from the field list
+	 of an aggregate.  These occur in C++.  */
       for (tree *prev = &TYPE_FIELDS (type), member; (member = *prev);)
-	if (TREE_CODE (member) == FIELD_DECL
-	    || (TREE_CODE (member) == TYPE_DECL
-		&& !DECL_IGNORED_P (member)
-		&& debug_info_level > DINFO_LEVEL_TERSE
-		&& !is_redundant_typedef (member)))
+	if (TREE_CODE (member) == FIELD_DECL)
 	  prev = &DECL_CHAIN (member);
 	else
 	  *prev = DECL_CHAIN (member);
@@ -5149,15 +5144,9 @@  free_lang_data_in_type (tree type)
 	{
 	  free_lang_data_in_binfo (TYPE_BINFO (type));
 	  /* We need to preserve link to bases and virtual table for all
-	     polymorphic types to make devirtualization machinery working.
-	     Debug output cares only about bases, but output also
-	     virtual table pointers so merging of -fdevirtualize and
-	     -fno-devirtualize units is easier.  */
-	  if ((!BINFO_VTABLE (TYPE_BINFO (type))
-	       || !flag_devirtualize)
-	      && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type))
-		   && !BINFO_VTABLE (TYPE_BINFO (type)))
-		  || debug_info_level != DINFO_LEVEL_NONE))
+	     polymorphic types to make devirtualization machinery working.  */
+	  if (!BINFO_VTABLE (TYPE_BINFO (type))
+	      || !flag_devirtualize)
 	    TYPE_BINFO (type) = NULL;
 	}
     }
@@ -5185,6 +5174,11 @@  free_lang_data_in_type (tree type)
       while (ctx && TREE_CODE (ctx) == BLOCK);
       TYPE_CONTEXT (type) = ctx;
     }
+
+  /* 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))
+    TYPE_NAME (type) = TYPE_IDENTIFIER (type);
 }
 
 
@@ -5407,34 +5401,6 @@  struct free_lang_data_d
 };
 
 
-/* Save all language fields needed to generate proper debug information
-   for DECL.  This saves most fields cleared out by free_lang_data_in_decl.  */
-
-static void
-save_debug_info_for_decl (tree t)
-{
-  /*struct saved_debug_info_d *sdi;*/
-
-  gcc_assert (debug_info_level > DINFO_LEVEL_TERSE && t && DECL_P (t));
-
-  /* FIXME.  Partial implementation for saving debug info removed.  */
-}
-
-
-/* Save all language fields needed to generate proper debug information
-   for TYPE.  This saves most fields cleared out by free_lang_data_in_type.  */
-
-static void
-save_debug_info_for_type (tree t)
-{
-  /*struct saved_debug_info_d *sdi;*/
-
-  gcc_assert (debug_info_level > DINFO_LEVEL_TERSE && t && TYPE_P (t));
-
-  /* FIXME.  Partial implementation for saving debug info removed.  */
-}
-
-
 /* 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.  */
 
@@ -5442,17 +5408,9 @@  static void
 add_tree_to_fld_list (tree t, struct free_lang_data_d *fld)
 {
   if (DECL_P (t))
-    {
-      fld->decls.safe_push (t);
-      if (debug_info_level > DINFO_LEVEL_TERSE)
-	save_debug_info_for_decl (t);
-    }
+    fld->decls.safe_push (t);
   else if (TYPE_P (t))
-    {
-      fld->types.safe_push (t);
-      if (debug_info_level > DINFO_LEVEL_TERSE)
-	save_debug_info_for_type (t);
-    }
+    fld->types.safe_push (t);
   else
     gcc_unreachable ();
 }