Properly merge forced_by_abi when merging nodes

Hi,
while merging nodes in lto-symtab, we need to copy the force_output and forced_by_abi flags.
Thanks to Markus who noticed the issue.

Bootstrapped/regtested x86_64-linux, comitted.

Honza

On Tue, Feb 4, 2014 at 6:52 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> while merging nodes in lto-symtab, we need to copy the force_output and forced_by_abi flags.
> Thanks to Markus who noticed the issue.

Don't we need to choose the forced_by_abi variant if there are variants
without that flag?  Do we maybe even need to warn about such mismatches?

Richard.

> Bootstrapped/regtested x86_64-linux, comitted.
>
> Honza
>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog   (revision 207438)
> +++ ChangeLog   (working copy)
> @@ -1,3 +1,9 @@
> +2014-02-04  Jan Hubicka  <hubicka@ucw.cz>
> +           Markus Trippelsdorf
> +
> +       * lto-symtab.c (lto_cgraph_replace_node, lto_varpool_replace_node):
> +       merge force_output and forced_by_abi flags.
> +
>  2014-01-24  Balaji V. Iyer  <balaji.v.iyer@intel.com>
>
>         * lto-lang.c (lto_init): Replaced flag_enable_cilkplus with
> Index: lto-symtab.c
> ===================================================================
> --- lto-symtab.c        (revision 207438)
> +++ lto-symtab.c        (working copy)
> @@ -59,6 +59,8 @@ lto_cgraph_replace_node (struct cgraph_n
>    /* Merge node flags.  */
>    if (node->force_output)
>      cgraph_mark_force_output_node (prevailing_node);
> +  if (node->forced_by_abi)
> +    prevailing_node->forced_by_abi = true;
>    if (node->address_taken)
>      {
>        gcc_assert (!prevailing_node->global.inlined_to);
> @@ -110,6 +112,10 @@ lto_varpool_replace_node (varpool_node *
>    gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
>
>    ipa_clone_referring (prevailing_node, &vnode->ref_list);
> +  if (vnode->force_output)
> +    prevailing_node->force_output = true;
> +  if (vnode->forced_by_abi)
> +    prevailing_node->forced_by_abi = true;
>
>    /* Be sure we can garbage collect the initializer.  */
>    if (DECL_INITIAL (vnode->decl)

> On Tue, Feb 4, 2014 at 6:52 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > Hi,
> > while merging nodes in lto-symtab, we need to copy the force_output and forced_by_abi flags.
> > Thanks to Markus who noticed the issue.
> 
> Don't we need to choose the forced_by_abi variant if there are variants
> without that flag?  Do we maybe even need to warn about such mismatches?

I think it is ok to choose any variant - just as linked would do.  In the
testcase these are all COMDAT. One is explicitely instantiated and the bug is
that we privatized the comdat to the DSO assuming that all users will make
their own comdats.

LLVM has two variants of the header - one with implementation in it and one
without and it assumes that because one of DSOs explicitely instantiate it,
the others may or may not use the other variant.
I am not 100% sure it is valid C++, but this way it should work.

Honza```
> 
> Richard.
> 
> > Bootstrapped/regtested x86_64-linux, comitted.
> >
> > Honza
> >
> > Index: ChangeLog
> > ===================================================================
> > --- ChangeLog   (revision 207438)
> > +++ ChangeLog   (working copy)
> > @@ -1,3 +1,9 @@
> > +2014-02-04  Jan Hubicka  <hubicka@ucw.cz>
> > +           Markus Trippelsdorf
> > +
> > +       * lto-symtab.c (lto_cgraph_replace_node, lto_varpool_replace_node):
> > +       merge force_output and forced_by_abi flags.
> > +
> >  2014-01-24  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >
> >         * lto-lang.c (lto_init): Replaced flag_enable_cilkplus with
> > Index: lto-symtab.c
> > ===================================================================
> > --- lto-symtab.c        (revision 207438)
> > +++ lto-symtab.c        (working copy)
> > @@ -59,6 +59,8 @@ lto_cgraph_replace_node (struct cgraph_n
> >    /* Merge node flags.  */
> >    if (node->force_output)
> >      cgraph_mark_force_output_node (prevailing_node);
> > +  if (node->forced_by_abi)
> > +    prevailing_node->forced_by_abi = true;
> >    if (node->address_taken)
> >      {
> >        gcc_assert (!prevailing_node->global.inlined_to);
> > @@ -110,6 +112,10 @@ lto_varpool_replace_node (varpool_node *
> >    gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
> >
> >    ipa_clone_referring (prevailing_node, &vnode->ref_list);
> > +  if (vnode->force_output)
> > +    prevailing_node->force_output = true;
> > +  if (vnode->forced_by_abi)
> > +    prevailing_node->forced_by_abi = true;
> >
> >    /* Be sure we can garbage collect the initializer.  */
> >    if (DECL_INITIAL (vnode->decl)