Re: Another AIX Bootstrap failure

[Ramana Radhakrishnan <ramana.gcc@googlemail.com>]

On Mon, Jun 16, 2014 at 5:35 AM, Jan Hubicka <hubicka@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.
>

It looks like this broke arm-none-linux-gnueabihf bootstraps with
section conflicts in strstream.c. Is that the kind of failures you are
seeing on AIX ?

/work/trunk-nightlies/builds/build-210919/armv7l-unknown-linux-gnueabihf/libstdc++-v3/include/backward/strstream:126:9:
error: std::istrstream::_ZTVSt10istrstream.localalias.0 causes a
section type conflict with std::istrstream::_ZTVSt10istrstream

 Can you please test it on this platform before reapplying / or let me
know when you have the smaller patches. I'll see what I can do.

regards
Ramana

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