Honza,
Thanks for reverting the patch. I will check if this resolves the
current bootstrap problem.
I was suggesting that you create a branch for all of the visibility
changes to make it easier to track the various original patches and
later correction patches from you.
I don't know if the gen* programs hang because of the visibility
changes or because of the change in sections. The change in sections
could conflict with the GCC code to handle AIX XCOFF CSECTs for
functions.
AIX recently added support for ELF-like visibility. AIX previously
supported the equivalent of visibility through "export" files. The
recent problems could be due to issues with assembly file ordering,
but they also could be related to the visibility changes affecting the
way that GCC emits code to branch to global functions.
Also, your recent ChangeLog entry included a Subversion conflict marker:
2014-06-15 Jan Hubicka <hubi...@ucw.cz>
* c-family/c-common.c (handle_tls_model_attribute): Use set_decl_tls_mod
el.
* c-family/c-common.c (handle_tls_model_attribute): Use
set_decl_tls_model.
>>>>>>> .r211699
* cgraph.h (struct varpool_node): Add tls_model.
* tree.c (decl_tls_model, set_decl_tls_model): New functions.
* tree.h (DECL_TLS_MODEL): Update.
Thanks, David
On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> Honza,
>>
>> The cgraph patch in r211600 broke AIX bootstrap again. I cannot find
>> the corresponding patch in the GCC Patches mailing list, so I do not
>> see where this was discussed or approved.
>
> Sorry, I remember writting mail about this patch but also can't find it. The
> patch introduces reset_section that is used when function or variable is
> brought local by the visibility code. In this case we previously incorrectly
> cleared user sections and we forgot to introduce implicit sections with
> -fdata-section and -ffunction-section.
>
> I will revert this change and lets debug it.
>
>>
>> With the patch in r211600, the "gen*" programs in stage2 go into an
>> endless loop.
>>
>> Please revert these comdat patches. These were not tested appropriately.
>>
>> Please create a branch and we can debug the problems on AIX.
>
> What patches you refer to? I already disabled the localization within
> initializers on AIX (well all targets w/o MAKE_ONE_ONLY support) and
> I am testing patch reverting this change (I want to keep reset section
> just remove the call to resolve unique section) and will commit once
> it converges.
>
> I will send you patch to enable those two changes and lets look into why
> they break. What is common to both patches is that they deal with static
> symbol in named sections, perhaps that is not correctly supported by AIX
> toolchain...
>
> My apologizes for the breakage. I am attaching the patch for reference.
> Honza
>
>
> * symtab.c (symtab_node::reset_section): New method.
> * cgraph.c (cgraph_node_cannot_be_local_p_1): Accept non-local
> for localization.
> * cgraph.h (reset_section): Declare.
> * ipa-inline-analysis.c (do_estimate_growth): Check for comdat groups;
> do not consider comdat locals.
> * cgraphclones.c (set_new_clone_decl_and_node_flags): Get section
> for new symbol.
> * ipa-visiblity.c (cgraph_externally_visible_p): Cleanup.
> (update_visibility_by_resolution_info): Consider UNDEF; fix checking;
> reset sections of symbols dragged out of the comdats.
> (function_and_variable_visibility): Reset sections of localized
> symbols.
> Index: symtab.c
> ===================================================================
> --- symtab.c (revision 211489)
> +++ symtab.c (working copy)
> @@ -1176,6 +1176,21 @@ symtab_node::set_section (const char *se
> symtab_for_node_and_aliases (this, set_section_1, const_cast<char
> *>(section), true);
> }
>
> +/* Reset section of NODE. That is when NODE is being brought local
> + we may want to clear section produced for comdat group and depending
> + on function-sections produce now, local, unique section for it. */
> +
> +void
> +symtab_node::reset_section ()
> +{
> + if (!this->implicit_section)
> + return;
> + this->set_section (NULL);
> + resolve_unique_section (this->decl, 0,
> + is_a <cgraph_node *> (this)
> + ? flag_function_sections : flag_data_sections);
> +}
> +
> /* Worker for symtab_resolve_alias. */
>
> static bool
> Index: cgraph.c
> ===================================================================
> --- cgraph.c (revision 211488)
> +++ cgraph.c (working copy)
> @@ -2169,6 +2169,7 @@ cgraph_node_cannot_be_local_p_1 (struct
> && !node->forced_by_abi
> && !symtab_used_from_object_file_p (node)
> && !node->same_comdat_group)
> + || DECL_EXTERNAL (node->decl)
> || !node->externally_visible));
> }
>
> @@ -2259,14 +2260,14 @@ cgraph_make_node_local_1 (struct cgraph_
> {
> symtab_make_decl_local (node->decl);
>
> - node->set_section (NULL);
> node->set_comdat_group (NULL);
> node->externally_visible = false;
> node->forced_by_abi = false;
> node->local.local = true;
> - node->set_section (NULL);
> + node->reset_section ();
> node->unique_name = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> - || node->resolution ==
> LDPR_PREVAILING_DEF_IRONLY_EXP);
> + || node->unique_name
> + || node->resolution ==
> LDPR_PREVAILING_DEF_IRONLY_EXP);
> node->resolution = LDPR_PREVAILING_DEF_IRONLY;
> gcc_assert (cgraph_function_body_availability (node) == AVAIL_LOCAL);
> }
> Index: cgraph.h
> ===================================================================
> --- cgraph.h (revision 211489)
> +++ cgraph.h (working copy)
> @@ -208,6 +208,7 @@ public:
> /* Set section for symbol and its aliases. */
> void set_section (const char *section);
> void set_section_for_node (const char *section);
> + void reset_section ();
> };
>
> enum availability
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c (revision 211488)
> +++ ipa-inline-analysis.c (working copy)
> @@ -3877,7 +3877,7 @@ do_estimate_growth (struct cgraph_node *
> /* COMDAT functions are very often not shared across multiple units
> since they come from various template instantiations.
> Take this into account. */
> - else if (DECL_COMDAT (node->decl)
> + else if (node->externally_visible && node->get_comdat_group ()
> && cgraph_can_remove_if_no_direct_calls_p (node))
> d.growth -= (info->size
> * (100 - PARAM_VALUE (PARAM_COMDAT_SHARING_PROBABILITY))
> @@ -3928,7 +3928,7 @@ growth_likely_positive (struct cgraph_no
> && (ret = node_growth_cache[node->uid]))
> return ret > 0;
> if (!cgraph_will_be_removed_from_program_if_no_direct_calls (node)
> - && (!DECL_COMDAT (node->decl)
> + && (!node->externally_visible || !node->get_comdat_group ()
> || !cgraph_can_remove_if_no_direct_calls_p (node)))
> return true;
> max_callers = inline_summary (node)->size * 4 / edge_growth + 2;
> Index: cgraphclones.c
> ===================================================================
> --- cgraphclones.c (revision 211488)
> +++ cgraphclones.c (working copy)
> @@ -293,6 +293,7 @@ set_new_clone_decl_and_node_flags (cgrap
> new_node->externally_visible = 0;
> new_node->local.local = 1;
> new_node->lowered = true;
> + new_node->reset_section ();
> }
>
> /* Duplicate thunk THUNK if necessary but make it to refer to NODE.
> Index: ipa-visibility.c
> ===================================================================
> --- ipa-visibility.c (revision 211488)
> +++ ipa-visibility.c (working copy)
> @@ -236,7 +236,7 @@ cgraph_externally_visible_p (struct cgra
> return true;
> if (node->resolution == LDPR_PREVAILING_DEF_IRONLY)
> return false;
> - /* When doing LTO or whole program, we can bring COMDAT functoins static.
> + /* When doing LTO or whole program, we can bring COMDAT functions static.
> This improves code quality and we know we will duplicate them at most
> twice
> (in the case that we are not using plugin and link with object file
> implementing same COMDAT) */
> @@ -295,8 +295,6 @@ varpool_externally_visible_p (varpool_no
> Even if the linker clams the symbol is unused, never bring internal
> symbols that are declared by user as used or externally visible.
> This is needed for i.e. references from asm statements. */
> - if (symtab_used_from_object_file_p (vnode))
> - return true;
> if (vnode->resolution == LDPR_PREVAILING_DEF_IRONLY)
> return false;
>
> @@ -386,7 +384,8 @@ update_visibility_by_resolution_info (sy
>
> if (!node->externally_visible
> || (!DECL_WEAK (node->decl) && !DECL_ONE_ONLY (node->decl))
> - || node->resolution == LDPR_UNKNOWN)
> + || node->resolution == LDPR_UNKNOWN
> + || node->resolution == LDPR_UNDEF)
> return;
>
> define = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> @@ -397,7 +396,7 @@ update_visibility_by_resolution_info (sy
> if (node->same_comdat_group)
> for (symtab_node *next = node->same_comdat_group;
> next != node; next = next->same_comdat_group)
> - gcc_assert (!node->externally_visible
> + gcc_assert (!next->externally_visible
> || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
> || next->resolution == LDPR_PREVAILING_DEF
> || next->resolution ==
> LDPR_PREVAILING_DEF_IRONLY_EXP));
> @@ -411,11 +410,15 @@ update_visibility_by_resolution_info (sy
> if (next->externally_visible
> && !define)
> DECL_EXTERNAL (next->decl) = true;
> + if (!next->alias)
> + next->reset_section ();
> }
> node->set_comdat_group (NULL);
> DECL_WEAK (node->decl) = false;
> if (!define)
> DECL_EXTERNAL (node->decl) = true;
> + if (!node->alias)
> + node->reset_section ();
> symtab_dissolve_same_comdat_group_list (node);
> }
>
> @@ -476,7 +479,7 @@ function_and_variable_visibility (bool w
> symtab_dissolve_same_comdat_group_list (node);
> }
> gcc_assert ((!DECL_WEAK (node->decl)
> - && !DECL_COMDAT (node->decl))
> + && !DECL_COMDAT (node->decl))
> || TREE_PUBLIC (node->decl)
> || node->weakref
> || DECL_EXTERNAL (node->decl));
> @@ -494,6 +497,7 @@ function_and_variable_visibility (bool w
> && node->definition && !node->weakref
> && !DECL_EXTERNAL (node->decl))
> {
> + bool reset = TREE_PUBLIC (node->decl);
> gcc_assert (whole_program || in_lto_p
> || !TREE_PUBLIC (node->decl));
> node->unique_name = ((node->resolution == LDPR_PREVAILING_DEF_IRONLY
> @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
> next = next->same_comdat_group)
> {
> next->set_comdat_group (NULL);
> - if (!next->alias)
> - next->set_section (NULL);
> symtab_make_decl_local (next->decl);
> + if (!node->alias)
> + node->reset_section ();
> next->unique_name = ((next->resolution ==
> LDPR_PREVAILING_DEF_IRONLY
> || next->unique_name
> || next->resolution ==
> LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -528,9 +532,9 @@ function_and_variable_visibility (bool w
> }
> if (TREE_PUBLIC (node->decl))
> node->set_comdat_group (NULL);
> - if (DECL_COMDAT (node->decl) && !node->alias)
> - node->set_section (NULL);
> symtab_make_decl_local (node->decl);
> + if (reset && !node->alias)
> + node->reset_section ();
> }
>
> if (node->thunk.thunk_p
> @@ -632,6 +636,7 @@ function_and_variable_visibility (bool w
> if (!vnode->externally_visible
> && !vnode->weakref)
> {
> + bool reset = TREE_PUBLIC (vnode->decl);
> 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)
> @@ -647,9 +652,9 @@ function_and_variable_visibility (bool w
> next = next->same_comdat_group)
> {
> next->set_comdat_group (NULL);
> - if (!next->alias)
> - next->set_section (NULL);
> symtab_make_decl_local (next->decl);
> + if (!next->alias)
> + next->reset_section ();
> next->unique_name = ((next->resolution ==
> LDPR_PREVAILING_DEF_IRONLY
> || next->unique_name
> || next->resolution ==
> LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -659,9 +664,9 @@ function_and_variable_visibility (bool w
> }
> if (TREE_PUBLIC (vnode->decl))
> vnode->set_comdat_group (NULL);
> - if (DECL_COMDAT (vnode->decl) && !vnode->alias)
> - vnode->set_section (NULL);
> symtab_make_decl_local (vnode->decl);
> + if (reset && !vnode->alias)
> + vnode->reset_section ();
> vnode->resolution = LDPR_PREVAILING_DEF_IRONLY;
> }
> update_visibility_by_resolution_info (vnode);
