Re: Stream TREE_TYPE of TYPE_DECLs again

[Richard Biener <rguenther at suse dot de>]

On Wed, 21 Nov 2018, Jan Hubicka wrote:

> Hi,
> this patch recovers location infomration in the ODR warnings.
> Because location info is not attached to types but corresponding
> TYPE_DECLs, we need to prevent TYPE_DECLs to be merged when
> corresponding types are not merged.
> 
> To achieve this I no longer clear TREE_TYPE of TYPE_DECLs which
> puts them back to the same SCC as the type itself.  While making
> incomplete type variant we need to produce copy of TYPE_DECL. Becuase
> it is possible that TYPE_DECL was not processed by free lang data
> we can not do copy_node but build it from scratch (because 
> the toplevel loops possibly processed all decls). This is not hard
> to do, but made me notice few extra flags that are streamed for
> TYPE_DECLs and free_lang_data is not seeing them.
> 
> I have also extended ipa-devirt to get rid of the duplicated decls
> once ODR warnings are done to save ltrans streaming (it actually
> added about 10% of ltrans data w/o this change)
> 
> I have checked that the patch does not increase number of type
> duplicates for cc1 (24), I will also re-do testing for Firefox
> which may uncover some extra flags/attributes to care about.
> 
> lto-bootstrapped/regtested x86_64-linux OK?

OK if you put a comment ...

> Honza
> 	
> 	PR lto/87957
> 	* tree.c (fld_decl_context): Break out from ...
> 	(free_lang_data_in_decl): ... here; free TREE_PUBLIC, TREE_PRIVATE
> 	DECL_ARTIFICIAL of TYPE_DECL; do not free TREE_TYPE of TYPE_DECL.
> 	(fld_incomplete_type_of): Build copy of TYP_DECL.
> 	* ipa-devirt.c (free_enum_values): Rename to ...
> 	(free_odr_warning_data): ... this one; free also duplicated TYPE_DECLs
> 	and TREE_TYPEs of TYPE_DECLs.
> 
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 266325)
> +++ tree.c	(working copy)
> @@ -5206,6 +5206,24 @@ fld_process_array_type (tree t, tree t2,
>    return array;
>  }
>  
> +/* Return CTX after removal of contexts that are not relevant  */
> +
> +static tree
> +fld_decl_context (tree ctx)
> +{
> +  /* Variably modified types are needed for tree_is_indexable to decide
> +     whether the type needs to go to local or global section.
> +     This code is semi-broken but for now it is easiest to keep contexts
> +     as expected.  */
> +  if (ctx && TYPE_P (ctx)
> +      && !variably_modified_type_p (ctx, NULL_TREE))
> +     {
> +       while (ctx && TYPE_P (ctx))
> +	 ctx = TYPE_CONTEXT (ctx);
> +     }
> +  return ctx;
> +}
> +
>  /* For T being aggregate type try to turn it into a incomplete variant.
>     Return T if no simplification is possible.  */
>  
> @@ -5267,6 +5285,27 @@ fld_incomplete_type_of (tree t, struct f
>  	    }
>  	  else
>  	    TYPE_VALUES (copy) = NULL;
> +
> +	  /* Build copy of TYPE_DECL in TYPE_NAME if necessary.
> +	     This is needed for ODR violation warnings to come out right (we
> +	     want duplicate TYPE_DECLs whenever the type is duplicated because
> +	     of ODR violation.  Because lang data in the TYPE_DECL may not
> +	     have been freed yet, rebuild it from scratch and copy relevant
> +	     fields.  */
> +	  TYPE_NAME (copy) = fld_simplified_type_name (copy);
> +	  tree name = TYPE_NAME (copy);
> +
> +	  if (name && TREE_CODE (name) == TYPE_DECL)
> +	    {
> +	      gcc_checking_assert (TREE_TYPE (name) == t);
> +	      tree name2 = build_decl (DECL_SOURCE_LOCATION (name), TYPE_DECL,
> +				       DECL_NAME (name), copy);
> +	      SET_DECL_ASSEMBLER_NAME (name2, DECL_ASSEMBLER_NAME (name));
> +	      SET_DECL_ALIGN (name2, 0);
> +	      DECL_CONTEXT (name2) = fld_decl_context
> +					 (DECL_CONTEXT (name));
> +	      TYPE_NAME (copy) = name2;
> +	    }
>  	}
>        return copy;
>     }
> @@ -5649,12 +5688,13 @@ free_lang_data_in_decl (tree decl, struc
>      {
>        DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
> -      /* TREE_PUBLIC is used to tell if type is anonymous.  */
> +      TREE_PUBLIC (decl) = 0;
> +      TREE_PRIVATE (decl) = 0;
> +      DECL_ARTIFICIAL (decl) = 0;
>        TYPE_DECL_SUPPRESS_DEBUG (decl) = 0;
>        DECL_INITIAL (decl) = NULL_TREE;
>        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
>        DECL_MODE (decl) = VOIDmode;
> -      TREE_TYPE (decl) = void_type_node;
>        SET_DECL_ALIGN (decl, 0);
>      }
>    else if (TREE_CODE (decl) == FIELD_DECL)
> @@ -5688,20 +5728,7 @@ free_lang_data_in_decl (tree decl, struc
>    if (TREE_CODE (decl) != FIELD_DECL
>        && ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL)
>            || !DECL_VIRTUAL_P (decl)))
> -    {
> -      tree ctx = DECL_CONTEXT (decl);
> -      /* Variably modified types are needed for tree_is_indexable to decide
> -	 whether the type needs to go to local or global section.
> -	 This code is semi-broken but for now it is easiest to keep contexts
> -	 as expected.  */
> -      if (ctx && TYPE_P (ctx)
> -	  && !variably_modified_type_p (ctx, NULL_TREE))
> -	 {
> -	   while (ctx && TYPE_P (ctx))
> -	     ctx = TYPE_CONTEXT (ctx);
> -	   DECL_CONTEXT (decl) = ctx;
> -	 }
> -    }
> +    DECL_CONTEXT (decl) = fld_decl_context (DECL_CONTEXT (decl));
>  }
>  
>  
> Index: ipa-devirt.c
> ===================================================================
> --- ipa-devirt.c	(revision 266325)
> +++ ipa-devirt.c	(working copy)
> @@ -2289,27 +2289,39 @@ dump_type_inheritance_graph (FILE *f)
>  	   "%i duplicates overall\n", num_all_types, num_types, num_duplicates);
>  }
>  
> -/* Save some WPA->ltrans streaming by freeing enum values.  */
> +/* Save some WPA->ltrans streaming by freeing stuff needed only for good
> +   ODR warnings.  */
>  
>  static void
> -free_enum_values ()
> +free_odr_warning_data ()
>  {
> -  static bool enum_values_freed = false;
> -  if (enum_values_freed || !flag_wpa || !odr_types_ptr)
> +  static bool odr_data_freed = false;
> +
> +  if (odr_data_freed || !flag_wpa || !odr_types_ptr)
>      return;
> -  enum_values_freed = true;
> -  unsigned int i;
> -  for (i = 0; i < odr_types.length (); i++)
> +
> +  odr_data_freed = true;
> +
> +  for (unsigned int i = 0; i < odr_types.length (); i++)
>      if (odr_types[i])
>        {
> -	if (TREE_CODE (odr_types[i]->type) == ENUMERAL_TYPE)
> -	  TYPE_VALUES (odr_types[i]->type) = NULL;
> +	tree t = odr_types[i]->type;
> +
> +	if (TREE_CODE (t) == ENUMERAL_TYPE)
> +	  TYPE_VALUES (t) = NULL;
> +	TREE_TYPE (TYPE_NAME (t)) = void_type_node;

... here explaining the late "free-lang-data".

Richard.

> +
>  	if (odr_types[i]->types)
>            for (unsigned int j = 0; j < odr_types[i]->types->length (); j++)
> -	    if (TREE_CODE ((*odr_types[i]->types)[j]) == ENUMERAL_TYPE)
> -	      TYPE_VALUES ((*odr_types[i]->types)[j]) = NULL;
> +	    {
> +	      tree td = (*odr_types[i]->types)[j];
> +
> +	      if (TREE_CODE (td) == ENUMERAL_TYPE)
> +	        TYPE_VALUES (td) = NULL;
> +	      TYPE_NAME (td) = TYPE_NAME (t);
> +	    }
>        }
> -  enum_values_freed = true;
> +  odr_data_freed = true;
>  }
>  
>  /* Initialize IPA devirt and build inheritance tree graph.  */
> @@ -2323,7 +2335,7 @@ build_type_inheritance_graph (void)
>  
>    if (odr_hash)
>      {
> -      free_enum_values ();
> +      free_odr_warning_data ();
>        return;
>      }
>    timevar_push (TV_IPA_INHERITANCE);
> @@ -2370,7 +2382,7 @@ build_type_inheritance_graph (void)
>        dump_type_inheritance_graph (inheritance_dump_file);
>        dump_end (TDI_inheritance, inheritance_dump_file);
>      }
> -  free_enum_values ();
> +  free_odr_warning_data ();
>    timevar_pop (TV_IPA_INHERITANCE);
>  }
>  
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)