> 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);
