> This patch refactors the decl localizing that happens in
> function_and_variable_visibility. It doesn't fix the bug I'm working on
> (that's next).
>
> Both the FOR_EACH_FUNCTION and FOR_EACH_VARIABLE loops contain very similar,
> but not quite the same code for localizing a definition that it's determined
> need not be externally visible. It looks to me that the
> not-quite-the-sameness is erroneous, and this patch refactors that code into
> a common subroutine. If the differences need to be maintained (slight
> differences in when unique_name is updated and whether resolution is set to
> LDPR_PREVAILING_DEF_IRONLY), I think a flag to the new function would be
> best, rather than keep the duplicated code.
>
> booted & tested on x86_64-linux, ok?
OK,
the code has indeed grown into quite a mess over the years ;)
Thanks,
Honza
>
> nathan
> --
> Nathan Sidwell
> 2017-01-06 Nathan Sidwell <nat...@acm.org>
>
> * ipa-visibility.c (localize_node): New function, broken out of ...
> (function_and_variable_visibility): Call it.
>
> Index: ipa-visibility.c
> ===================================================================
> --- ipa-visibility.c (revision 244159)
> +++ ipa-visibility.c (working copy)
> @@ -529,6 +529,53 @@ optimize_weakref (symtab_node *node)
> gcc_assert (node->alias);
> }
>
> +/* NODE is an externally visible definition, which we've discovered is
> + not needed externally. Make it local to this compilation. */
> +
> +static void
> +localize_node (bool whole_program, symtab_node *node)
> +{
> + gcc_assert (whole_program || in_lto_p || !TREE_PUBLIC (node->decl));
> +
> + if (node->same_comdat_group && TREE_PUBLIC (node->decl))
> + {
> + for (symtab_node *next = node->same_comdat_group;
> + next != node; next = next->same_comdat_group)
> + {
> + next->set_comdat_group (NULL);
> + if (!next->alias)
> + next->set_section (NULL);
> + if (!next->transparent_alias)
> + next->make_decl_local ();
> + next->unique_name
> + |= ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
> + || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> + && TREE_PUBLIC (next->decl)
> + && !flag_incremental_link);
> + }
> +
> + /* Now everything's localized, the grouping has no meaning, and
> + will cause crashes if we keep it around. */
> + node->dissolve_same_comdat_group_list ();
> + }
> +
> + node->unique_name
> + |= ((node->resolution == LDPR_PREVAILING_DEF_IRONLY
> + || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> + && TREE_PUBLIC (node->decl)
> + && !flag_incremental_link);
> +
> + if (TREE_PUBLIC (node->decl))
> + node->set_comdat_group (NULL);
> + if (DECL_COMDAT (node->decl) && !node->alias)
> + node->set_section (NULL);
> + if (!node->transparent_alias)
> + {
> + node->resolution = LDPR_PREVAILING_DEF_IRONLY;
> + node->make_decl_local ();
> + }
> +}
> +
> /* Decide on visibility of all symbols. */
>
> static unsigned int
> @@ -606,48 +653,7 @@ function_and_variable_visibility (bool w
> if (!node->externally_visible
> && node->definition && !node->weakref
> && !DECL_EXTERNAL (node->decl))
> - {
> - gcc_assert (whole_program || in_lto_p
> - || !TREE_PUBLIC (node->decl));
> - node->unique_name
> - |= ((node->resolution == LDPR_PREVAILING_DEF_IRONLY
> - || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> - && TREE_PUBLIC (node->decl)
> - && !flag_incremental_link);
> - node->resolution = LDPR_PREVAILING_DEF_IRONLY;
> - if (node->same_comdat_group && TREE_PUBLIC (node->decl))
> - {
> - symtab_node *next = node;
> -
> - /* Set all members of comdat group local. */
> - for (next = node->same_comdat_group;
> - next != node;
> - next = next->same_comdat_group)
> - {
> - next->set_comdat_group (NULL);
> - if (!next->alias)
> - next->set_section (NULL);
> - if (!next->transparent_alias)
> - next->make_decl_local ();
> - next->unique_name
> - |= ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
> - || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> - && TREE_PUBLIC (next->decl)
> - && !flag_incremental_link);
> - }
> - /* cgraph_externally_visible_p has already checked all
> - other nodes in the group and they will all be made
> - local. We need to dissolve the group at once so that
> - the predicate does not segfault though. */
> - node->dissolve_same_comdat_group_list ();
> - }
> - if (TREE_PUBLIC (node->decl))
> - node->set_comdat_group (NULL);
> - if (DECL_COMDAT (node->decl) && !node->alias)
> - node->set_section (NULL);
> - if (!node->transparent_alias)
> - node->make_decl_local ();
> - }
> + localize_node (whole_program, node);
>
> if (node->thunk.thunk_p
> && !node->thunk.add_pointer_bounds_args
> @@ -757,49 +763,11 @@ function_and_variable_visibility (bool w
> if (lookup_attribute ("no_reorder",
> DECL_ATTRIBUTES (vnode->decl)))
> vnode->no_reorder = 1;
> +
> if (!vnode->externally_visible
> && !vnode->transparent_alias)
> - {
> - gcc_assert (in_lto_p || whole_program || !TREE_PUBLIC (vnode->decl));
> - vnode->unique_name |= ((vnode->resolution ==
> LDPR_PREVAILING_DEF_IRONLY
> - || vnode->resolution
> - == LDPR_PREVAILING_DEF_IRONLY_EXP)
> - && TREE_PUBLIC (vnode->decl)
> - && !flag_incremental_link);
> - if (vnode->same_comdat_group && TREE_PUBLIC (vnode->decl))
> - {
> - symtab_node *next = vnode;
> + localize_node (whole_program, vnode);
>
> - /* Set all members of comdat group local. */
> - if (vnode->same_comdat_group)
> - for (next = vnode->same_comdat_group;
> - next != vnode;
> - next = next->same_comdat_group)
> - {
> - next->set_comdat_group (NULL);
> - if (!next->alias)
> - next->set_section (NULL);
> - if (!next->transparent_alias)
> - {
> - next->make_decl_local ();
> - next->unique_name |= ((next->resolution ==
> LDPR_PREVAILING_DEF_IRONLY
> - || next->resolution ==
> LDPR_PREVAILING_DEF_IRONLY_EXP)
> - && TREE_PUBLIC (next->decl)
> - && !flag_incremental_link);
> - }
> - }
> - vnode->dissolve_same_comdat_group_list ();
> - }
> - if (TREE_PUBLIC (vnode->decl))
> - vnode->set_comdat_group (NULL);
> - if (DECL_COMDAT (vnode->decl) && !vnode->alias)
> - vnode->set_section (NULL);
> - if (!vnode->transparent_alias)
> - {
> - vnode->make_decl_local ();
> - vnode->resolution = LDPR_PREVAILING_DEF_IRONLY;
> - }
> - }
> update_visibility_by_resolution_info (vnode);
>
> /* Update virtual tables to point to local aliases where possible. */
