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 <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
