[PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

Hi, 
This patch addresses issue discussed in 
http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html

With the patch, any declaration which is resolved as LDPR_PREVAILING_DEF
and compiled with -fwhole-program is annotated with
attribute "externally_visible" if it doesn't exist already. 
This eliminates the error-prone process of manual annotation
of the attribute when compiling mixed LTO/non-LTO applications. 

For the following test files:
a.c

#include <string.h>
#include <stdio.h>
extern int foo(int);
/* Called by b.c, should not be optimized by -fwhole-program */
void bar() 
{
  printf("bar\n");
}  
/* Not used by others, should be optimized out by -fwhole-program*/
void bar2()
{
  printf("bar2\n");
}
extern int src[], dst[];
int vvvvvv;
int main()
{ 
  int ret;
  vvvvvv = 12; 
  ret = foo(20);
  bar2();
  memcpy(dst, src, 100);
  return ret + 3;
}  

b.c

#include <stdio.h>
int src[100];
int dst[100];
extern int vvvvvv;
extern void bar();
int foo(int c)
{
  printf("Hello world: %d\n", c);
  bar();
  return 1000 + vvvvvv;
}

~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
~/work/install-x86/bin/gcc  b.c -O2 -c 
ar cru libb.a b.o
~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-plugin -o f -fwhole-program

The code is compiled and linked correctly. bar & vvvvvv don't become static function
and cause link errors, whereas bar2 is inlined as expected. 


Bootstrapped and tested on x86_64-unknown-linux-gnu.

Cheers,
Bingfeng Mei

2010-06-09  Bingfeng Mei <bmei@broadcom.com>

	* lto-symbtab.c (lto_symtab_resolve_symbols): Add externally_visible
        attribute for declaration of LDPR_PREVAILING_DEF when compiling with
        -fwhole-program
        

Index: lto-symtab.c
===================================================================
--- lto-symtab.c        (revision 160463)
+++ lto-symtab.c        (working copy)
@@ -476,7 +476,19 @@

   /* If the chain is already resolved there is nothing else to do.  */
   if (e->resolution != LDPR_UNKNOWN)
-    return;
+    {
+      /* Add externally_visible attribute for declaration of LDPR_PREVAILING_DEF */
+      if (e->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
+        {
+          if (!lookup_attribute ("externally_visible", DECL_ATTRIBUTES (e->decl)))
+            {
+              DECL_ATTRIBUTES (e->decl)
+                = tree_cons (get_identifier ("externally_visible"), NULL_TREE,
+                             DECL_ATTRIBUTES (e->decl));
+            }
+        }
+      return;
+    }

   /* Find the single non-replaceable prevailing symbol and
      diagnose ODR violations.  */

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Hi,
> This patch addresses issue discussed in
> http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
>
> With the patch, any declaration which is resolved as LDPR_PREVAILING_DEF
> and compiled with -fwhole-program is annotated with
> attribute "externally_visible" if it doesn't exist already.
> This eliminates the error-prone process of manual annotation
> of the attribute when compiling mixed LTO/non-LTO applications.
>
> For the following test files:
> a.c
>
> #include <string.h>
> #include <stdio.h>
> extern int foo(int);
> /* Called by b.c, should not be optimized by -fwhole-program */
> void bar()
> {
>  printf("bar\n");
> }
> /* Not used by others, should be optimized out by -fwhole-program*/
> void bar2()
> {
>  printf("bar2\n");
> }
> extern int src[], dst[];
> int vvvvvv;
> int main()
> {
>  int ret;
>  vvvvvv = 12;
>  ret = foo(20);
>  bar2();
>  memcpy(dst, src, 100);
>  return ret + 3;
> }
>
> b.c
>
> #include <stdio.h>
> int src[100];
> int dst[100];
> extern int vvvvvv;
> extern void bar();
> int foo(int c)
> {
>  printf("Hello world: %d\n", c);
>  bar();
>  return 1000 + vvvvvv;
> }
>
> ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> ~/work/install-x86/bin/gcc  b.c -O2 -c
> ar cru libb.a b.o
> ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-plugin -o f -fwhole-program
>
> The code is compiled and linked correctly. bar & vvvvvv don't become static function
> and cause link errors, whereas bar2 is inlined as expected.
>
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Cheers,
> Bingfeng Mei
>
> 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
>
>        * lto-symbtab.c (lto_symtab_resolve_symbols): Add externally_visible
>        attribute for declaration of LDPR_PREVAILING_DEF when compiling with
>        -fwhole-program
>
>
> Index: lto-symtab.c
> ===================================================================
> --- lto-symtab.c        (revision 160463)
> +++ lto-symtab.c        (working copy)
> @@ -476,7 +476,19 @@
>
>   /* If the chain is already resolved there is nothing else to do.  */
>   if (e->resolution != LDPR_UNKNOWN)
> -    return;
> +    {
> +      /* Add externally_visible attribute for declaration of LDPR_PREVAILING_DEF */
> +      if (e->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
> +        {
> +          if (!lookup_attribute ("externally_visible", DECL_ATTRIBUTES (e->decl)))
> +            {
> +              DECL_ATTRIBUTES (e->decl)
> +                = tree_cons (get_identifier ("externally_visible"), NULL_TREE,
> +                             DECL_ATTRIBUTES (e->decl));

I don't think we need to add an attribute here but we can play
with some cgraph flags which is cheaper.

Also I think this isn't really correct - not everything that prevails
needs to be externally visible (in fact, you seem to simply
remove the effect of -fwhole-program completely).

A testcase that should still work is

t1.c:
void foo(void) { bar (); }
t2.c
extern void foo (void);
void bar (void) {}
void eliminate_me (void) {}
int main()
{
   foo();
}

and eliminate_me should still be eliminated with -fwhole-program
if you do

gcc -c t1.c
gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin

Thus, the resolution file probably does not have the information
you need (a list of references from outside of the LTO file set).

Richard.

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 09 June 2010 16:02
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> > Hi,
> > This patch addresses issue discussed in
> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >
> > With the patch, any declaration which is resolved as
> LDPR_PREVAILING_DEF
> > and compiled with -fwhole-program is annotated with
> > attribute "externally_visible" if it doesn't exist already.
> > This eliminates the error-prone process of manual annotation
> > of the attribute when compiling mixed LTO/non-LTO applications.
> >
> > For the following test files:
> > a.c
> >
> > #include <string.h>
> > #include <stdio.h>
> > extern int foo(int);
> > /* Called by b.c, should not be optimized by -fwhole-program */
> > void bar()
> > {
> >  printf("bar\n");
> > }
> > /* Not used by others, should be optimized out by -fwhole-program*/
> > void bar2()
> > {
> >  printf("bar2\n");
> > }
> > extern int src[], dst[];
> > int vvvvvv;
> > int main()
> > {
> >  int ret;
> >  vvvvvv = 12;
> >  ret = foo(20);
> >  bar2();
> >  memcpy(dst, src, 100);
> >  return ret + 3;
> > }
> >
> > b.c
> >
> > #include <stdio.h>
> > int src[100];
> > int dst[100];
> > extern int vvvvvv;
> > extern void bar();
> > int foo(int c)
> > {
> >  printf("Hello world: %d\n", c);
> >  bar();
> >  return 1000 + vvvvvv;
> > }
> >
> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> > ar cru libb.a b.o
> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-plugin
> -o f -fwhole-program
> >
> > The code is compiled and linked correctly. bar & vvvvvv don't become
> static function
> > and cause link errors, whereas bar2 is inlined as expected.
> >
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > Cheers,
> > Bingfeng Mei
> >
> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >
> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> externally_visible
> >        attribute for declaration of LDPR_PREVAILING_DEF when
> compiling with
> >        -fwhole-program
> >
> >
> > Index: lto-symtab.c
> > ===================================================================
> > --- lto-symtab.c        (revision 160463)
> > +++ lto-symtab.c        (working copy)
> > @@ -476,7 +476,19 @@
> >
> >   /* If the chain is already resolved there is nothing else to
> do.  */
> >   if (e->resolution != LDPR_UNKNOWN)
> > -    return;
> > +    {
> > +      /* Add externally_visible attribute for declaration of
> LDPR_PREVAILING_DEF */
> > +      if (e->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
> > +        {
> > +          if (!lookup_attribute ("externally_visible",
> DECL_ATTRIBUTES (e->decl)))
> > +            {
> > +              DECL_ATTRIBUTES (e->decl)
> > +                = tree_cons (get_identifier ("externally_visible"),
> NULL_TREE,
> > +                             DECL_ATTRIBUTES (e->decl));
> 
> I don't think we need to add an attribute here but we can play
> with some cgraph flags which is cheaper.

Then I also need to change relevant flag for variable declaration. Not sure what 
I should change. 

> 
> Also I think this isn't really correct - not everything that prevails
> needs to be externally visible (in fact, you seem to simply
> remove the effect of -fwhole-program completely).

According to doc, LDPR_PREVAILING_DEF is the definition accessed outside
of LTO IR as opposed to LDPR_PREVAILING_DEF_IRONLY. So I think it should
be externally visible. In my test file, bar2 is the similar as eliminate_me
in your testcase. It is correctly eliminated with -fwhole-program. 

> 
> A testcase that should still work is
> 
> t1.c:
> void foo(void) { bar (); }
> t2.c
> extern void foo (void);
> void bar (void) {}
> void eliminate_me (void) {}
> int main()
> {
>    foo();
> }
> 
> and eliminate_me should still be eliminated with -fwhole-program
> if you do
> 
> gcc -c t1.c
> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> 
> Thus, the resolution file probably does not have the information
> you need (a list of references from outside of the LTO file set).
> 
> Richard.

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Wed, Jun 9, 2010 at 5:14 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> Sent: 09 June 2010 16:02
>> To: Bingfeng Mei
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> necessary with -fwhole-program and resolution file.
>>
>> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>> > Hi,
>> > This patch addresses issue discussed in
>> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
>> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
>> >
>> > With the patch, any declaration which is resolved as
>> LDPR_PREVAILING_DEF
>> > and compiled with -fwhole-program is annotated with
>> > attribute "externally_visible" if it doesn't exist already.
>> > This eliminates the error-prone process of manual annotation
>> > of the attribute when compiling mixed LTO/non-LTO applications.
>> >
>> > For the following test files:
>> > a.c
>> >
>> > #include <string.h>
>> > #include <stdio.h>
>> > extern int foo(int);
>> > /* Called by b.c, should not be optimized by -fwhole-program */
>> > void bar()
>> > {
>> >  printf("bar\n");
>> > }
>> > /* Not used by others, should be optimized out by -fwhole-program*/
>> > void bar2()
>> > {
>> >  printf("bar2\n");
>> > }
>> > extern int src[], dst[];
>> > int vvvvvv;
>> > int main()
>> > {
>> >  int ret;
>> >  vvvvvv = 12;
>> >  ret = foo(20);
>> >  bar2();
>> >  memcpy(dst, src, 100);
>> >  return ret + 3;
>> > }
>> >
>> > b.c
>> >
>> > #include <stdio.h>
>> > int src[100];
>> > int dst[100];
>> > extern int vvvvvv;
>> > extern void bar();
>> > int foo(int c)
>> > {
>> >  printf("Hello world: %d\n", c);
>> >  bar();
>> >  return 1000 + vvvvvv;
>> > }
>> >
>> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
>> > ~/work/install-x86/bin/gcc  b.c -O2 -c
>> > ar cru libb.a b.o
>> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-plugin
>> -o f -fwhole-program
>> >
>> > The code is compiled and linked correctly. bar & vvvvvv don't become
>> static function
>> > and cause link errors, whereas bar2 is inlined as expected.
>> >
>> >
>> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >
>> > Cheers,
>> > Bingfeng Mei
>> >
>> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
>> >
>> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
>> externally_visible
>> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> compiling with
>> >        -fwhole-program
>> >
>> >
>> > Index: lto-symtab.c
>> > ===================================================================
>> > --- lto-symtab.c        (revision 160463)
>> > +++ lto-symtab.c        (working copy)
>> > @@ -476,7 +476,19 @@
>> >
>> >   /* If the chain is already resolved there is nothing else to
>> do.  */
>> >   if (e->resolution != LDPR_UNKNOWN)
>> > -    return;
>> > +    {
>> > +      /* Add externally_visible attribute for declaration of
>> LDPR_PREVAILING_DEF */
>> > +      if (e->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
>> > +        {
>> > +          if (!lookup_attribute ("externally_visible",
>> DECL_ATTRIBUTES (e->decl)))
>> > +            {
>> > +              DECL_ATTRIBUTES (e->decl)
>> > +                = tree_cons (get_identifier ("externally_visible"),
>> NULL_TREE,
>> > +                             DECL_ATTRIBUTES (e->decl));
>>
>> I don't think we need to add an attribute here but we can play
>> with some cgraph flags which is cheaper.
>
> Then I also need to change relevant flag for variable declaration. Not sure what
> I should change.
>
>>
>> Also I think this isn't really correct - not everything that prevails
>> needs to be externally visible (in fact, you seem to simply
>> remove the effect of -fwhole-program completely).
>
> According to doc, LDPR_PREVAILING_DEF is the definition accessed outside
> of LTO IR as opposed to LDPR_PREVAILING_DEF_IRONLY. So I think it should
> be externally visible. In my test file, bar2 is the similar as eliminate_me
> in your testcase. It is correctly eliminated with -fwhole-program.

Oh, I see.  (where's that doc btw?)

I guess the internal resolver should then be updated to use
IRONLY and applying external visibility should be done at
the end of lto_symtab_merge_decls_1 then.

Honza - what's the cgraph/varpool way of marking sth externally
visible?

Richard.



>>
>> A testcase that should still work is
>>
>> t1.c:
>> void foo(void) { bar (); }
>> t2.c
>> extern void foo (void);
>> void bar (void) {}
>> void eliminate_me (void) {}
>> int main()
>> {
>>    foo();
>> }
>>
>> and eliminate_me should still be eliminated with -fwhole-program
>> if you do
>>
>> gcc -c t1.c
>> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
>>
>> Thus, the resolution file probably does not have the information
>> you need (a list of references from outside of the LTO file set).
>>
>> Richard.
>
>
>

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 09 June 2010 16:25
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Wed, Jun 9, 2010 at 5:14 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 09 June 2010 16:02
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> >> > Hi,
> >> > This patch addresses issue discussed in
> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >
> >> > With the patch, any declaration which is resolved as
> >> LDPR_PREVAILING_DEF
> >> > and compiled with -fwhole-program is annotated with
> >> > attribute "externally_visible" if it doesn't exist already.
> >> > This eliminates the error-prone process of manual annotation
> >> > of the attribute when compiling mixed LTO/non-LTO applications.
> >> >
> >> > For the following test files:
> >> > a.c
> >> >
> >> > #include <string.h>
> >> > #include <stdio.h>
> >> > extern int foo(int);
> >> > /* Called by b.c, should not be optimized by -fwhole-program */
> >> > void bar()
> >> > {
> >> >  printf("bar\n");
> >> > }
> >> > /* Not used by others, should be optimized out by -fwhole-
> program*/
> >> > void bar2()
> >> > {
> >> >  printf("bar2\n");
> >> > }
> >> > extern int src[], dst[];
> >> > int vvvvvv;
> >> > int main()
> >> > {
> >> >  int ret;
> >> >  vvvvvv = 12;
> >> >  ret = foo(20);
> >> >  bar2();
> >> >  memcpy(dst, src, 100);
> >> >  return ret + 3;
> >> > }
> >> >
> >> > b.c
> >> >
> >> > #include <stdio.h>
> >> > int src[100];
> >> > int dst[100];
> >> > extern int vvvvvv;
> >> > extern void bar();
> >> > int foo(int c)
> >> > {
> >> >  printf("Hello world: %d\n", c);
> >> >  bar();
> >> >  return 1000 + vvvvvv;
> >> > }
> >> >
> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> > ar cru libb.a b.o
> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
> plugin
> >> -o f -fwhole-program
> >> >
> >> > The code is compiled and linked correctly. bar & vvvvvv don't
> become
> >> static function
> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >
> >> >
> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >
> >> > Cheers,
> >> > Bingfeng Mei
> >> >
> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >
> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> externally_visible
> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> compiling with
> >> >        -fwhole-program
> >> >
> >> >
> >> > Index: lto-symtab.c
> >> >
> ===================================================================
> >> > --- lto-symtab.c        (revision 160463)
> >> > +++ lto-symtab.c        (working copy)
> >> > @@ -476,7 +476,19 @@
> >> >
> >> >   /* If the chain is already resolved there is nothing else to
> >> do.  */
> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> > -    return;
> >> > +    {
> >> > +      /* Add externally_visible attribute for declaration of
> >> LDPR_PREVAILING_DEF */
> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> flag_whole_program)
> >> > +        {
> >> > +          if (!lookup_attribute ("externally_visible",
> >> DECL_ATTRIBUTES (e->decl)))
> >> > +            {
> >> > +              DECL_ATTRIBUTES (e->decl)
> >> > +                = tree_cons (get_identifier
> ("externally_visible"),
> >> NULL_TREE,
> >> > +                             DECL_ATTRIBUTES (e->decl));
> >>
> >> I don't think we need to add an attribute here but we can play
> >> with some cgraph flags which is cheaper.
> >
> > Then I also need to change relevant flag for variable declaration.
> Not sure what
> > I should change.
> >
> >>
> >> Also I think this isn't really correct - not everything that
> prevails
> >> needs to be externally visible (in fact, you seem to simply
> >> remove the effect of -fwhole-program completely).
> >
> > According to doc, LDPR_PREVAILING_DEF is the definition accessed
> outside
> > of LTO IR as opposed to LDPR_PREVAILING_DEF_IRONLY. So I think it
> should
> > be externally visible. In my test file, bar2 is the similar as
> eliminate_me
> > in your testcase. It is correctly eliminated with -fwhole-program.
> 
> Oh, I see.  (where's that doc btw?)

Actually comments in plugin-api.h in binutils
enum ld_plugin_symbol_resolution
{
  LDPR_UNKNOWN = 0,

  /* Symbol is still undefined at this point.  */
  LDPR_UNDEF,

  /* This is the prevailing definition of the symbol, with references from
     regular object code.  */
  LDPR_PREVAILING_DEF,

  /* This is the prevailing definition of the symbol, with no
     references from regular objects.  It is only referenced from IR
     code.  */
  LDPR_PREVAILING_DEF_IRONLY,

  /* This definition was pre-empted by a definition in a regular
     object file.  */
  LDPR_PREEMPTED_REG,

  /* This definition was pre-empted by a definition in another IR file.  */
  LDPR_PREEMPTED_IR,

  /* This symbol was resolved by a definition in another IR file.  */
  LDPR_RESOLVED_IR,

  /* This symbol was resolved by a definition in a regular object
     linked into the main executable.  */
  LDPR_RESOLVED_EXEC,

  /* This symbol was resolved by a definition in a shared object.  */
  LDPR_RESOLVED_DYN
};

> 
> I guess the internal resolver should then be updated to use
> IRONLY and applying external visibility should be done at
> the end of lto_symtab_merge_decls_1 then.
> 
> Honza - what's the cgraph/varpool way of marking sth externally
> visible?
> 
> Richard.
> 
> 
> 
> >>
> >> A testcase that should still work is
> >>
> >> t1.c:
> >> void foo(void) { bar (); }
> >> t2.c
> >> extern void foo (void);
> >> void bar (void) {}
> >> void eliminate_me (void) {}
> >> int main()
> >> {
> >>    foo();
> >> }
> >>
> >> and eliminate_me should still be eliminated with -fwhole-program
> >> if you do
> >>
> >> gcc -c t1.c
> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >>
> >> Thus, the resolution file probably does not have the information
> >> you need (a list of references from outside of the LTO file set).
> >>
> >> Richard.
> >
> >
> >

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

I added an attribute because -fwhole-program/externally_visible is handled in ipa.c

...
  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node->decl)))
    return true;
...

Adding attribute seems cleaner than changing flags, otherwise I need to change
handling in ipa.c as well.

Bingfeng

> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 09 June 2010 16:02
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> > Hi,
> > This patch addresses issue discussed in
> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >
> > With the patch, any declaration which is resolved as
> LDPR_PREVAILING_DEF
> > and compiled with -fwhole-program is annotated with
> > attribute "externally_visible" if it doesn't exist already.
> > This eliminates the error-prone process of manual annotation
> > of the attribute when compiling mixed LTO/non-LTO applications.
> >
> > For the following test files:
> > a.c
> >
> > #include <string.h>
> > #include <stdio.h>
> > extern int foo(int);
> > /* Called by b.c, should not be optimized by -fwhole-program */
> > void bar()
> > {
> >  printf("bar\n");
> > }
> > /* Not used by others, should be optimized out by -fwhole-program*/
> > void bar2()
> > {
> >  printf("bar2\n");
> > }
> > extern int src[], dst[];
> > int vvvvvv;
> > int main()
> > {
> >  int ret;
> >  vvvvvv = 12;
> >  ret = foo(20);
> >  bar2();
> >  memcpy(dst, src, 100);
> >  return ret + 3;
> > }
> >
> > b.c
> >
> > #include <stdio.h>
> > int src[100];
> > int dst[100];
> > extern int vvvvvv;
> > extern void bar();
> > int foo(int c)
> > {
> >  printf("Hello world: %d\n", c);
> >  bar();
> >  return 1000 + vvvvvv;
> > }
> >
> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> > ar cru libb.a b.o
> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-plugin
> -o f -fwhole-program
> >
> > The code is compiled and linked correctly. bar & vvvvvv don't become
> static function
> > and cause link errors, whereas bar2 is inlined as expected.
> >
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > Cheers,
> > Bingfeng Mei
> >
> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >
> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> externally_visible
> >        attribute for declaration of LDPR_PREVAILING_DEF when
> compiling with
> >        -fwhole-program
> >
> >
> > Index: lto-symtab.c
> > ===================================================================
> > --- lto-symtab.c        (revision 160463)
> > +++ lto-symtab.c        (working copy)
> > @@ -476,7 +476,19 @@
> >
> >   /* If the chain is already resolved there is nothing else to
> do.  */
> >   if (e->resolution != LDPR_UNKNOWN)
> > -    return;
> > +    {
> > +      /* Add externally_visible attribute for declaration of
> LDPR_PREVAILING_DEF */
> > +      if (e->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
> > +        {
> > +          if (!lookup_attribute ("externally_visible",
> DECL_ATTRIBUTES (e->decl)))
> > +            {
> > +              DECL_ATTRIBUTES (e->decl)
> > +                = tree_cons (get_identifier ("externally_visible"),
> NULL_TREE,
> > +                             DECL_ATTRIBUTES (e->decl));
> 
> I don't think we need to add an attribute here but we can play
> with some cgraph flags which is cheaper.
> 
> Also I think this isn't really correct - not everything that prevails
> needs to be externally visible (in fact, you seem to simply
> remove the effect of -fwhole-program completely).
> 
> A testcase that should still work is
> 
> t1.c:
> void foo(void) { bar (); }
> t2.c
> extern void foo (void);
> void bar (void) {}
> void eliminate_me (void) {}
> int main()
> {
>    foo();
> }
> 
> and eliminate_me should still be eliminated with -fwhole-program
> if you do
> 
> gcc -c t1.c
> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> 
> Thus, the resolution file probably does not have the information
> you need (a list of references from outside of the LTO file set).
> 
> Richard.

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> I added an attribute because -fwhole-program/externally_visible is handled in ipa.c
>
> ...
>  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node->decl)))
>    return true;
> ...
>
> Adding attribute seems cleaner than changing flags, otherwise I need to change
> handling in ipa.c as well.

True, but there is an externally_visible flag in cgraph_node,
so I guess that attribute lookup is bogus.

Richard.

> Bingfeng
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> Sent: 09 June 2010 16:02
>> To: Bingfeng Mei
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> necessary with -fwhole-program and resolution file.
>>
>> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>> > Hi,
>> > This patch addresses issue discussed in
>> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
>> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
>> >
>> > With the patch, any declaration which is resolved as
>> LDPR_PREVAILING_DEF
>> > and compiled with -fwhole-program is annotated with
>> > attribute "externally_visible" if it doesn't exist already.
>> > This eliminates the error-prone process of manual annotation
>> > of the attribute when compiling mixed LTO/non-LTO applications.
>> >
>> > For the following test files:
>> > a.c
>> >
>> > #include <string.h>
>> > #include <stdio.h>
>> > extern int foo(int);
>> > /* Called by b.c, should not be optimized by -fwhole-program */
>> > void bar()
>> > {
>> >  printf("bar\n");
>> > }
>> > /* Not used by others, should be optimized out by -fwhole-program*/
>> > void bar2()
>> > {
>> >  printf("bar2\n");
>> > }
>> > extern int src[], dst[];
>> > int vvvvvv;
>> > int main()
>> > {
>> >  int ret;
>> >  vvvvvv = 12;
>> >  ret = foo(20);
>> >  bar2();
>> >  memcpy(dst, src, 100);
>> >  return ret + 3;
>> > }
>> >
>> > b.c
>> >
>> > #include <stdio.h>
>> > int src[100];
>> > int dst[100];
>> > extern int vvvvvv;
>> > extern void bar();
>> > int foo(int c)
>> > {
>> >  printf("Hello world: %d\n", c);
>> >  bar();
>> >  return 1000 + vvvvvv;
>> > }
>> >
>> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
>> > ~/work/install-x86/bin/gcc  b.c -O2 -c
>> > ar cru libb.a b.o
>> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-plugin
>> -o f -fwhole-program
>> >
>> > The code is compiled and linked correctly. bar & vvvvvv don't become
>> static function
>> > and cause link errors, whereas bar2 is inlined as expected.
>> >
>> >
>> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >
>> > Cheers,
>> > Bingfeng Mei
>> >
>> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
>> >
>> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
>> externally_visible
>> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> compiling with
>> >        -fwhole-program
>> >
>> >
>> > Index: lto-symtab.c
>> > ===================================================================
>> > --- lto-symtab.c        (revision 160463)
>> > +++ lto-symtab.c        (working copy)
>> > @@ -476,7 +476,19 @@
>> >
>> >   /* If the chain is already resolved there is nothing else to
>> do.  */
>> >   if (e->resolution != LDPR_UNKNOWN)
>> > -    return;
>> > +    {
>> > +      /* Add externally_visible attribute for declaration of
>> LDPR_PREVAILING_DEF */
>> > +      if (e->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
>> > +        {
>> > +          if (!lookup_attribute ("externally_visible",
>> DECL_ATTRIBUTES (e->decl)))
>> > +            {
>> > +              DECL_ATTRIBUTES (e->decl)
>> > +                = tree_cons (get_identifier ("externally_visible"),
>> NULL_TREE,
>> > +                             DECL_ATTRIBUTES (e->decl));
>>
>> I don't think we need to add an attribute here but we can play
>> with some cgraph flags which is cheaper.
>>
>> Also I think this isn't really correct - not everything that prevails
>> needs to be externally visible (in fact, you seem to simply
>> remove the effect of -fwhole-program completely).
>>
>> A testcase that should still work is
>>
>> t1.c:
>> void foo(void) { bar (); }
>> t2.c
>> extern void foo (void);
>> void bar (void) {}
>> void eliminate_me (void) {}
>> int main()
>> {
>>    foo();
>> }
>>
>> and eliminate_me should still be eliminated with -fwhole-program
>> if you do
>>
>> gcc -c t1.c
>> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
>>
>> Thus, the resolution file probably does not have the information
>> you need (a list of references from outside of the LTO file set).
>>
>> Richard.
>
>
>

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

Richard,

I tried to set externally_visible flags directly for cgraph node & varbool node.
Unfortunately, these flags seem to be set and used elsewhere too. The values
set in lto-symtab.c cannot be reliably passed to ipa.c (function_and_variable_visibility),
where the flags are actually re-computed.
 
      if (cgraph_externally_visible_p (node, whole_program))
        {
	  gcc_assert (!node->global.inlined_to);
	  node->local.externally_visible = true;
	}
      else
	node->local.externally_visible = false;

The flag for bar2 function is true before this piece of code even I doesn't set
it in lto-symtab.c. 

For now, I think attribute is still cleaner solution though
not cheap. The following is the updated patch.

Cheers,
Bingfeng

2010-06-10  Bingfeng Mei <bmei@broadcom.com>

	* lto-symbtab.c (lto_symtab_merge_decls_1): Add externally_visible
	attribute for declaration of LDPR_PREVAILING_DEF when compiling with
	-fwhole-program 
	(lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for 
	internal resolver
        
Index: lto-symtab.c
===================================================================
--- lto-symtab.c        (revision 160463)
+++ lto-symtab.c        (working copy)
@@ -530,11 +530,15 @@
     return;

 found:
-  if (TREE_CODE (prevailing->decl) == VAR_DECL
-      && TREE_READONLY (prevailing->decl))
-    prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
-  else
-    prevailing->resolution = LDPR_PREVAILING_DEF;
+  /* If current lto files represent the whole program,
+    it is correct to use LDPR_PREVALING_DEF_IRONLY.
+    If current lto files are part of whole program, internal
+    resolver doesn't know if it is LDPR_PREVAILING_DEF
+    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
+    using -fwhole-program. Otherwise, it doesn't
+    matter using either LDPR_PREVAILING_DEF or
+    LDPR_PREVAILING_DEF_IRONLY */
+  prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
 }

 /* Merge all decls in the symbol table chain to the prevailing decl and
@@ -698,6 +702,18 @@
       && TREE_CODE (prevailing->decl) != VAR_DECL)
     prevailing->next = NULL;

+
+  /* Add externally_visible attribute for declaration of LDPR_PREVAILING_DEF */
+  if (prevailing->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
+    {
+      if (!lookup_attribute ("externally_visible",
+                             DECL_ATTRIBUTES (prevailing->decl)))
+        {
+          DECL_ATTRIBUTES (prevailing->decl)
+            = tree_cons (get_identifier ("externally_visible"), NULL_TREE,
+                         DECL_ATTRIBUTES (prevailing->decl));
+        }
+    }
   return 1;
 }

> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 09 June 2010 16:26
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> > I added an attribute because -fwhole-program/externally_visible is
> handled in ipa.c
> >
> > ...
> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node-
> >decl)))
> >    return true;
> > ...
> >
> > Adding attribute seems cleaner than changing flags, otherwise I need
> to change
> > handling in ipa.c as well.
> 
> True, but there is an externally_visible flag in cgraph_node,
> so I guess that attribute lookup is bogus.
> 
> Richard.
> 
> > Bingfeng
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 09 June 2010 16:02
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> >> > Hi,
> >> > This patch addresses issue discussed in
> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >
> >> > With the patch, any declaration which is resolved as
> >> LDPR_PREVAILING_DEF
> >> > and compiled with -fwhole-program is annotated with
> >> > attribute "externally_visible" if it doesn't exist already.
> >> > This eliminates the error-prone process of manual annotation
> >> > of the attribute when compiling mixed LTO/non-LTO applications.
> >> >
> >> > For the following test files:
> >> > a.c
> >> >
> >> > #include <string.h>
> >> > #include <stdio.h>
> >> > extern int foo(int);
> >> > /* Called by b.c, should not be optimized by -fwhole-program */
> >> > void bar()
> >> > {
> >> >  printf("bar\n");
> >> > }
> >> > /* Not used by others, should be optimized out by -fwhole-
> program*/
> >> > void bar2()
> >> > {
> >> >  printf("bar2\n");
> >> > }
> >> > extern int src[], dst[];
> >> > int vvvvvv;
> >> > int main()
> >> > {
> >> >  int ret;
> >> >  vvvvvv = 12;
> >> >  ret = foo(20);
> >> >  bar2();
> >> >  memcpy(dst, src, 100);
> >> >  return ret + 3;
> >> > }
> >> >
> >> > b.c
> >> >
> >> > #include <stdio.h>
> >> > int src[100];
> >> > int dst[100];
> >> > extern int vvvvvv;
> >> > extern void bar();
> >> > int foo(int c)
> >> > {
> >> >  printf("Hello world: %d\n", c);
> >> >  bar();
> >> >  return 1000 + vvvvvv;
> >> > }
> >> >
> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> > ar cru libb.a b.o
> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
> plugin
> >> -o f -fwhole-program
> >> >
> >> > The code is compiled and linked correctly. bar & vvvvvv don't
> become
> >> static function
> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >
> >> >
> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >
> >> > Cheers,
> >> > Bingfeng Mei
> >> >
> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >
> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> externally_visible
> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> compiling with
> >> >        -fwhole-program
> >> >
> >> >
> >> > Index: lto-symtab.c
> >> >
> ===================================================================
> >> > --- lto-symtab.c        (revision 160463)
> >> > +++ lto-symtab.c        (working copy)
> >> > @@ -476,7 +476,19 @@
> >> >
> >> >   /* If the chain is already resolved there is nothing else to
> >> do.  */
> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> > -    return;
> >> > +    {
> >> > +      /* Add externally_visible attribute for declaration of
> >> LDPR_PREVAILING_DEF */
> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> flag_whole_program)
> >> > +        {
> >> > +          if (!lookup_attribute ("externally_visible",
> >> DECL_ATTRIBUTES (e->decl)))
> >> > +            {
> >> > +              DECL_ATTRIBUTES (e->decl)
> >> > +                = tree_cons (get_identifier
> ("externally_visible"),
> >> NULL_TREE,
> >> > +                             DECL_ATTRIBUTES (e->decl));
> >>
> >> I don't think we need to add an attribute here but we can play
> >> with some cgraph flags which is cheaper.
> >>
> >> Also I think this isn't really correct - not everything that
> prevails
> >> needs to be externally visible (in fact, you seem to simply
> >> remove the effect of -fwhole-program completely).
> >>
> >> A testcase that should still work is
> >>
> >> t1.c:
> >> void foo(void) { bar (); }
> >> t2.c
> >> extern void foo (void);
> >> void bar (void) {}
> >> void eliminate_me (void) {}
> >> int main()
> >> {
> >>    foo();
> >> }
> >>
> >> and eliminate_me should still be eliminated with -fwhole-program
> >> if you do
> >>
> >> gcc -c t1.c
> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >>
> >> Thus, the resolution file probably does not have the information
> >> you need (a list of references from outside of the LTO file set).
> >>
> >> Richard.
> >
> >
> >

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Thu, Jun 10, 2010 at 12:03 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Richard,
>
> I tried to set externally_visible flags directly for cgraph node & varbool node.
> Unfortunately, these flags seem to be set and used elsewhere too. The values
> set in lto-symtab.c cannot be reliably passed to ipa.c (function_and_variable_visibility),
> where the flags are actually re-computed.
>
>      if (cgraph_externally_visible_p (node, whole_program))
>        {
>          gcc_assert (!node->global.inlined_to);
>          node->local.externally_visible = true;
>        }
>      else
>        node->local.externally_visible = false;
>
> The flag for bar2 function is true before this piece of code even I doesn't set
> it in lto-symtab.c.

I believe this is an error - Honza knows the code and will probably
comment.

> For now, I think attribute is still cleaner solution though
> not cheap. The following is the updated patch.

Thanks - I believe this is valuable but want to hear comments
from Honza as we shouldn't need to use attributes here.

Richard.

> Cheers,
> Bingfeng
>
> 2010-06-10  Bingfeng Mei <bmei@broadcom.com>
>
>        * lto-symbtab.c (lto_symtab_merge_decls_1): Add externally_visible
>        attribute for declaration of LDPR_PREVAILING_DEF when compiling with
>        -fwhole-program
>        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for
>        internal resolver
>
> Index: lto-symtab.c
> ===================================================================
> --- lto-symtab.c        (revision 160463)
> +++ lto-symtab.c        (working copy)
> @@ -530,11 +530,15 @@
>     return;
>
>  found:
> -  if (TREE_CODE (prevailing->decl) == VAR_DECL
> -      && TREE_READONLY (prevailing->decl))
> -    prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> -  else
> -    prevailing->resolution = LDPR_PREVAILING_DEF;
> +  /* If current lto files represent the whole program,
> +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
> +    If current lto files are part of whole program, internal
> +    resolver doesn't know if it is LDPR_PREVAILING_DEF
> +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
> +    using -fwhole-program. Otherwise, it doesn't
> +    matter using either LDPR_PREVAILING_DEF or
> +    LDPR_PREVAILING_DEF_IRONLY */
> +  prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
>  }
>
>  /* Merge all decls in the symbol table chain to the prevailing decl and
> @@ -698,6 +702,18 @@
>       && TREE_CODE (prevailing->decl) != VAR_DECL)
>     prevailing->next = NULL;
>
> +
> +  /* Add externally_visible attribute for declaration of LDPR_PREVAILING_DEF */
> +  if (prevailing->resolution == LDPR_PREVAILING_DEF && flag_whole_program)
> +    {
> +      if (!lookup_attribute ("externally_visible",
> +                             DECL_ATTRIBUTES (prevailing->decl)))
> +        {
> +          DECL_ATTRIBUTES (prevailing->decl)
> +            = tree_cons (get_identifier ("externally_visible"), NULL_TREE,
> +                         DECL_ATTRIBUTES (prevailing->decl));
> +        }
> +    }
>   return 1;
>  }
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> Sent: 09 June 2010 16:26
>> To: Bingfeng Mei
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> necessary with -fwhole-program and resolution file.
>>
>> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>> > I added an attribute because -fwhole-program/externally_visible is
>> handled in ipa.c
>> >
>> > ...
>> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node-
>> >decl)))
>> >    return true;
>> > ...
>> >
>> > Adding attribute seems cleaner than changing flags, otherwise I need
>> to change
>> > handling in ipa.c as well.
>>
>> True, but there is an externally_visible flag in cgraph_node,
>> so I guess that attribute lookup is bogus.
>>
>> Richard.
>>
>> > Bingfeng
>> >
>> >> -----Original Message-----
>> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> >> Sent: 09 June 2010 16:02
>> >> To: Bingfeng Mei
>> >> Cc: gcc-patches@gcc.gnu.org
>> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> >> necessary with -fwhole-program and resolution file.
>> >>
>> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
>> wrote:
>> >> > Hi,
>> >> > This patch addresses issue discussed in
>> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
>> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
>> >> >
>> >> > With the patch, any declaration which is resolved as
>> >> LDPR_PREVAILING_DEF
>> >> > and compiled with -fwhole-program is annotated with
>> >> > attribute "externally_visible" if it doesn't exist already.
>> >> > This eliminates the error-prone process of manual annotation
>> >> > of the attribute when compiling mixed LTO/non-LTO applications.
>> >> >
>> >> > For the following test files:
>> >> > a.c
>> >> >
>> >> > #include <string.h>
>> >> > #include <stdio.h>
>> >> > extern int foo(int);
>> >> > /* Called by b.c, should not be optimized by -fwhole-program */
>> >> > void bar()
>> >> > {
>> >> >  printf("bar\n");
>> >> > }
>> >> > /* Not used by others, should be optimized out by -fwhole-
>> program*/
>> >> > void bar2()
>> >> > {
>> >> >  printf("bar2\n");
>> >> > }
>> >> > extern int src[], dst[];
>> >> > int vvvvvv;
>> >> > int main()
>> >> > {
>> >> >  int ret;
>> >> >  vvvvvv = 12;
>> >> >  ret = foo(20);
>> >> >  bar2();
>> >> >  memcpy(dst, src, 100);
>> >> >  return ret + 3;
>> >> > }
>> >> >
>> >> > b.c
>> >> >
>> >> > #include <stdio.h>
>> >> > int src[100];
>> >> > int dst[100];
>> >> > extern int vvvvvv;
>> >> > extern void bar();
>> >> > int foo(int c)
>> >> > {
>> >> >  printf("Hello world: %d\n", c);
>> >> >  bar();
>> >> >  return 1000 + vvvvvv;
>> >> > }
>> >> >
>> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
>> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
>> >> > ar cru libb.a b.o
>> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
>> plugin
>> >> -o f -fwhole-program
>> >> >
>> >> > The code is compiled and linked correctly. bar & vvvvvv don't
>> become
>> >> static function
>> >> > and cause link errors, whereas bar2 is inlined as expected.
>> >> >
>> >> >
>> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >> >
>> >> > Cheers,
>> >> > Bingfeng Mei
>> >> >
>> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
>> >> >
>> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
>> >> externally_visible
>> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> >> compiling with
>> >> >        -fwhole-program
>> >> >
>> >> >
>> >> > Index: lto-symtab.c
>> >> >
>> ===================================================================
>> >> > --- lto-symtab.c        (revision 160463)
>> >> > +++ lto-symtab.c        (working copy)
>> >> > @@ -476,7 +476,19 @@
>> >> >
>> >> >   /* If the chain is already resolved there is nothing else to
>> >> do.  */
>> >> >   if (e->resolution != LDPR_UNKNOWN)
>> >> > -    return;
>> >> > +    {
>> >> > +      /* Add externally_visible attribute for declaration of
>> >> LDPR_PREVAILING_DEF */
>> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
>> flag_whole_program)
>> >> > +        {
>> >> > +          if (!lookup_attribute ("externally_visible",
>> >> DECL_ATTRIBUTES (e->decl)))
>> >> > +            {
>> >> > +              DECL_ATTRIBUTES (e->decl)
>> >> > +                = tree_cons (get_identifier
>> ("externally_visible"),
>> >> NULL_TREE,
>> >> > +                             DECL_ATTRIBUTES (e->decl));
>> >>
>> >> I don't think we need to add an attribute here but we can play
>> >> with some cgraph flags which is cheaper.
>> >>
>> >> Also I think this isn't really correct - not everything that
>> prevails
>> >> needs to be externally visible (in fact, you seem to simply
>> >> remove the effect of -fwhole-program completely).
>> >>
>> >> A testcase that should still work is
>> >>
>> >> t1.c:
>> >> void foo(void) { bar (); }
>> >> t2.c
>> >> extern void foo (void);
>> >> void bar (void) {}
>> >> void eliminate_me (void) {}
>> >> int main()
>> >> {
>> >>    foo();
>> >> }
>> >>
>> >> and eliminate_me should still be eliminated with -fwhole-program
>> >> if you do
>> >>
>> >> gcc -c t1.c
>> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
>> >>
>> >> Thus, the resolution file probably does not have the information
>> >> you need (a list of references from outside of the LTO file set).
>> >>
>> >> Richard.
>> >
>> >
>> >
>
>
>

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

Richard,
I figured out what happen with externally_visible flags in cgraph node/vnode.

In ipa.c (function_and_varialbe_visibility), the externally_visible flags are set to
true when compile with -flto because cgraph_externally_visible_p always returns true as
whole_program is false (regardless -fwhole-program option is set or not, which is 
always ignored in the first phase of LTO compilation). 

      if (cgraph_externally_visible_p (node, whole_program))
        {
	  gcc_assert (!node->global.inlined_to);
	  node->local.externally_visible = true;
	}

Then the flags are packed/unpacked from LTO byte code. That's why the externally_visible
flag is set to true for "bar2" function before executing my patch. 



Another issue is following code (your patch on 22/5). The vvvvvv
 variable in my example is going to be resolved by internal
 resolver as LDPR_PREVAILING_DEF_IRONLY instead of 
LDPR_PREVAILING_DEF by resolution file. Could you
Elaborate what is the exact issue here? 

	  /* The LTO plugin for gold doesn't handle common symbols
	     properly.  Let us choose manually.  */
	  if (DECL_COMMON (e->decl))
	    e->resolution = LDPR_UNKNOWN;

Thanks,
Bingfeng
> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 10 June 2010 11:12
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Thu, Jun 10, 2010 at 12:03 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> > Richard,
> >
> > I tried to set externally_visible flags directly for cgraph node &
> varbool node.
> > Unfortunately, these flags seem to be set and used elsewhere too. The
> values
> > set in lto-symtab.c cannot be reliably passed to ipa.c
> (function_and_variable_visibility),
> > where the flags are actually re-computed.
> >
> >      if (cgraph_externally_visible_p (node, whole_program))
> >        {
> >          gcc_assert (!node->global.inlined_to);
> >          node->local.externally_visible = true;
> >        }
> >      else
> >        node->local.externally_visible = false;
> >
> > The flag for bar2 function is true before this piece of code even I
> doesn't set
> > it in lto-symtab.c.
> 
> I believe this is an error - Honza knows the code and will probably
> comment.
> 
> > For now, I think attribute is still cleaner solution though
> > not cheap. The following is the updated patch.
> 
> Thanks - I believe this is valuable but want to hear comments
> from Honza as we shouldn't need to use attributes here.
> 
> Richard.
> 
> > Cheers,
> > Bingfeng
> >
> > 2010-06-10  Bingfeng Mei <bmei@broadcom.com>
> >
> >        * lto-symbtab.c (lto_symtab_merge_decls_1): Add
> externally_visible
> >        attribute for declaration of LDPR_PREVAILING_DEF when
> compiling with
> >        -fwhole-program
> >        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
> for
> >        internal resolver
> >
> > Index: lto-symtab.c
> > ===================================================================
> > --- lto-symtab.c        (revision 160463)
> > +++ lto-symtab.c        (working copy)
> > @@ -530,11 +530,15 @@
> >     return;
> >
> >  found:
> > -  if (TREE_CODE (prevailing->decl) == VAR_DECL
> > -      && TREE_READONLY (prevailing->decl))
> > -    prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> > -  else
> > -    prevailing->resolution = LDPR_PREVAILING_DEF;
> > +  /* If current lto files represent the whole program,
> > +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
> > +    If current lto files are part of whole program, internal
> > +    resolver doesn't know if it is LDPR_PREVAILING_DEF
> > +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
> > +    using -fwhole-program. Otherwise, it doesn't
> > +    matter using either LDPR_PREVAILING_DEF or
> > +    LDPR_PREVAILING_DEF_IRONLY */
> > +  prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> >  }
> >
> >  /* Merge all decls in the symbol table chain to the prevailing decl
> and
> > @@ -698,6 +702,18 @@
> >       && TREE_CODE (prevailing->decl) != VAR_DECL)
> >     prevailing->next = NULL;
> >
> > +
> > +  /* Add externally_visible attribute for declaration of
> LDPR_PREVAILING_DEF */
> > +  if (prevailing->resolution == LDPR_PREVAILING_DEF &&
> flag_whole_program)
> > +    {
> > +      if (!lookup_attribute ("externally_visible",
> > +                             DECL_ATTRIBUTES (prevailing->decl)))
> > +        {
> > +          DECL_ATTRIBUTES (prevailing->decl)
> > +            = tree_cons (get_identifier ("externally_visible"),
> NULL_TREE,
> > +                         DECL_ATTRIBUTES (prevailing->decl));
> > +        }
> > +    }
> >   return 1;
> >  }
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 09 June 2010 16:26
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> >> > I added an attribute because -fwhole-program/externally_visible is
> >> handled in ipa.c
> >> >
> >> > ...
> >> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES
> (node-
> >> >decl)))
> >> >    return true;
> >> > ...
> >> >
> >> > Adding attribute seems cleaner than changing flags, otherwise I
> need
> >> to change
> >> > handling in ipa.c as well.
> >>
> >> True, but there is an externally_visible flag in cgraph_node,
> >> so I guess that attribute lookup is bogus.
> >>
> >> Richard.
> >>
> >> > Bingfeng
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> >> Sent: 09 June 2010 16:02
> >> >> To: Bingfeng Mei
> >> >> Cc: gcc-patches@gcc.gnu.org
> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> >> necessary with -fwhole-program and resolution file.
> >> >>
> >> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
> >> wrote:
> >> >> > Hi,
> >> >> > This patch addresses issue discussed in
> >> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >> >
> >> >> > With the patch, any declaration which is resolved as
> >> >> LDPR_PREVAILING_DEF
> >> >> > and compiled with -fwhole-program is annotated with
> >> >> > attribute "externally_visible" if it doesn't exist already.
> >> >> > This eliminates the error-prone process of manual annotation
> >> >> > of the attribute when compiling mixed LTO/non-LTO applications.
> >> >> >
> >> >> > For the following test files:
> >> >> > a.c
> >> >> >
> >> >> > #include <string.h>
> >> >> > #include <stdio.h>
> >> >> > extern int foo(int);
> >> >> > /* Called by b.c, should not be optimized by -fwhole-program */
> >> >> > void bar()
> >> >> > {
> >> >> >  printf("bar\n");
> >> >> > }
> >> >> > /* Not used by others, should be optimized out by -fwhole-
> >> program*/
> >> >> > void bar2()
> >> >> > {
> >> >> >  printf("bar2\n");
> >> >> > }
> >> >> > extern int src[], dst[];
> >> >> > int vvvvvv;
> >> >> > int main()
> >> >> > {
> >> >> >  int ret;
> >> >> >  vvvvvv = 12;
> >> >> >  ret = foo(20);
> >> >> >  bar2();
> >> >> >  memcpy(dst, src, 100);
> >> >> >  return ret + 3;
> >> >> > }
> >> >> >
> >> >> > b.c
> >> >> >
> >> >> > #include <stdio.h>
> >> >> > int src[100];
> >> >> > int dst[100];
> >> >> > extern int vvvvvv;
> >> >> > extern void bar();
> >> >> > int foo(int c)
> >> >> > {
> >> >> >  printf("Hello world: %d\n", c);
> >> >> >  bar();
> >> >> >  return 1000 + vvvvvv;
> >> >> > }
> >> >> >
> >> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> >> > ar cru libb.a b.o
> >> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
> >> plugin
> >> >> -o f -fwhole-program
> >> >> >
> >> >> > The code is compiled and linked correctly. bar & vvvvvv don't
> >> become
> >> >> static function
> >> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >> >
> >> >> >
> >> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >> >
> >> >> > Cheers,
> >> >> > Bingfeng Mei
> >> >> >
> >> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >> >
> >> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> >> externally_visible
> >> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> >> compiling with
> >> >> >        -fwhole-program
> >> >> >
> >> >> >
> >> >> > Index: lto-symtab.c
> >> >> >
> >> ===================================================================
> >> >> > --- lto-symtab.c        (revision 160463)
> >> >> > +++ lto-symtab.c        (working copy)
> >> >> > @@ -476,7 +476,19 @@
> >> >> >
> >> >> >   /* If the chain is already resolved there is nothing else to
> >> >> do.  */
> >> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> >> > -    return;
> >> >> > +    {
> >> >> > +      /* Add externally_visible attribute for declaration of
> >> >> LDPR_PREVAILING_DEF */
> >> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> >> flag_whole_program)
> >> >> > +        {
> >> >> > +          if (!lookup_attribute ("externally_visible",
> >> >> DECL_ATTRIBUTES (e->decl)))
> >> >> > +            {
> >> >> > +              DECL_ATTRIBUTES (e->decl)
> >> >> > +                = tree_cons (get_identifier
> >> ("externally_visible"),
> >> >> NULL_TREE,
> >> >> > +                             DECL_ATTRIBUTES (e->decl));
> >> >>
> >> >> I don't think we need to add an attribute here but we can play
> >> >> with some cgraph flags which is cheaper.
> >> >>
> >> >> Also I think this isn't really correct - not everything that
> >> prevails
> >> >> needs to be externally visible (in fact, you seem to simply
> >> >> remove the effect of -fwhole-program completely).
> >> >>
> >> >> A testcase that should still work is
> >> >>
> >> >> t1.c:
> >> >> void foo(void) { bar (); }
> >> >> t2.c
> >> >> extern void foo (void);
> >> >> void bar (void) {}
> >> >> void eliminate_me (void) {}
> >> >> int main()
> >> >> {
> >> >>    foo();
> >> >> }
> >> >>
> >> >> and eliminate_me should still be eliminated with -fwhole-program
> >> >> if you do
> >> >>
> >> >> gcc -c t1.c
> >> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >> >>
> >> >> Thus, the resolution file probably does not have the information
> >> >> you need (a list of references from outside of the LTO file set).
> >> >>
> >> >> Richard.
> >> >
> >> >
> >> >
> >
> >
> >

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Thu, Jun 10, 2010 at 4:51 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Richard,
> I figured out what happen with externally_visible flags in cgraph node/vnode.
>
> In ipa.c (function_and_varialbe_visibility), the externally_visible flags are set to
> true when compile with -flto because cgraph_externally_visible_p always returns true as
> whole_program is false (regardless -fwhole-program option is set or not, which is
> always ignored in the first phase of LTO compilation).
>
>      if (cgraph_externally_visible_p (node, whole_program))
>        {
>          gcc_assert (!node->global.inlined_to);
>          node->local.externally_visible = true;
>        }
>
> Then the flags are packed/unpacked from LTO byte code. That's why the externally_visible
> flag is set to true for "bar2" function before executing my patch.
>
>
>
> Another issue is following code (your patch on 22/5). The vvvvvv
>  variable in my example is going to be resolved by internal
>  resolver as LDPR_PREVAILING_DEF_IRONLY instead of
> LDPR_PREVAILING_DEF by resolution file. Could you
> Elaborate what is the exact issue here?

Gold does not keep track of which symbol from which object
file it will end up using for commons but instead simply picks
the first one.  We rely on the fact that we get the one with
the largest size - with gold we even can end up with
a non-prevailing one.  And there's some version of gold which doesn't
resolve commons at all.

So we need to run through our own resolution code here.

An example is

extern int i;
--
int i;

where gold can claim that 'extern int i' was prevailing.

>          /* The LTO plugin for gold doesn't handle common symbols
>             properly.  Let us choose manually.  */
>          if (DECL_COMMON (e->decl))
>            e->resolution = LDPR_UNKNOWN;
>
> Thanks,
> Bingfeng
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> Sent: 10 June 2010 11:12
>> To: Bingfeng Mei
>> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
>> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> necessary with -fwhole-program and resolution file.
>>
>> On Thu, Jun 10, 2010 at 12:03 PM, Bingfeng Mei <bmei@broadcom.com>
>> wrote:
>> > Richard,
>> >
>> > I tried to set externally_visible flags directly for cgraph node &
>> varbool node.
>> > Unfortunately, these flags seem to be set and used elsewhere too. The
>> values
>> > set in lto-symtab.c cannot be reliably passed to ipa.c
>> (function_and_variable_visibility),
>> > where the flags are actually re-computed.
>> >
>> >      if (cgraph_externally_visible_p (node, whole_program))
>> >        {
>> >          gcc_assert (!node->global.inlined_to);
>> >          node->local.externally_visible = true;
>> >        }
>> >      else
>> >        node->local.externally_visible = false;
>> >
>> > The flag for bar2 function is true before this piece of code even I
>> doesn't set
>> > it in lto-symtab.c.
>>
>> I believe this is an error - Honza knows the code and will probably
>> comment.
>>
>> > For now, I think attribute is still cleaner solution though
>> > not cheap. The following is the updated patch.
>>
>> Thanks - I believe this is valuable but want to hear comments
>> from Honza as we shouldn't need to use attributes here.
>>
>> Richard.
>>
>> > Cheers,
>> > Bingfeng
>> >
>> > 2010-06-10  Bingfeng Mei <bmei@broadcom.com>
>> >
>> >        * lto-symbtab.c (lto_symtab_merge_decls_1): Add
>> externally_visible
>> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> compiling with
>> >        -fwhole-program
>> >        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
>> for
>> >        internal resolver
>> >
>> > Index: lto-symtab.c
>> > ===================================================================
>> > --- lto-symtab.c        (revision 160463)
>> > +++ lto-symtab.c        (working copy)
>> > @@ -530,11 +530,15 @@
>> >     return;
>> >
>> >  found:
>> > -  if (TREE_CODE (prevailing->decl) == VAR_DECL
>> > -      && TREE_READONLY (prevailing->decl))
>> > -    prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
>> > -  else
>> > -    prevailing->resolution = LDPR_PREVAILING_DEF;
>> > +  /* If current lto files represent the whole program,
>> > +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
>> > +    If current lto files are part of whole program, internal
>> > +    resolver doesn't know if it is LDPR_PREVAILING_DEF
>> > +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
>> > +    using -fwhole-program. Otherwise, it doesn't
>> > +    matter using either LDPR_PREVAILING_DEF or
>> > +    LDPR_PREVAILING_DEF_IRONLY */
>> > +  prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
>> >  }
>> >
>> >  /* Merge all decls in the symbol table chain to the prevailing decl
>> and
>> > @@ -698,6 +702,18 @@
>> >       && TREE_CODE (prevailing->decl) != VAR_DECL)
>> >     prevailing->next = NULL;
>> >
>> > +
>> > +  /* Add externally_visible attribute for declaration of
>> LDPR_PREVAILING_DEF */
>> > +  if (prevailing->resolution == LDPR_PREVAILING_DEF &&
>> flag_whole_program)
>> > +    {
>> > +      if (!lookup_attribute ("externally_visible",
>> > +                             DECL_ATTRIBUTES (prevailing->decl)))
>> > +        {
>> > +          DECL_ATTRIBUTES (prevailing->decl)
>> > +            = tree_cons (get_identifier ("externally_visible"),
>> NULL_TREE,
>> > +                         DECL_ATTRIBUTES (prevailing->decl));
>> > +        }
>> > +    }
>> >   return 1;
>> >  }
>> >
>> >> -----Original Message-----
>> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> >> Sent: 09 June 2010 16:26
>> >> To: Bingfeng Mei
>> >> Cc: gcc-patches@gcc.gnu.org
>> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> >> necessary with -fwhole-program and resolution file.
>> >>
>> >> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com>
>> wrote:
>> >> > I added an attribute because -fwhole-program/externally_visible is
>> >> handled in ipa.c
>> >> >
>> >> > ...
>> >> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES
>> (node-
>> >> >decl)))
>> >> >    return true;
>> >> > ...
>> >> >
>> >> > Adding attribute seems cleaner than changing flags, otherwise I
>> need
>> >> to change
>> >> > handling in ipa.c as well.
>> >>
>> >> True, but there is an externally_visible flag in cgraph_node,
>> >> so I guess that attribute lookup is bogus.
>> >>
>> >> Richard.
>> >>
>> >> > Bingfeng
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> >> >> Sent: 09 June 2010 16:02
>> >> >> To: Bingfeng Mei
>> >> >> Cc: gcc-patches@gcc.gnu.org
>> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> >> >> necessary with -fwhole-program and resolution file.
>> >> >>
>> >> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
>> >> wrote:
>> >> >> > Hi,
>> >> >> > This patch addresses issue discussed in
>> >> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
>> >> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
>> >> >> >
>> >> >> > With the patch, any declaration which is resolved as
>> >> >> LDPR_PREVAILING_DEF
>> >> >> > and compiled with -fwhole-program is annotated with
>> >> >> > attribute "externally_visible" if it doesn't exist already.
>> >> >> > This eliminates the error-prone process of manual annotation
>> >> >> > of the attribute when compiling mixed LTO/non-LTO applications.
>> >> >> >
>> >> >> > For the following test files:
>> >> >> > a.c
>> >> >> >
>> >> >> > #include <string.h>
>> >> >> > #include <stdio.h>
>> >> >> > extern int foo(int);
>> >> >> > /* Called by b.c, should not be optimized by -fwhole-program */
>> >> >> > void bar()
>> >> >> > {
>> >> >> >  printf("bar\n");
>> >> >> > }
>> >> >> > /* Not used by others, should be optimized out by -fwhole-
>> >> program*/
>> >> >> > void bar2()
>> >> >> > {
>> >> >> >  printf("bar2\n");
>> >> >> > }
>> >> >> > extern int src[], dst[];
>> >> >> > int vvvvvv;
>> >> >> > int main()
>> >> >> > {
>> >> >> >  int ret;
>> >> >> >  vvvvvv = 12;
>> >> >> >  ret = foo(20);
>> >> >> >  bar2();
>> >> >> >  memcpy(dst, src, 100);
>> >> >> >  return ret + 3;
>> >> >> > }
>> >> >> >
>> >> >> > b.c
>> >> >> >
>> >> >> > #include <stdio.h>
>> >> >> > int src[100];
>> >> >> > int dst[100];
>> >> >> > extern int vvvvvv;
>> >> >> > extern void bar();
>> >> >> > int foo(int c)
>> >> >> > {
>> >> >> >  printf("Hello world: %d\n", c);
>> >> >> >  bar();
>> >> >> >  return 1000 + vvvvvv;
>> >> >> > }
>> >> >> >
>> >> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
>> >> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
>> >> >> > ar cru libb.a b.o
>> >> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
>> >> plugin
>> >> >> -o f -fwhole-program
>> >> >> >
>> >> >> > The code is compiled and linked correctly. bar & vvvvvv don't
>> >> become
>> >> >> static function
>> >> >> > and cause link errors, whereas bar2 is inlined as expected.
>> >> >> >
>> >> >> >
>> >> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >> >> >
>> >> >> > Cheers,
>> >> >> > Bingfeng Mei
>> >> >> >
>> >> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
>> >> >> >
>> >> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
>> >> >> externally_visible
>> >> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> >> >> compiling with
>> >> >> >        -fwhole-program
>> >> >> >
>> >> >> >
>> >> >> > Index: lto-symtab.c
>> >> >> >
>> >> ===================================================================
>> >> >> > --- lto-symtab.c        (revision 160463)
>> >> >> > +++ lto-symtab.c        (working copy)
>> >> >> > @@ -476,7 +476,19 @@
>> >> >> >
>> >> >> >   /* If the chain is already resolved there is nothing else to
>> >> >> do.  */
>> >> >> >   if (e->resolution != LDPR_UNKNOWN)
>> >> >> > -    return;
>> >> >> > +    {
>> >> >> > +      /* Add externally_visible attribute for declaration of
>> >> >> LDPR_PREVAILING_DEF */
>> >> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
>> >> flag_whole_program)
>> >> >> > +        {
>> >> >> > +          if (!lookup_attribute ("externally_visible",
>> >> >> DECL_ATTRIBUTES (e->decl)))
>> >> >> > +            {
>> >> >> > +              DECL_ATTRIBUTES (e->decl)
>> >> >> > +                = tree_cons (get_identifier
>> >> ("externally_visible"),
>> >> >> NULL_TREE,
>> >> >> > +                             DECL_ATTRIBUTES (e->decl));
>> >> >>
>> >> >> I don't think we need to add an attribute here but we can play
>> >> >> with some cgraph flags which is cheaper.
>> >> >>
>> >> >> Also I think this isn't really correct - not everything that
>> >> prevails
>> >> >> needs to be externally visible (in fact, you seem to simply
>> >> >> remove the effect of -fwhole-program completely).
>> >> >>
>> >> >> A testcase that should still work is
>> >> >>
>> >> >> t1.c:
>> >> >> void foo(void) { bar (); }
>> >> >> t2.c
>> >> >> extern void foo (void);
>> >> >> void bar (void) {}
>> >> >> void eliminate_me (void) {}
>> >> >> int main()
>> >> >> {
>> >> >>    foo();
>> >> >> }
>> >> >>
>> >> >> and eliminate_me should still be eliminated with -fwhole-program
>> >> >> if you do
>> >> >>
>> >> >> gcc -c t1.c
>> >> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
>> >> >>
>> >> >> Thus, the resolution file probably does not have the information
>> >> >> you need (a list of references from outside of the LTO file set).
>> >> >>
>> >> >> Richard.
>> >> >
>> >> >
>> >> >
>> >
>> >
>> >
>
>
>

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

Actually, I got correct results from gold
x.c
extern int i;

y.c
int i;

~/work/install-x86/bin/gcc  y.o x.o -O2 -fuse-linker-plugin -o f -flto -fwhole-program -save-temps
x.res:
2
y.o 1
78 PREVAILING_DEF_IRONLY i
x.o 0

~/work/install-x86/bin/gcc  x.o y.o -O2 -fuse-linker-plugin -o f -flto -fwhole-program -save-temps
y.res
2
x.o 0
y.o 1
78 PREVAILING_DEF_IRONLY i


I am using binutils-2.20-51.

Bingfeng
> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 10 June 2010 17:12
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Thu, Jun 10, 2010 at 4:51 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> > Richard,
> > I figured out what happen with externally_visible flags in cgraph
> node/vnode.
> >
> > In ipa.c (function_and_varialbe_visibility), the externally_visible
> flags are set to
> > true when compile with -flto because cgraph_externally_visible_p
> always returns true as
> > whole_program is false (regardless -fwhole-program option is set or
> not, which is
> > always ignored in the first phase of LTO compilation).
> >
> >      if (cgraph_externally_visible_p (node, whole_program))
> >        {
> >          gcc_assert (!node->global.inlined_to);
> >          node->local.externally_visible = true;
> >        }
> >
> > Then the flags are packed/unpacked from LTO byte code. That's why the
> externally_visible
> > flag is set to true for "bar2" function before executing my patch.
> >
> >
> >
> > Another issue is following code (your patch on 22/5). The vvvvvv
> >  variable in my example is going to be resolved by internal
> >  resolver as LDPR_PREVAILING_DEF_IRONLY instead of
> > LDPR_PREVAILING_DEF by resolution file. Could you
> > Elaborate what is the exact issue here?
> 
> Gold does not keep track of which symbol from which object
> file it will end up using for commons but instead simply picks
> the first one.  We rely on the fact that we get the one with
> the largest size - with gold we even can end up with
> a non-prevailing one.  And there's some version of gold which doesn't
> resolve commons at all.
> 
> So we need to run through our own resolution code here.
> 
> An example is
> 
> extern int i;
> --
> int i;
> 
> where gold can claim that 'extern int i' was prevailing.
> 
> >          /* The LTO plugin for gold doesn't handle common symbols
> >             properly.  Let us choose manually.  */
> >          if (DECL_COMMON (e->decl))
> >            e->resolution = LDPR_UNKNOWN;
> >
> > Thanks,
> > Bingfeng
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 10 June 2010 11:12
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Thu, Jun 10, 2010 at 12:03 PM, Bingfeng Mei <bmei@broadcom.com>
> >> wrote:
> >> > Richard,
> >> >
> >> > I tried to set externally_visible flags directly for cgraph node &
> >> varbool node.
> >> > Unfortunately, these flags seem to be set and used elsewhere too.
> The
> >> values
> >> > set in lto-symtab.c cannot be reliably passed to ipa.c
> >> (function_and_variable_visibility),
> >> > where the flags are actually re-computed.
> >> >
> >> >      if (cgraph_externally_visible_p (node, whole_program))
> >> >        {
> >> >          gcc_assert (!node->global.inlined_to);
> >> >          node->local.externally_visible = true;
> >> >        }
> >> >      else
> >> >        node->local.externally_visible = false;
> >> >
> >> > The flag for bar2 function is true before this piece of code even
> I
> >> doesn't set
> >> > it in lto-symtab.c.
> >>
> >> I believe this is an error - Honza knows the code and will probably
> >> comment.
> >>
> >> > For now, I think attribute is still cleaner solution though
> >> > not cheap. The following is the updated patch.
> >>
> >> Thanks - I believe this is valuable but want to hear comments
> >> from Honza as we shouldn't need to use attributes here.
> >>
> >> Richard.
> >>
> >> > Cheers,
> >> > Bingfeng
> >> >
> >> > 2010-06-10  Bingfeng Mei <bmei@broadcom.com>
> >> >
> >> >        * lto-symbtab.c (lto_symtab_merge_decls_1): Add
> >> externally_visible
> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> compiling with
> >> >        -fwhole-program
> >> >        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
> >> for
> >> >        internal resolver
> >> >
> >> > Index: lto-symtab.c
> >> >
> ===================================================================
> >> > --- lto-symtab.c        (revision 160463)
> >> > +++ lto-symtab.c        (working copy)
> >> > @@ -530,11 +530,15 @@
> >> >     return;
> >> >
> >> >  found:
> >> > -  if (TREE_CODE (prevailing->decl) == VAR_DECL
> >> > -      && TREE_READONLY (prevailing->decl))
> >> > -    prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> >> > -  else
> >> > -    prevailing->resolution = LDPR_PREVAILING_DEF;
> >> > +  /* If current lto files represent the whole program,
> >> > +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
> >> > +    If current lto files are part of whole program, internal
> >> > +    resolver doesn't know if it is LDPR_PREVAILING_DEF
> >> > +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
> >> > +    using -fwhole-program. Otherwise, it doesn't
> >> > +    matter using either LDPR_PREVAILING_DEF or
> >> > +    LDPR_PREVAILING_DEF_IRONLY */
> >> > +  prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> >> >  }
> >> >
> >> >  /* Merge all decls in the symbol table chain to the prevailing
> decl
> >> and
> >> > @@ -698,6 +702,18 @@
> >> >       && TREE_CODE (prevailing->decl) != VAR_DECL)
> >> >     prevailing->next = NULL;
> >> >
> >> > +
> >> > +  /* Add externally_visible attribute for declaration of
> >> LDPR_PREVAILING_DEF */
> >> > +  if (prevailing->resolution == LDPR_PREVAILING_DEF &&
> >> flag_whole_program)
> >> > +    {
> >> > +      if (!lookup_attribute ("externally_visible",
> >> > +                             DECL_ATTRIBUTES (prevailing->decl)))
> >> > +        {
> >> > +          DECL_ATTRIBUTES (prevailing->decl)
> >> > +            = tree_cons (get_identifier ("externally_visible"),
> >> NULL_TREE,
> >> > +                         DECL_ATTRIBUTES (prevailing->decl));
> >> > +        }
> >> > +    }
> >> >   return 1;
> >> >  }
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> >> Sent: 09 June 2010 16:26
> >> >> To: Bingfeng Mei
> >> >> Cc: gcc-patches@gcc.gnu.org
> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> >> necessary with -fwhole-program and resolution file.
> >> >>
> >> >> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com>
> >> wrote:
> >> >> > I added an attribute because -fwhole-program/externally_visible
> is
> >> >> handled in ipa.c
> >> >> >
> >> >> > ...
> >> >> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES
> >> (node-
> >> >> >decl)))
> >> >> >    return true;
> >> >> > ...
> >> >> >
> >> >> > Adding attribute seems cleaner than changing flags, otherwise I
> >> need
> >> >> to change
> >> >> > handling in ipa.c as well.
> >> >>
> >> >> True, but there is an externally_visible flag in cgraph_node,
> >> >> so I guess that attribute lookup is bogus.
> >> >>
> >> >> Richard.
> >> >>
> >> >> > Bingfeng
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> >> >> Sent: 09 June 2010 16:02
> >> >> >> To: Bingfeng Mei
> >> >> >> Cc: gcc-patches@gcc.gnu.org
> >> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute
> when
> >> >> >> necessary with -fwhole-program and resolution file.
> >> >> >>
> >> >> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei
> <bmei@broadcom.com>
> >> >> wrote:
> >> >> >> > Hi,
> >> >> >> > This patch addresses issue discussed in
> >> >> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> >> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >> >> >
> >> >> >> > With the patch, any declaration which is resolved as
> >> >> >> LDPR_PREVAILING_DEF
> >> >> >> > and compiled with -fwhole-program is annotated with
> >> >> >> > attribute "externally_visible" if it doesn't exist already.
> >> >> >> > This eliminates the error-prone process of manual annotation
> >> >> >> > of the attribute when compiling mixed LTO/non-LTO
> applications.
> >> >> >> >
> >> >> >> > For the following test files:
> >> >> >> > a.c
> >> >> >> >
> >> >> >> > #include <string.h>
> >> >> >> > #include <stdio.h>
> >> >> >> > extern int foo(int);
> >> >> >> > /* Called by b.c, should not be optimized by -fwhole-program
> */
> >> >> >> > void bar()
> >> >> >> > {
> >> >> >> >  printf("bar\n");
> >> >> >> > }
> >> >> >> > /* Not used by others, should be optimized out by -fwhole-
> >> >> program*/
> >> >> >> > void bar2()
> >> >> >> > {
> >> >> >> >  printf("bar2\n");
> >> >> >> > }
> >> >> >> > extern int src[], dst[];
> >> >> >> > int vvvvvv;
> >> >> >> > int main()
> >> >> >> > {
> >> >> >> >  int ret;
> >> >> >> >  vvvvvv = 12;
> >> >> >> >  ret = foo(20);
> >> >> >> >  bar2();
> >> >> >> >  memcpy(dst, src, 100);
> >> >> >> >  return ret + 3;
> >> >> >> > }
> >> >> >> >
> >> >> >> > b.c
> >> >> >> >
> >> >> >> > #include <stdio.h>
> >> >> >> > int src[100];
> >> >> >> > int dst[100];
> >> >> >> > extern int vvvvvv;
> >> >> >> > extern void bar();
> >> >> >> > int foo(int c)
> >> >> >> > {
> >> >> >> >  printf("Hello world: %d\n", c);
> >> >> >> >  bar();
> >> >> >> >  return 1000 + vvvvvv;
> >> >> >> > }
> >> >> >> >
> >> >> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> >> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> >> >> > ar cru libb.a b.o
> >> >> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-
> linker-
> >> >> plugin
> >> >> >> -o f -fwhole-program
> >> >> >> >
> >> >> >> > The code is compiled and linked correctly. bar & vvvvvv
> don't
> >> >> become
> >> >> >> static function
> >> >> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >> >> >
> >> >> >> >
> >> >> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >> >> >
> >> >> >> > Cheers,
> >> >> >> > Bingfeng Mei
> >> >> >> >
> >> >> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >> >> >
> >> >> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> >> >> externally_visible
> >> >> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> >> >> compiling with
> >> >> >> >        -fwhole-program
> >> >> >> >
> >> >> >> >
> >> >> >> > Index: lto-symtab.c
> >> >> >> >
> >> >>
> ===================================================================
> >> >> >> > --- lto-symtab.c        (revision 160463)
> >> >> >> > +++ lto-symtab.c        (working copy)
> >> >> >> > @@ -476,7 +476,19 @@
> >> >> >> >
> >> >> >> >   /* If the chain is already resolved there is nothing else
> to
> >> >> >> do.  */
> >> >> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> >> >> > -    return;
> >> >> >> > +    {
> >> >> >> > +      /* Add externally_visible attribute for declaration
> of
> >> >> >> LDPR_PREVAILING_DEF */
> >> >> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> >> >> flag_whole_program)
> >> >> >> > +        {
> >> >> >> > +          if (!lookup_attribute ("externally_visible",
> >> >> >> DECL_ATTRIBUTES (e->decl)))
> >> >> >> > +            {
> >> >> >> > +              DECL_ATTRIBUTES (e->decl)
> >> >> >> > +                = tree_cons (get_identifier
> >> >> ("externally_visible"),
> >> >> >> NULL_TREE,
> >> >> >> > +                             DECL_ATTRIBUTES (e->decl));
> >> >> >>
> >> >> >> I don't think we need to add an attribute here but we can play
> >> >> >> with some cgraph flags which is cheaper.
> >> >> >>
> >> >> >> Also I think this isn't really correct - not everything that
> >> >> prevails
> >> >> >> needs to be externally visible (in fact, you seem to simply
> >> >> >> remove the effect of -fwhole-program completely).
> >> >> >>
> >> >> >> A testcase that should still work is
> >> >> >>
> >> >> >> t1.c:
> >> >> >> void foo(void) { bar (); }
> >> >> >> t2.c
> >> >> >> extern void foo (void);
> >> >> >> void bar (void) {}
> >> >> >> void eliminate_me (void) {}
> >> >> >> int main()
> >> >> >> {
> >> >> >>    foo();
> >> >> >> }
> >> >> >>
> >> >> >> and eliminate_me should still be eliminated with -fwhole-
> program
> >> >> >> if you do
> >> >> >>
> >> >> >> gcc -c t1.c
> >> >> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >> >> >>
> >> >> >> Thus, the resolution file probably does not have the
> information
> >> >> >> you need (a list of references from outside of the LTO file
> set).
> >> >> >>
> >> >> >> Richard.
> >> >> >
> >> >> >
> >> >> >
> >> >
> >> >
> >> >
> >
> >
> >

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Thu, Jun 10, 2010 at 6:23 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Actually, I got correct results from gold
> x.c
> extern int i;
>
> y.c
> int i;
>
> ~/work/install-x86/bin/gcc  y.o x.o -O2 -fuse-linker-plugin -o f -flto -fwhole-program -save-temps
> x.res:
> 2
> y.o 1
> 78 PREVAILING_DEF_IRONLY i
> x.o 0
>
> ~/work/install-x86/bin/gcc  x.o y.o -O2 -fuse-linker-plugin -o f -flto -fwhole-program -save-temps
> y.res
> 2
> x.o 0
> y.o 1
> 78 PREVAILING_DEF_IRONLY i
>
>
> I am using binutils-2.20-51.

Yes, the above was a bug that was actually fixed.  The issue that
it chooses a random fortran common block and not the largest
one still prevails.

We might consider reverting the change on the trunk (it's certainly
safe on the branch and makes more things work there).  But it
would also be nice for either gold or the linker-plugin being fixed
for the size issue.  (I think you run into the issue when building
SPEC 2k6, you might also find a closed bugzilla that reports it)

Richard.

> Bingfeng
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> Sent: 10 June 2010 17:12
>> To: Bingfeng Mei
>> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
>> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> necessary with -fwhole-program and resolution file.
>>
>> On Thu, Jun 10, 2010 at 4:51 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>> > Richard,
>> > I figured out what happen with externally_visible flags in cgraph
>> node/vnode.
>> >
>> > In ipa.c (function_and_varialbe_visibility), the externally_visible
>> flags are set to
>> > true when compile with -flto because cgraph_externally_visible_p
>> always returns true as
>> > whole_program is false (regardless -fwhole-program option is set or
>> not, which is
>> > always ignored in the first phase of LTO compilation).
>> >
>> >      if (cgraph_externally_visible_p (node, whole_program))
>> >        {
>> >          gcc_assert (!node->global.inlined_to);
>> >          node->local.externally_visible = true;
>> >        }
>> >
>> > Then the flags are packed/unpacked from LTO byte code. That's why the
>> externally_visible
>> > flag is set to true for "bar2" function before executing my patch.
>> >
>> >
>> >
>> > Another issue is following code (your patch on 22/5). The vvvvvv
>> >  variable in my example is going to be resolved by internal
>> >  resolver as LDPR_PREVAILING_DEF_IRONLY instead of
>> > LDPR_PREVAILING_DEF by resolution file. Could you
>> > Elaborate what is the exact issue here?
>>
>> Gold does not keep track of which symbol from which object
>> file it will end up using for commons but instead simply picks
>> the first one.  We rely on the fact that we get the one with
>> the largest size - with gold we even can end up with
>> a non-prevailing one.  And there's some version of gold which doesn't
>> resolve commons at all.
>>
>> So we need to run through our own resolution code here.
>>
>> An example is
>>
>> extern int i;
>> --
>> int i;
>>
>> where gold can claim that 'extern int i' was prevailing.
>>
>> >          /* The LTO plugin for gold doesn't handle common symbols
>> >             properly.  Let us choose manually.  */
>> >          if (DECL_COMMON (e->decl))
>> >            e->resolution = LDPR_UNKNOWN;
>> >
>> > Thanks,
>> > Bingfeng
>> >> -----Original Message-----
>> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> >> Sent: 10 June 2010 11:12
>> >> To: Bingfeng Mei
>> >> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
>> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> >> necessary with -fwhole-program and resolution file.
>> >>
>> >> On Thu, Jun 10, 2010 at 12:03 PM, Bingfeng Mei <bmei@broadcom.com>
>> >> wrote:
>> >> > Richard,
>> >> >
>> >> > I tried to set externally_visible flags directly for cgraph node &
>> >> varbool node.
>> >> > Unfortunately, these flags seem to be set and used elsewhere too.
>> The
>> >> values
>> >> > set in lto-symtab.c cannot be reliably passed to ipa.c
>> >> (function_and_variable_visibility),
>> >> > where the flags are actually re-computed.
>> >> >
>> >> >      if (cgraph_externally_visible_p (node, whole_program))
>> >> >        {
>> >> >          gcc_assert (!node->global.inlined_to);
>> >> >          node->local.externally_visible = true;
>> >> >        }
>> >> >      else
>> >> >        node->local.externally_visible = false;
>> >> >
>> >> > The flag for bar2 function is true before this piece of code even
>> I
>> >> doesn't set
>> >> > it in lto-symtab.c.
>> >>
>> >> I believe this is an error - Honza knows the code and will probably
>> >> comment.
>> >>
>> >> > For now, I think attribute is still cleaner solution though
>> >> > not cheap. The following is the updated patch.
>> >>
>> >> Thanks - I believe this is valuable but want to hear comments
>> >> from Honza as we shouldn't need to use attributes here.
>> >>
>> >> Richard.
>> >>
>> >> > Cheers,
>> >> > Bingfeng
>> >> >
>> >> > 2010-06-10  Bingfeng Mei <bmei@broadcom.com>
>> >> >
>> >> >        * lto-symbtab.c (lto_symtab_merge_decls_1): Add
>> >> externally_visible
>> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> >> compiling with
>> >> >        -fwhole-program
>> >> >        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
>> >> for
>> >> >        internal resolver
>> >> >
>> >> > Index: lto-symtab.c
>> >> >
>> ===================================================================
>> >> > --- lto-symtab.c        (revision 160463)
>> >> > +++ lto-symtab.c        (working copy)
>> >> > @@ -530,11 +530,15 @@
>> >> >     return;
>> >> >
>> >> >  found:
>> >> > -  if (TREE_CODE (prevailing->decl) == VAR_DECL
>> >> > -      && TREE_READONLY (prevailing->decl))
>> >> > -    prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
>> >> > -  else
>> >> > -    prevailing->resolution = LDPR_PREVAILING_DEF;
>> >> > +  /* If current lto files represent the whole program,
>> >> > +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
>> >> > +    If current lto files are part of whole program, internal
>> >> > +    resolver doesn't know if it is LDPR_PREVAILING_DEF
>> >> > +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
>> >> > +    using -fwhole-program. Otherwise, it doesn't
>> >> > +    matter using either LDPR_PREVAILING_DEF or
>> >> > +    LDPR_PREVAILING_DEF_IRONLY */
>> >> > +  prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
>> >> >  }
>> >> >
>> >> >  /* Merge all decls in the symbol table chain to the prevailing
>> decl
>> >> and
>> >> > @@ -698,6 +702,18 @@
>> >> >       && TREE_CODE (prevailing->decl) != VAR_DECL)
>> >> >     prevailing->next = NULL;
>> >> >
>> >> > +
>> >> > +  /* Add externally_visible attribute for declaration of
>> >> LDPR_PREVAILING_DEF */
>> >> > +  if (prevailing->resolution == LDPR_PREVAILING_DEF &&
>> >> flag_whole_program)
>> >> > +    {
>> >> > +      if (!lookup_attribute ("externally_visible",
>> >> > +                             DECL_ATTRIBUTES (prevailing->decl)))
>> >> > +        {
>> >> > +          DECL_ATTRIBUTES (prevailing->decl)
>> >> > +            = tree_cons (get_identifier ("externally_visible"),
>> >> NULL_TREE,
>> >> > +                         DECL_ATTRIBUTES (prevailing->decl));
>> >> > +        }
>> >> > +    }
>> >> >   return 1;
>> >> >  }
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> >> >> Sent: 09 June 2010 16:26
>> >> >> To: Bingfeng Mei
>> >> >> Cc: gcc-patches@gcc.gnu.org
>> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> >> >> necessary with -fwhole-program and resolution file.
>> >> >>
>> >> >> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com>
>> >> wrote:
>> >> >> > I added an attribute because -fwhole-program/externally_visible
>> is
>> >> >> handled in ipa.c
>> >> >> >
>> >> >> > ...
>> >> >> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES
>> >> (node-
>> >> >> >decl)))
>> >> >> >    return true;
>> >> >> > ...
>> >> >> >
>> >> >> > Adding attribute seems cleaner than changing flags, otherwise I
>> >> need
>> >> >> to change
>> >> >> > handling in ipa.c as well.
>> >> >>
>> >> >> True, but there is an externally_visible flag in cgraph_node,
>> >> >> so I guess that attribute lookup is bogus.
>> >> >>
>> >> >> Richard.
>> >> >>
>> >> >> > Bingfeng
>> >> >> >
>> >> >> >> -----Original Message-----
>> >> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> >> >> >> Sent: 09 June 2010 16:02
>> >> >> >> To: Bingfeng Mei
>> >> >> >> Cc: gcc-patches@gcc.gnu.org
>> >> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute
>> when
>> >> >> >> necessary with -fwhole-program and resolution file.
>> >> >> >>
>> >> >> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei
>> <bmei@broadcom.com>
>> >> >> wrote:
>> >> >> >> > Hi,
>> >> >> >> > This patch addresses issue discussed in
>> >> >> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
>> >> >> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
>> >> >> >> >
>> >> >> >> > With the patch, any declaration which is resolved as
>> >> >> >> LDPR_PREVAILING_DEF
>> >> >> >> > and compiled with -fwhole-program is annotated with
>> >> >> >> > attribute "externally_visible" if it doesn't exist already.
>> >> >> >> > This eliminates the error-prone process of manual annotation
>> >> >> >> > of the attribute when compiling mixed LTO/non-LTO
>> applications.
>> >> >> >> >
>> >> >> >> > For the following test files:
>> >> >> >> > a.c
>> >> >> >> >
>> >> >> >> > #include <string.h>
>> >> >> >> > #include <stdio.h>
>> >> >> >> > extern int foo(int);
>> >> >> >> > /* Called by b.c, should not be optimized by -fwhole-program
>> */
>> >> >> >> > void bar()
>> >> >> >> > {
>> >> >> >> >  printf("bar\n");
>> >> >> >> > }
>> >> >> >> > /* Not used by others, should be optimized out by -fwhole-
>> >> >> program*/
>> >> >> >> > void bar2()
>> >> >> >> > {
>> >> >> >> >  printf("bar2\n");
>> >> >> >> > }
>> >> >> >> > extern int src[], dst[];
>> >> >> >> > int vvvvvv;
>> >> >> >> > int main()
>> >> >> >> > {
>> >> >> >> >  int ret;
>> >> >> >> >  vvvvvv = 12;
>> >> >> >> >  ret = foo(20);
>> >> >> >> >  bar2();
>> >> >> >> >  memcpy(dst, src, 100);
>> >> >> >> >  return ret + 3;
>> >> >> >> > }
>> >> >> >> >
>> >> >> >> > b.c
>> >> >> >> >
>> >> >> >> > #include <stdio.h>
>> >> >> >> > int src[100];
>> >> >> >> > int dst[100];
>> >> >> >> > extern int vvvvvv;
>> >> >> >> > extern void bar();
>> >> >> >> > int foo(int c)
>> >> >> >> > {
>> >> >> >> >  printf("Hello world: %d\n", c);
>> >> >> >> >  bar();
>> >> >> >> >  return 1000 + vvvvvv;
>> >> >> >> > }
>> >> >> >> >
>> >> >> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
>> >> >> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
>> >> >> >> > ar cru libb.a b.o
>> >> >> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-
>> linker-
>> >> >> plugin
>> >> >> >> -o f -fwhole-program
>> >> >> >> >
>> >> >> >> > The code is compiled and linked correctly. bar & vvvvvv
>> don't
>> >> >> become
>> >> >> >> static function
>> >> >> >> > and cause link errors, whereas bar2 is inlined as expected.
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >> >> >> >
>> >> >> >> > Cheers,
>> >> >> >> > Bingfeng Mei
>> >> >> >> >
>> >> >> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
>> >> >> >> >
>> >> >> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
>> >> >> >> externally_visible
>> >> >> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> >> >> >> compiling with
>> >> >> >> >        -fwhole-program
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > Index: lto-symtab.c
>> >> >> >> >
>> >> >>
>> ===================================================================
>> >> >> >> > --- lto-symtab.c        (revision 160463)
>> >> >> >> > +++ lto-symtab.c        (working copy)
>> >> >> >> > @@ -476,7 +476,19 @@
>> >> >> >> >
>> >> >> >> >   /* If the chain is already resolved there is nothing else
>> to
>> >> >> >> do.  */
>> >> >> >> >   if (e->resolution != LDPR_UNKNOWN)
>> >> >> >> > -    return;
>> >> >> >> > +    {
>> >> >> >> > +      /* Add externally_visible attribute for declaration
>> of
>> >> >> >> LDPR_PREVAILING_DEF */
>> >> >> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
>> >> >> flag_whole_program)
>> >> >> >> > +        {
>> >> >> >> > +          if (!lookup_attribute ("externally_visible",
>> >> >> >> DECL_ATTRIBUTES (e->decl)))
>> >> >> >> > +            {
>> >> >> >> > +              DECL_ATTRIBUTES (e->decl)
>> >> >> >> > +                = tree_cons (get_identifier
>> >> >> ("externally_visible"),
>> >> >> >> NULL_TREE,
>> >> >> >> > +                             DECL_ATTRIBUTES (e->decl));
>> >> >> >>
>> >> >> >> I don't think we need to add an attribute here but we can play
>> >> >> >> with some cgraph flags which is cheaper.
>> >> >> >>
>> >> >> >> Also I think this isn't really correct - not everything that
>> >> >> prevails
>> >> >> >> needs to be externally visible (in fact, you seem to simply
>> >> >> >> remove the effect of -fwhole-program completely).
>> >> >> >>
>> >> >> >> A testcase that should still work is
>> >> >> >>
>> >> >> >> t1.c:
>> >> >> >> void foo(void) { bar (); }
>> >> >> >> t2.c
>> >> >> >> extern void foo (void);
>> >> >> >> void bar (void) {}
>> >> >> >> void eliminate_me (void) {}
>> >> >> >> int main()
>> >> >> >> {
>> >> >> >>    foo();
>> >> >> >> }
>> >> >> >>
>> >> >> >> and eliminate_me should still be eliminated with -fwhole-
>> program
>> >> >> >> if you do
>> >> >> >>
>> >> >> >> gcc -c t1.c
>> >> >> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
>> >> >> >>
>> >> >> >> Thus, the resolution file probably does not have the
>> information
>> >> >> >> you need (a list of references from outside of the LTO file
>> set).
>> >> >> >>
>> >> >> >> Richard.
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >
>> >> >
>> >> >
>> >
>> >
>> >
>
>
>

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Cary Coutant]

> Yes, the above was a bug that was actually fixed.  The issue that
> it chooses a random fortran common block and not the largest
> one still prevails.
>
> We might consider reverting the change on the trunk (it's certainly
> safe on the branch and makes more things work there).  But it
> would also be nice for either gold or the linker-plugin being fixed
> for the size issue.  (I think you run into the issue when building
> SPEC 2k6, you might also find a closed bugzilla that reports it)

Yes, gold merges common symbols by keeping the first and simply
increasing its size to the maximum of all subsequent ones. By the time
the plugin asks for symbol resolution information, there's no record
of which one was actually the largest. I'll work on a fix for this.

Is this in fact what PR 44149 is about?

-cary

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Mon, Jun 14, 2010 at 8:43 PM, Cary Coutant <ccoutant@google.com> wrote:
>> Yes, the above was a bug that was actually fixed.  The issue that
>> it chooses a random fortran common block and not the largest
>> one still prevails.
>>
>> We might consider reverting the change on the trunk (it's certainly
>> safe on the branch and makes more things work there).  But it
>> would also be nice for either gold or the linker-plugin being fixed
>> for the size issue.  (I think you run into the issue when building
>> SPEC 2k6, you might also find a closed bugzilla that reports it)
>
> Yes, gold merges common symbols by keeping the first and simply
> increasing its size to the maximum of all subsequent ones. By the time
> the plugin asks for symbol resolution information, there's no record
> of which one was actually the largest. I'll work on a fix for this.
>
> Is this in fact what PR 44149 is about?

Yes.  In that PR the linker-plugin reports a file with only a DECL_EXTERNAL
symbol as providing the prevailing one even.

Richard.

> -cary
>

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

Hi, Richard,
Here is the updated patch. The flags are set instead of attributes now. 
The check is moved to the end of lto_symtab_merge_decls_1. For the DECL_COMM,
since internal resolver is always used due to your workaround for gold plugin,
These variables would still need explicit externally_visible attributes. 

Bootstrapped and tested.

Cheers,
Bingfeng.

2010-06-11  Bingfeng Mei <bmei@broadcom.com>

	* lto-symbtab.c (lto_symtab_merge_decls_1): Set externally_visible
        flags for symbols of LDPR_PREVAILING_DEF when compiling with
	-fwhole-program 
	(lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for 
	internal resolver
        * ipa.c (function_and_variable_visibility): Set externally_visible
        flags only if they are false. This allows flags to be passed from


Index: lto-symtab.c
===================================================================
--- lto-symtab.c        (revision 160529)
+++ lto-symtab.c        (working copy)
@@ -530,11 +530,21 @@
     return;

 found:
-  if (TREE_CODE (prevailing->decl) == VAR_DECL
-      && TREE_READONLY (prevailing->decl))
+  /* If current lto files represent the whole program,
+    it is correct to use LDPR_PREVALING_DEF_IRONLY.
+    If current lto files are part of whole program, internal
+    resolver doesn't know if it is LDPR_PREVAILING_DEF
+    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
+    using -fwhole-program. Otherwise, it doesn't
+    matter using either LDPR_PREVAILING_DEF or
+    LDPR_PREVAILING_DEF_IRONLY
+
+    FIXME: above workaround due to gold plugin makes some
+    variables IRONLY, which are indeed PREVAILING_DEF in
+    resolution file. These variables still need manual
+    externally_visible attribute
+    */
     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
-  else
-    prevailing->resolution = LDPR_PREVAILING_DEF;
 }

 /* Merge all decls in the symbol table chain to the prevailing decl and
@@ -698,6 +708,25 @@
       && TREE_CODE (prevailing->decl) != VAR_DECL)
     prevailing->next = NULL;

+
+  /* Add externally_visible attribute for declaration of LDPR_PREVAILING_DEF */
+  if (flag_whole_program)
+    {
+      if (prevailing->resolution == LDPR_PREVAILING_DEF)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = true;
+          else
+            prevailing->vnode->externally_visible = true;
+        }
+      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = false;
+          else
+            prevailing->vnode->externally_visible = false;
+        }
+    }
   return 1;
 }

Index: ipa.c
===================================================================
--- ipa.c       (revision 160529)
+++ ipa.c       (working copy)
@@ -665,13 +665,12 @@
        }
       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl))
                  || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl));
-      if (cgraph_externally_visible_p (node, whole_program))
+      if (!node->local.externally_visible
+          && cgraph_externally_visible_p (node, whole_program))
         {
          gcc_assert (!node->global.inlined_to);
          node->local.externally_visible = true;
        }
-      else
-       node->local.externally_visible = false;
       if (!node->local.externally_visible && node->analyzed
          && !DECL_EXTERNAL (node->decl))
        {
@@ -721,7 +720,8 @@
     {
       if (!vnode->finalized)
         continue;
-      if (vnode->needed
+      if (!vnode->externally_visible
+          && vnode->needed
          && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
          && (!whole_program
              /* We can privatize comdat readonly variables whose address is not taken,
@@ -732,8 +732,6 @@
              || lookup_attribute ("externally_visible",
                                   DECL_ATTRIBUTES (vnode->decl))))
        vnode->externally_visible = true;
-      else
-        vnode->externally_visible = false;
       if (!vnode->externally_visible)
        {
          gcc_assert (whole_program || !TREE_PUBLIC (vnode->decl));

> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 09 June 2010 16:26
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> > I added an attribute because -fwhole-program/externally_visible is
> handled in ipa.c
> >
> > ...
> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node-
> >decl)))
> >    return true;
> > ...
> >
> > Adding attribute seems cleaner than changing flags, otherwise I need
> to change
> > handling in ipa.c as well.
> 
> True, but there is an externally_visible flag in cgraph_node,
> so I guess that attribute lookup is bogus.
> 
> Richard.
> 
> > Bingfeng
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 09 June 2010 16:02
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> >> > Hi,
> >> > This patch addresses issue discussed in
> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >
> >> > With the patch, any declaration which is resolved as
> >> LDPR_PREVAILING_DEF
> >> > and compiled with -fwhole-program is annotated with
> >> > attribute "externally_visible" if it doesn't exist already.
> >> > This eliminates the error-prone process of manual annotation
> >> > of the attribute when compiling mixed LTO/non-LTO applications.
> >> >
> >> > For the following test files:
> >> > a.c
> >> >
> >> > #include <string.h>
> >> > #include <stdio.h>
> >> > extern int foo(int);
> >> > /* Called by b.c, should not be optimized by -fwhole-program */
> >> > void bar()
> >> > {
> >> >  printf("bar\n");
> >> > }
> >> > /* Not used by others, should be optimized out by -fwhole-
> program*/
> >> > void bar2()
> >> > {
> >> >  printf("bar2\n");
> >> > }
> >> > extern int src[], dst[];
> >> > int vvvvvv;
> >> > int main()
> >> > {
> >> >  int ret;
> >> >  vvvvvv = 12;
> >> >  ret = foo(20);
> >> >  bar2();
> >> >  memcpy(dst, src, 100);
> >> >  return ret + 3;
> >> > }
> >> >
> >> > b.c
> >> >
> >> > #include <stdio.h>
> >> > int src[100];
> >> > int dst[100];
> >> > extern int vvvvvv;
> >> > extern void bar();
> >> > int foo(int c)
> >> > {
> >> >  printf("Hello world: %d\n", c);
> >> >  bar();
> >> >  return 1000 + vvvvvv;
> >> > }
> >> >
> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> > ar cru libb.a b.o
> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
> plugin
> >> -o f -fwhole-program
> >> >
> >> > The code is compiled and linked correctly. bar & vvvvvv don't
> become
> >> static function
> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >
> >> >
> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >
> >> > Cheers,
> >> > Bingfeng Mei
> >> >
> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >
> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> externally_visible
> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> compiling with
> >> >        -fwhole-program
> >> >
> >> >
> >> > Index: lto-symtab.c
> >> >
> ===================================================================
> >> > --- lto-symtab.c        (revision 160463)
> >> > +++ lto-symtab.c        (working copy)
> >> > @@ -476,7 +476,19 @@
> >> >
> >> >   /* If the chain is already resolved there is nothing else to
> >> do.  */
> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> > -    return;
> >> > +    {
> >> > +      /* Add externally_visible attribute for declaration of
> >> LDPR_PREVAILING_DEF */
> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> flag_whole_program)
> >> > +        {
> >> > +          if (!lookup_attribute ("externally_visible",
> >> DECL_ATTRIBUTES (e->decl)))
> >> > +            {
> >> > +              DECL_ATTRIBUTES (e->decl)
> >> > +                = tree_cons (get_identifier
> ("externally_visible"),
> >> NULL_TREE,
> >> > +                             DECL_ATTRIBUTES (e->decl));
> >>
> >> I don't think we need to add an attribute here but we can play
> >> with some cgraph flags which is cheaper.
> >>
> >> Also I think this isn't really correct - not everything that
> prevails
> >> needs to be externally visible (in fact, you seem to simply
> >> remove the effect of -fwhole-program completely).
> >>
> >> A testcase that should still work is
> >>
> >> t1.c:
> >> void foo(void) { bar (); }
> >> t2.c
> >> extern void foo (void);
> >> void bar (void) {}
> >> void eliminate_me (void) {}
> >> int main()
> >> {
> >>    foo();
> >> }
> >>
> >> and eliminate_me should still be eliminated with -fwhole-program
> >> if you do
> >>
> >> gcc -c t1.c
> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >>
> >> Thus, the resolution file probably does not have the information
> >> you need (a list of references from outside of the LTO file set).
> >>
> >> Richard.
> >
> >
> >

Re: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Richard Guenther]

On Mon, Jun 14, 2010 at 10:39 AM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Hi, Richard,
> Here is the updated patch. The flags are set instead of attributes now.
> The check is moved to the end of lto_symtab_merge_decls_1. For the DECL_COMM,
> since internal resolver is always used due to your workaround for gold plugin,
> These variables would still need explicit externally_visible attributes.

Can you amend the docs that talk about -flto + -fwhole-program
accordingly?

I'd like Honza to go over the cgraph related changes, otherwise
the patch looks good with ...

> Bootstrapped and tested.
>
> Cheers,
> Bingfeng.
>
> 2010-06-11  Bingfeng Mei <bmei@broadcom.com>
>
>        * lto-symbtab.c (lto_symtab_merge_decls_1): Set externally_visible
>        flags for symbols of LDPR_PREVAILING_DEF when compiling with
>        -fwhole-program
>        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for
>        internal resolver
>        * ipa.c (function_and_variable_visibility): Set externally_visible
>        flags only if they are false. This allows flags to be passed from
>
>
> Index: lto-symtab.c
> ===================================================================
> --- lto-symtab.c        (revision 160529)
> +++ lto-symtab.c        (working copy)
> @@ -530,11 +530,21 @@
>     return;
>
>  found:
> -  if (TREE_CODE (prevailing->decl) == VAR_DECL
> -      && TREE_READONLY (prevailing->decl))
> +  /* If current lto files represent the whole program,
> +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
> +    If current lto files are part of whole program, internal
> +    resolver doesn't know if it is LDPR_PREVAILING_DEF
> +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
> +    using -fwhole-program. Otherwise, it doesn't
> +    matter using either LDPR_PREVAILING_DEF or
> +    LDPR_PREVAILING_DEF_IRONLY
> +
> +    FIXME: above workaround due to gold plugin makes some
> +    variables IRONLY, which are indeed PREVAILING_DEF in
> +    resolution file. These variables still need manual
> +    externally_visible attribute
> +    */

Full-stop at the end of the sentence, */ not on the next line.  Also
two spaces after each '.'

>     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> -  else
> -    prevailing->resolution = LDPR_PREVAILING_DEF;
>  }
>
>  /* Merge all decls in the symbol table chain to the prevailing decl and
> @@ -698,6 +708,25 @@
>       && TREE_CODE (prevailing->decl) != VAR_DECL)
>     prevailing->next = NULL;
>
> +

No extra vertical space please.

> +  /* Add externally_visible attribute for declaration of LDPR_PREVAILING_DEF */

Adjust the comment, to sth like "In whole-program mode mark
LDPR_PREVAILING_DEFs as externally visible. "

> +  if (flag_whole_program)
> +    {
> +      if (prevailing->resolution == LDPR_PREVAILING_DEF)
> +        {
> +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> +            prevailing->node->local.externally_visible = true;
> +          else
> +            prevailing->vnode->externally_visible = true;
> +        }
> +      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
> +        {
> +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> +            prevailing->node->local.externally_visible = false;
> +          else
> +            prevailing->vnode->externally_visible = false;
> +        }
> +    }

Honza will tell you if the above is correct, I am not 100% sure.

Did you verify we generate the same code with and without your
patch when all symbols are resolved inside the IL?

Thanks,
Richard.

>   return 1;
>  }
>
> Index: ipa.c
> ===================================================================
> --- ipa.c       (revision 160529)
> +++ ipa.c       (working copy)
> @@ -665,13 +665,12 @@
>        }
>       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl))
>                  || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl));
> -      if (cgraph_externally_visible_p (node, whole_program))
> +      if (!node->local.externally_visible
> +          && cgraph_externally_visible_p (node, whole_program))
>         {
>          gcc_assert (!node->global.inlined_to);
>          node->local.externally_visible = true;
>        }
> -      else
> -       node->local.externally_visible = false;
>       if (!node->local.externally_visible && node->analyzed
>          && !DECL_EXTERNAL (node->decl))
>        {
> @@ -721,7 +720,8 @@
>     {
>       if (!vnode->finalized)
>         continue;
> -      if (vnode->needed
> +      if (!vnode->externally_visible
> +          && vnode->needed
>          && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
>          && (!whole_program
>              /* We can privatize comdat readonly variables whose address is not taken,
> @@ -732,8 +732,6 @@
>              || lookup_attribute ("externally_visible",
>                                   DECL_ATTRIBUTES (vnode->decl))))
>        vnode->externally_visible = true;
> -      else
> -        vnode->externally_visible = false;
>       if (!vnode->externally_visible)
>        {
>          gcc_assert (whole_program || !TREE_PUBLIC (vnode->decl));
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> Sent: 09 June 2010 16:26
>> To: Bingfeng Mei
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> necessary with -fwhole-program and resolution file.
>>
>> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>> > I added an attribute because -fwhole-program/externally_visible is
>> handled in ipa.c
>> >
>> > ...
>> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node-
>> >decl)))
>> >    return true;
>> > ...
>> >
>> > Adding attribute seems cleaner than changing flags, otherwise I need
>> to change
>> > handling in ipa.c as well.
>>
>> True, but there is an externally_visible flag in cgraph_node,
>> so I guess that attribute lookup is bogus.
>>
>> Richard.
>>
>> > Bingfeng
>> >
>> >> -----Original Message-----
>> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> >> Sent: 09 June 2010 16:02
>> >> To: Bingfeng Mei
>> >> Cc: gcc-patches@gcc.gnu.org
>> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
>> >> necessary with -fwhole-program and resolution file.
>> >>
>> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
>> wrote:
>> >> > Hi,
>> >> > This patch addresses issue discussed in
>> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
>> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
>> >> >
>> >> > With the patch, any declaration which is resolved as
>> >> LDPR_PREVAILING_DEF
>> >> > and compiled with -fwhole-program is annotated with
>> >> > attribute "externally_visible" if it doesn't exist already.
>> >> > This eliminates the error-prone process of manual annotation
>> >> > of the attribute when compiling mixed LTO/non-LTO applications.
>> >> >
>> >> > For the following test files:
>> >> > a.c
>> >> >
>> >> > #include <string.h>
>> >> > #include <stdio.h>
>> >> > extern int foo(int);
>> >> > /* Called by b.c, should not be optimized by -fwhole-program */
>> >> > void bar()
>> >> > {
>> >> >  printf("bar\n");
>> >> > }
>> >> > /* Not used by others, should be optimized out by -fwhole-
>> program*/
>> >> > void bar2()
>> >> > {
>> >> >  printf("bar2\n");
>> >> > }
>> >> > extern int src[], dst[];
>> >> > int vvvvvv;
>> >> > int main()
>> >> > {
>> >> >  int ret;
>> >> >  vvvvvv = 12;
>> >> >  ret = foo(20);
>> >> >  bar2();
>> >> >  memcpy(dst, src, 100);
>> >> >  return ret + 3;
>> >> > }
>> >> >
>> >> > b.c
>> >> >
>> >> > #include <stdio.h>
>> >> > int src[100];
>> >> > int dst[100];
>> >> > extern int vvvvvv;
>> >> > extern void bar();
>> >> > int foo(int c)
>> >> > {
>> >> >  printf("Hello world: %d\n", c);
>> >> >  bar();
>> >> >  return 1000 + vvvvvv;
>> >> > }
>> >> >
>> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
>> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
>> >> > ar cru libb.a b.o
>> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
>> plugin
>> >> -o f -fwhole-program
>> >> >
>> >> > The code is compiled and linked correctly. bar & vvvvvv don't
>> become
>> >> static function
>> >> > and cause link errors, whereas bar2 is inlined as expected.
>> >> >
>> >> >
>> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >> >
>> >> > Cheers,
>> >> > Bingfeng Mei
>> >> >
>> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
>> >> >
>> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
>> >> externally_visible
>> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
>> >> compiling with
>> >> >        -fwhole-program
>> >> >
>> >> >
>> >> > Index: lto-symtab.c
>> >> >
>> ===================================================================
>> >> > --- lto-symtab.c        (revision 160463)
>> >> > +++ lto-symtab.c        (working copy)
>> >> > @@ -476,7 +476,19 @@
>> >> >
>> >> >   /* If the chain is already resolved there is nothing else to
>> >> do.  */
>> >> >   if (e->resolution != LDPR_UNKNOWN)
>> >> > -    return;
>> >> > +    {
>> >> > +      /* Add externally_visible attribute for declaration of
>> >> LDPR_PREVAILING_DEF */
>> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
>> flag_whole_program)
>> >> > +        {
>> >> > +          if (!lookup_attribute ("externally_visible",
>> >> DECL_ATTRIBUTES (e->decl)))
>> >> > +            {
>> >> > +              DECL_ATTRIBUTES (e->decl)
>> >> > +                = tree_cons (get_identifier
>> ("externally_visible"),
>> >> NULL_TREE,
>> >> > +                             DECL_ATTRIBUTES (e->decl));
>> >>
>> >> I don't think we need to add an attribute here but we can play
>> >> with some cgraph flags which is cheaper.
>> >>
>> >> Also I think this isn't really correct - not everything that
>> prevails
>> >> needs to be externally visible (in fact, you seem to simply
>> >> remove the effect of -fwhole-program completely).
>> >>
>> >> A testcase that should still work is
>> >>
>> >> t1.c:
>> >> void foo(void) { bar (); }
>> >> t2.c
>> >> extern void foo (void);
>> >> void bar (void) {}
>> >> void eliminate_me (void) {}
>> >> int main()
>> >> {
>> >>    foo();
>> >> }
>> >>
>> >> and eliminate_me should still be eliminated with -fwhole-program
>> >> if you do
>> >>
>> >> gcc -c t1.c
>> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
>> >>
>> >> Thus, the resolution file probably does not have the information
>> >> you need (a list of references from outside of the LTO file set).
>> >>
>> >> Richard.
>> >
>> >
>> >
>
>
>

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

[-- Attachment #1: Type: text/plain, Size: 12488 bytes --]

I will amend the LTO documents. 

With/without my patch, GCC produces the same results compiling
for IR only for my examples. And it passes LTO tests.  However, bar2 
and foo functions are not inlined as I expected using GNU ld. I believe
this is due to the "externally_visible" flags are improperly passed
from compile-time to link time as discussed in previous mails in this
thread, and this won't be affected by my patch. I will send a separate patch.


Cheers,
Bingfeng




> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 14 June 2010 10:25
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Mon, Jun 14, 2010 at 10:39 AM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> > Hi, Richard,
> > Here is the updated patch. The flags are set instead of attributes
> now.
> > The check is moved to the end of lto_symtab_merge_decls_1. For the
> DECL_COMM,
> > since internal resolver is always used due to your workaround for
> gold plugin,
> > These variables would still need explicit externally_visible
> attributes.
> 
> Can you amend the docs that talk about -flto + -fwhole-program
> accordingly?
> 
> I'd like Honza to go over the cgraph related changes, otherwise
> the patch looks good with ...
> 
> > Bootstrapped and tested.
> >
> > Cheers,
> > Bingfeng.
> >
> > 2010-06-11  Bingfeng Mei <bmei@broadcom.com>
> >
> >        * lto-symbtab.c (lto_symtab_merge_decls_1): Set
> externally_visible
> >        flags for symbols of LDPR_PREVAILING_DEF when compiling with
> >        -fwhole-program
> >        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
> for
> >        internal resolver
> >        * ipa.c (function_and_variable_visibility): Set
> externally_visible
> >        flags only if they are false. This allows flags to be passed
> from
> >
> >
> > Index: lto-symtab.c
> > ===================================================================
> > --- lto-symtab.c        (revision 160529)
> > +++ lto-symtab.c        (working copy)
> > @@ -530,11 +530,21 @@
> >     return;
> >
> >  found:
> > -  if (TREE_CODE (prevailing->decl) == VAR_DECL
> > -      && TREE_READONLY (prevailing->decl))
> > +  /* If current lto files represent the whole program,
> > +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
> > +    If current lto files are part of whole program, internal
> > +    resolver doesn't know if it is LDPR_PREVAILING_DEF
> > +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
> > +    using -fwhole-program. Otherwise, it doesn't
> > +    matter using either LDPR_PREVAILING_DEF or
> > +    LDPR_PREVAILING_DEF_IRONLY
> > +
> > +    FIXME: above workaround due to gold plugin makes some
> > +    variables IRONLY, which are indeed PREVAILING_DEF in
> > +    resolution file. These variables still need manual
> > +    externally_visible attribute
> > +    */
> 
> Full-stop at the end of the sentence, */ not on the next line.  Also
> two spaces after each '.'
> 
> >     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> > -  else
> > -    prevailing->resolution = LDPR_PREVAILING_DEF;
> >  }
> >
> >  /* Merge all decls in the symbol table chain to the prevailing decl
> and
> > @@ -698,6 +708,25 @@
> >       && TREE_CODE (prevailing->decl) != VAR_DECL)
> >     prevailing->next = NULL;
> >
> > +
> 
> No extra vertical space please.
> 
> > +  /* Add externally_visible attribute for declaration of
> LDPR_PREVAILING_DEF */
> 
> Adjust the comment, to sth like "In whole-program mode mark
> LDPR_PREVAILING_DEFs as externally visible. "
> 
> > +  if (flag_whole_program)
> > +    {
> > +      if (prevailing->resolution == LDPR_PREVAILING_DEF)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = true;
> > +          else
> > +            prevailing->vnode->externally_visible = true;
> > +        }
> > +      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = false;
> > +          else
> > +            prevailing->vnode->externally_visible = false;
> > +        }
> > +    }
> 
> Honza will tell you if the above is correct, I am not 100% sure.
> 
> Did you verify we generate the same code with and without your
> patch when all symbols are resolved inside the IL?
> 
> Thanks,
> Richard.
> 
> >   return 1;
> >  }
> >
> > Index: ipa.c
> > ===================================================================
> > --- ipa.c       (revision 160529)
> > +++ ipa.c       (working copy)
> > @@ -665,13 +665,12 @@
> >        }
> >       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node-
> >decl))
> >                  || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node-
> >decl));
> > -      if (cgraph_externally_visible_p (node, whole_program))
> > +      if (!node->local.externally_visible
> > +          && cgraph_externally_visible_p (node, whole_program))
> >         {
> >          gcc_assert (!node->global.inlined_to);
> >          node->local.externally_visible = true;
> >        }
> > -      else
> > -       node->local.externally_visible = false;
> >       if (!node->local.externally_visible && node->analyzed
> >          && !DECL_EXTERNAL (node->decl))
> >        {
> > @@ -721,7 +720,8 @@
> >     {
> >       if (!vnode->finalized)
> >         continue;
> > -      if (vnode->needed
> > +      if (!vnode->externally_visible
> > +          && vnode->needed
> >          && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
> >          && (!whole_program
> >              /* We can privatize comdat readonly variables whose
> address is not taken,
> > @@ -732,8 +732,6 @@
> >              || lookup_attribute ("externally_visible",
> >                                   DECL_ATTRIBUTES (vnode->decl))))
> >        vnode->externally_visible = true;
> > -      else
> > -        vnode->externally_visible = false;
> >       if (!vnode->externally_visible)
> >        {
> >          gcc_assert (whole_program || !TREE_PUBLIC (vnode->decl));
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 09 June 2010 16:26
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> >> > I added an attribute because -fwhole-program/externally_visible is
> >> handled in ipa.c
> >> >
> >> > ...
> >> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES
> (node-
> >> >decl)))
> >> >    return true;
> >> > ...
> >> >
> >> > Adding attribute seems cleaner than changing flags, otherwise I
> need
> >> to change
> >> > handling in ipa.c as well.
> >>
> >> True, but there is an externally_visible flag in cgraph_node,
> >> so I guess that attribute lookup is bogus.
> >>
> >> Richard.
> >>
> >> > Bingfeng
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> >> Sent: 09 June 2010 16:02
> >> >> To: Bingfeng Mei
> >> >> Cc: gcc-patches@gcc.gnu.org
> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> >> necessary with -fwhole-program and resolution file.
> >> >>
> >> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
> >> wrote:
> >> >> > Hi,
> >> >> > This patch addresses issue discussed in
> >> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >> >
> >> >> > With the patch, any declaration which is resolved as
> >> >> LDPR_PREVAILING_DEF
> >> >> > and compiled with -fwhole-program is annotated with
> >> >> > attribute "externally_visible" if it doesn't exist already.
> >> >> > This eliminates the error-prone process of manual annotation
> >> >> > of the attribute when compiling mixed LTO/non-LTO applications.
> >> >> >
> >> >> > For the following test files:
> >> >> > a.c
> >> >> >
> >> >> > #include <string.h>
> >> >> > #include <stdio.h>
> >> >> > extern int foo(int);
> >> >> > /* Called by b.c, should not be optimized by -fwhole-program */
> >> >> > void bar()
> >> >> > {
> >> >> >  printf("bar\n");
> >> >> > }
> >> >> > /* Not used by others, should be optimized out by -fwhole-
> >> program*/
> >> >> > void bar2()
> >> >> > {
> >> >> >  printf("bar2\n");
> >> >> > }
> >> >> > extern int src[], dst[];
> >> >> > int vvvvvv;
> >> >> > int main()
> >> >> > {
> >> >> >  int ret;
> >> >> >  vvvvvv = 12;
> >> >> >  ret = foo(20);
> >> >> >  bar2();
> >> >> >  memcpy(dst, src, 100);
> >> >> >  return ret + 3;
> >> >> > }
> >> >> >
> >> >> > b.c
> >> >> >
> >> >> > #include <stdio.h>
> >> >> > int src[100];
> >> >> > int dst[100];
> >> >> > extern int vvvvvv;
> >> >> > extern void bar();
> >> >> > int foo(int c)
> >> >> > {
> >> >> >  printf("Hello world: %d\n", c);
> >> >> >  bar();
> >> >> >  return 1000 + vvvvvv;
> >> >> > }
> >> >> >
> >> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> >> > ar cru libb.a b.o
> >> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
> >> plugin
> >> >> -o f -fwhole-program
> >> >> >
> >> >> > The code is compiled and linked correctly. bar & vvvvvv don't
> >> become
> >> >> static function
> >> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >> >
> >> >> >
> >> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >> >
> >> >> > Cheers,
> >> >> > Bingfeng Mei
> >> >> >
> >> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >> >
> >> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> >> externally_visible
> >> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> >> compiling with
> >> >> >        -fwhole-program
> >> >> >
> >> >> >
> >> >> > Index: lto-symtab.c
> >> >> >
> >> ===================================================================
> >> >> > --- lto-symtab.c        (revision 160463)
> >> >> > +++ lto-symtab.c        (working copy)
> >> >> > @@ -476,7 +476,19 @@
> >> >> >
> >> >> >   /* If the chain is already resolved there is nothing else to
> >> >> do.  */
> >> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> >> > -    return;
> >> >> > +    {
> >> >> > +      /* Add externally_visible attribute for declaration of
> >> >> LDPR_PREVAILING_DEF */
> >> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> >> flag_whole_program)
> >> >> > +        {
> >> >> > +          if (!lookup_attribute ("externally_visible",
> >> >> DECL_ATTRIBUTES (e->decl)))
> >> >> > +            {
> >> >> > +              DECL_ATTRIBUTES (e->decl)
> >> >> > +                = tree_cons (get_identifier
> >> ("externally_visible"),
> >> >> NULL_TREE,
> >> >> > +                             DECL_ATTRIBUTES (e->decl));
> >> >>
> >> >> I don't think we need to add an attribute here but we can play
> >> >> with some cgraph flags which is cheaper.
> >> >>
> >> >> Also I think this isn't really correct - not everything that
> >> prevails
> >> >> needs to be externally visible (in fact, you seem to simply
> >> >> remove the effect of -fwhole-program completely).
> >> >>
> >> >> A testcase that should still work is
> >> >>
> >> >> t1.c:
> >> >> void foo(void) { bar (); }
> >> >> t2.c
> >> >> extern void foo (void);
> >> >> void bar (void) {}
> >> >> void eliminate_me (void) {}
> >> >> int main()
> >> >> {
> >> >>    foo();
> >> >> }
> >> >>
> >> >> and eliminate_me should still be eliminated with -fwhole-program
> >> >> if you do
> >> >>
> >> >> gcc -c t1.c
> >> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >> >>
> >> >> Thus, the resolution file probably does not have the information
> >> >> you need (a list of references from outside of the LTO file set).
> >> >>
> >> >> Richard.
> >> >
> >> >
> >> >
> >
> >
> >


[-- Attachment #2: b.c --]
[-- Type: text/plain, Size: 171 bytes --]

#include <stdio.h>
int src[100];
int dst[100];
extern int vvvvvv;
extern void bar();
int foo(int c)
{
  printf("Hello world: %d\n", c);
  bar();
  return 1000 + vvvvvv;
}

[-- Attachment #3: a.c --]
[-- Type: text/plain, Size: 417 bytes --]

#include <string.h>
#include <stdio.h>
extern int foo(int);
/* Called by b.c, should not be optimized by -fwhole-program */
void bar() 
{
  printf("bar\n");
}  
/* Not used by others, should be optimized out by -fwhole-program*/
void bar2()
{
  printf("bar2\n");
}
extern int src[], dst[];
int vvvvvv;
int main()
{ 
  int ret;
  vvvvvv = 12; 
  ret = foo(20);
  bar2();
  memcpy(dst, src, 100);
  return ret + 3;
}  

RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

[-- Attachment #1: Type: text/plain, Size: 12098 bytes --]

Honza,
Could you have a look at cgraph related changes as suggested by Richard? 
Thanks!

Bingfeng
> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 14 June 2010 10:25
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Mon, Jun 14, 2010 at 10:39 AM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> > Hi, Richard,
> > Here is the updated patch. The flags are set instead of attributes
> now.
> > The check is moved to the end of lto_symtab_merge_decls_1. For the
> DECL_COMM,
> > since internal resolver is always used due to your workaround for
> gold plugin,
> > These variables would still need explicit externally_visible
> attributes.
> 
> Can you amend the docs that talk about -flto + -fwhole-program
> accordingly?
> 
> I'd like Honza to go over the cgraph related changes, otherwise
> the patch looks good with ...
> 
> > Bootstrapped and tested.
> >
> > Cheers,
> > Bingfeng.
> >
> > 2010-06-11  Bingfeng Mei <bmei@broadcom.com>
> >
> >        * lto-symbtab.c (lto_symtab_merge_decls_1): Set
> externally_visible
> >        flags for symbols of LDPR_PREVAILING_DEF when compiling with
> >        -fwhole-program
> >        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
> for
> >        internal resolver
> >        * ipa.c (function_and_variable_visibility): Set
> externally_visible
> >        flags only if they are false. This allows flags to be passed
> from
> >
> >
> > Index: lto-symtab.c
> > ===================================================================
> > --- lto-symtab.c        (revision 160529)
> > +++ lto-symtab.c        (working copy)
> > @@ -530,11 +530,21 @@
> >     return;
> >
> >  found:
> > -  if (TREE_CODE (prevailing->decl) == VAR_DECL
> > -      && TREE_READONLY (prevailing->decl))
> > +  /* If current lto files represent the whole program,
> > +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
> > +    If current lto files are part of whole program, internal
> > +    resolver doesn't know if it is LDPR_PREVAILING_DEF
> > +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
> > +    using -fwhole-program. Otherwise, it doesn't
> > +    matter using either LDPR_PREVAILING_DEF or
> > +    LDPR_PREVAILING_DEF_IRONLY
> > +
> > +    FIXME: above workaround due to gold plugin makes some
> > +    variables IRONLY, which are indeed PREVAILING_DEF in
> > +    resolution file. These variables still need manual
> > +    externally_visible attribute
> > +    */
> 
> Full-stop at the end of the sentence, */ not on the next line.  Also
> two spaces after each '.'
> 
> >     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> > -  else
> > -    prevailing->resolution = LDPR_PREVAILING_DEF;
> >  }
> >
> >  /* Merge all decls in the symbol table chain to the prevailing decl
> and
> > @@ -698,6 +708,25 @@
> >       && TREE_CODE (prevailing->decl) != VAR_DECL)
> >     prevailing->next = NULL;
> >
> > +
> 
> No extra vertical space please.
> 
> > +  /* Add externally_visible attribute for declaration of
> LDPR_PREVAILING_DEF */
> 
> Adjust the comment, to sth like "In whole-program mode mark
> LDPR_PREVAILING_DEFs as externally visible. "
> 
> > +  if (flag_whole_program)
> > +    {
> > +      if (prevailing->resolution == LDPR_PREVAILING_DEF)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = true;
> > +          else
> > +            prevailing->vnode->externally_visible = true;
> > +        }
> > +      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = false;
> > +          else
> > +            prevailing->vnode->externally_visible = false;
> > +        }
> > +    }
> 
> Honza will tell you if the above is correct, I am not 100% sure.
> 
> Did you verify we generate the same code with and without your
> patch when all symbols are resolved inside the IL?
> 
> Thanks,
> Richard.
> 
> >   return 1;
> >  }
> >
> > Index: ipa.c
> > ===================================================================
> > --- ipa.c       (revision 160529)
> > +++ ipa.c       (working copy)
> > @@ -665,13 +665,12 @@
> >        }
> >       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node-
> >decl))
> >                  || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node-
> >decl));
> > -      if (cgraph_externally_visible_p (node, whole_program))
> > +      if (!node->local.externally_visible
> > +          && cgraph_externally_visible_p (node, whole_program))
> >         {
> >          gcc_assert (!node->global.inlined_to);
> >          node->local.externally_visible = true;
> >        }
> > -      else
> > -       node->local.externally_visible = false;
> >       if (!node->local.externally_visible && node->analyzed
> >          && !DECL_EXTERNAL (node->decl))
> >        {
> > @@ -721,7 +720,8 @@
> >     {
> >       if (!vnode->finalized)
> >         continue;
> > -      if (vnode->needed
> > +      if (!vnode->externally_visible
> > +          && vnode->needed
> >          && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
> >          && (!whole_program
> >              /* We can privatize comdat readonly variables whose
> address is not taken,
> > @@ -732,8 +732,6 @@
> >              || lookup_attribute ("externally_visible",
> >                                   DECL_ATTRIBUTES (vnode->decl))))
> >        vnode->externally_visible = true;
> > -      else
> > -        vnode->externally_visible = false;
> >       if (!vnode->externally_visible)
> >        {
> >          gcc_assert (whole_program || !TREE_PUBLIC (vnode->decl));
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 09 June 2010 16:26
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> >> > I added an attribute because -fwhole-program/externally_visible is
> >> handled in ipa.c
> >> >
> >> > ...
> >> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES
> (node-
> >> >decl)))
> >> >    return true;
> >> > ...
> >> >
> >> > Adding attribute seems cleaner than changing flags, otherwise I
> need
> >> to change
> >> > handling in ipa.c as well.
> >>
> >> True, but there is an externally_visible flag in cgraph_node,
> >> so I guess that attribute lookup is bogus.
> >>
> >> Richard.
> >>
> >> > Bingfeng
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> >> Sent: 09 June 2010 16:02
> >> >> To: Bingfeng Mei
> >> >> Cc: gcc-patches@gcc.gnu.org
> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> >> necessary with -fwhole-program and resolution file.
> >> >>
> >> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
> >> wrote:
> >> >> > Hi,
> >> >> > This patch addresses issue discussed in
> >> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >> >
> >> >> > With the patch, any declaration which is resolved as
> >> >> LDPR_PREVAILING_DEF
> >> >> > and compiled with -fwhole-program is annotated with
> >> >> > attribute "externally_visible" if it doesn't exist already.
> >> >> > This eliminates the error-prone process of manual annotation
> >> >> > of the attribute when compiling mixed LTO/non-LTO applications.
> >> >> >
> >> >> > For the following test files:
> >> >> > a.c
> >> >> >
> >> >> > #include <string.h>
> >> >> > #include <stdio.h>
> >> >> > extern int foo(int);
> >> >> > /* Called by b.c, should not be optimized by -fwhole-program */
> >> >> > void bar()
> >> >> > {
> >> >> >  printf("bar\n");
> >> >> > }
> >> >> > /* Not used by others, should be optimized out by -fwhole-
> >> program*/
> >> >> > void bar2()
> >> >> > {
> >> >> >  printf("bar2\n");
> >> >> > }
> >> >> > extern int src[], dst[];
> >> >> > int vvvvvv;
> >> >> > int main()
> >> >> > {
> >> >> >  int ret;
> >> >> >  vvvvvv = 12;
> >> >> >  ret = foo(20);
> >> >> >  bar2();
> >> >> >  memcpy(dst, src, 100);
> >> >> >  return ret + 3;
> >> >> > }
> >> >> >
> >> >> > b.c
> >> >> >
> >> >> > #include <stdio.h>
> >> >> > int src[100];
> >> >> > int dst[100];
> >> >> > extern int vvvvvv;
> >> >> > extern void bar();
> >> >> > int foo(int c)
> >> >> > {
> >> >> >  printf("Hello world: %d\n", c);
> >> >> >  bar();
> >> >> >  return 1000 + vvvvvv;
> >> >> > }
> >> >> >
> >> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> >> > ar cru libb.a b.o
> >> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
> >> plugin
> >> >> -o f -fwhole-program
> >> >> >
> >> >> > The code is compiled and linked correctly. bar & vvvvvv don't
> >> become
> >> >> static function
> >> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >> >
> >> >> >
> >> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >> >
> >> >> > Cheers,
> >> >> > Bingfeng Mei
> >> >> >
> >> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >> >
> >> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> >> externally_visible
> >> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> >> compiling with
> >> >> >        -fwhole-program
> >> >> >
> >> >> >
> >> >> > Index: lto-symtab.c
> >> >> >
> >> ===================================================================
> >> >> > --- lto-symtab.c        (revision 160463)
> >> >> > +++ lto-symtab.c        (working copy)
> >> >> > @@ -476,7 +476,19 @@
> >> >> >
> >> >> >   /* If the chain is already resolved there is nothing else to
> >> >> do.  */
> >> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> >> > -    return;
> >> >> > +    {
> >> >> > +      /* Add externally_visible attribute for declaration of
> >> >> LDPR_PREVAILING_DEF */
> >> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> >> flag_whole_program)
> >> >> > +        {
> >> >> > +          if (!lookup_attribute ("externally_visible",
> >> >> DECL_ATTRIBUTES (e->decl)))
> >> >> > +            {
> >> >> > +              DECL_ATTRIBUTES (e->decl)
> >> >> > +                = tree_cons (get_identifier
> >> ("externally_visible"),
> >> >> NULL_TREE,
> >> >> > +                             DECL_ATTRIBUTES (e->decl));
> >> >>
> >> >> I don't think we need to add an attribute here but we can play
> >> >> with some cgraph flags which is cheaper.
> >> >>
> >> >> Also I think this isn't really correct - not everything that
> >> prevails
> >> >> needs to be externally visible (in fact, you seem to simply
> >> >> remove the effect of -fwhole-program completely).
> >> >>
> >> >> A testcase that should still work is
> >> >>
> >> >> t1.c:
> >> >> void foo(void) { bar (); }
> >> >> t2.c
> >> >> extern void foo (void);
> >> >> void bar (void) {}
> >> >> void eliminate_me (void) {}
> >> >> int main()
> >> >> {
> >> >>    foo();
> >> >> }
> >> >>
> >> >> and eliminate_me should still be eliminated with -fwhole-program
> >> >> if you do
> >> >>
> >> >> gcc -c t1.c
> >> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >> >>
> >> >> Thus, the resolution file probably does not have the information
> >> >> you need (a list of references from outside of the LTO file set).
> >> >>
> >> >> Richard.
> >> >
> >> >
> >> >
> >
> >
> >


[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 5569 bytes --]

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 160529)
+++ doc/extend.texi	(working copy)
@@ -2296,7 +2296,7 @@
 @cindex @code{externally_visible} attribute.
 This attribute, attached to a global variable or function, nullifies
 the effect of the @option{-fwhole-program} command-line option, so the
-object remains visible outside the current compilation unit.
+object remains visible outside the current compilation unit. If @option{-fwhole-program} is used together with @option{-flto} and @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 
 @item far
 @cindex functions which handle memory bank switching
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 160529)
+++ doc/invoke.texi	(working copy)
@@ -7294,7 +7294,7 @@
 Assume that the current compilation unit represents the whole program being
 compiled.  All public functions and variables with the exception of @code{main}
 and those merged by attribute @code{externally_visible} become static functions
-and in effect are optimized more aggressively by interprocedural optimizers.
+and in effect are optimized more aggressively by interprocedural optimizers. If @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 While this option is equivalent to proper use of the @code{static} keyword for
 programs consisting of a single file, in combination with option
 @option{-combine}, @option{-flto} or @option{-fwhopr} this flag can be used to
Index: lto-symtab.c
===================================================================
--- lto-symtab.c	(revision 160529)
+++ lto-symtab.c	(working copy)
@@ -530,11 +530,20 @@
     return;
 
 found:
-  if (TREE_CODE (prevailing->decl) == VAR_DECL
-      && TREE_READONLY (prevailing->decl))
+  /* If current lto files represent the whole program,
+    it is correct to use LDPR_PREVALING_DEF_IRONLY.
+    If current lto files are part of whole program, internal
+    resolver doesn't know if it is LDPR_PREVAILING_DEF
+    or LDPR_PREVAILING_DEF_IRONLY.  Use IRONLY conforms to
+    using -fwhole-program.  Otherwise, it doesn't
+    matter using either LDPR_PREVAILING_DEF or
+    LDPR_PREVAILING_DEF_IRONLY.
+    
+    FIXME: above workaround due to gold plugin makes some
+    variables IRONLY, which are indeed PREVAILING_DEF in
+    resolution file.  These variables still need manual
+    externally_visible attribute.  */
     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
-  else
-    prevailing->resolution = LDPR_PREVAILING_DEF;
 }
 
 /* Merge all decls in the symbol table chain to the prevailing decl and
@@ -698,6 +707,24 @@
       && TREE_CODE (prevailing->decl) != VAR_DECL)
     prevailing->next = NULL;
 
+  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
+  if (flag_whole_program)
+    {
+      if (prevailing->resolution == LDPR_PREVAILING_DEF)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = true;
+          else
+            prevailing->vnode->externally_visible = true;
+        }
+      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = false;
+          else
+            prevailing->vnode->externally_visible = false;
+        }
+    }    
   return 1;
 }
 
Index: ipa.c
===================================================================
--- ipa.c	(revision 160529)
+++ ipa.c	(working copy)
@@ -665,13 +665,12 @@
 	}
       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl))
       	          || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl));
-      if (cgraph_externally_visible_p (node, whole_program))
+      if (!node->local.externally_visible
+          && cgraph_externally_visible_p (node, whole_program))
         {
 	  gcc_assert (!node->global.inlined_to);
 	  node->local.externally_visible = true;
 	}
-      else
-	node->local.externally_visible = false;
       if (!node->local.externally_visible && node->analyzed
 	  && !DECL_EXTERNAL (node->decl))
 	{
@@ -721,7 +720,8 @@
     {
       if (!vnode->finalized)
         continue;
-      if (vnode->needed
+      if (!vnode->externally_visible
+          && vnode->needed
 	  && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
 	  && (!whole_program
 	      /* We can privatize comdat readonly variables whose address is not taken,
@@ -732,8 +732,6 @@
 	      || lookup_attribute ("externally_visible",
 				   DECL_ATTRIBUTES (vnode->decl))))
 	vnode->externally_visible = true;
-      else
-        vnode->externally_visible = false;
       if (!vnode->externally_visible)
 	{
 	  gcc_assert (whole_program || !TREE_PUBLIC (vnode->decl));

PING: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

[-- Attachment #1: Type: text/plain, Size: 12105 bytes --]

Honza,
Could you have a look at the patch, especially on how cgraph related changes? 

Thanks,
Bingfeng

> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: 14 June 2010 10:25
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org; Jan Hubicka
> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> On Mon, Jun 14, 2010 at 10:39 AM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> > Hi, Richard,
> > Here is the updated patch. The flags are set instead of attributes
> now.
> > The check is moved to the end of lto_symtab_merge_decls_1. For the
> DECL_COMM,
> > since internal resolver is always used due to your workaround for
> gold plugin,
> > These variables would still need explicit externally_visible
> attributes.
> 
> Can you amend the docs that talk about -flto + -fwhole-program
> accordingly?
> 
> I'd like Honza to go over the cgraph related changes, otherwise
> the patch looks good with ...
> 
> > Bootstrapped and tested.
> >
> > Cheers,
> > Bingfeng.
> >
> > 2010-06-11  Bingfeng Mei <bmei@broadcom.com>
> >
> >        * lto-symbtab.c (lto_symtab_merge_decls_1): Set
> externally_visible
> >        flags for symbols of LDPR_PREVAILING_DEF when compiling with
> >        -fwhole-program
> >        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
> for
> >        internal resolver
> >        * ipa.c (function_and_variable_visibility): Set
> externally_visible
> >        flags only if they are false. This allows flags to be passed
> from
> >
> >
> > Index: lto-symtab.c
> > ===================================================================
> > --- lto-symtab.c        (revision 160529)
> > +++ lto-symtab.c        (working copy)
> > @@ -530,11 +530,21 @@
> >     return;
> >
> >  found:
> > -  if (TREE_CODE (prevailing->decl) == VAR_DECL
> > -      && TREE_READONLY (prevailing->decl))
> > +  /* If current lto files represent the whole program,
> > +    it is correct to use LDPR_PREVALING_DEF_IRONLY.
> > +    If current lto files are part of whole program, internal
> > +    resolver doesn't know if it is LDPR_PREVAILING_DEF
> > +    or LDPR_PREVAILING_DEF_IRONLY. Use IRONLY conforms to
> > +    using -fwhole-program. Otherwise, it doesn't
> > +    matter using either LDPR_PREVAILING_DEF or
> > +    LDPR_PREVAILING_DEF_IRONLY
> > +
> > +    FIXME: above workaround due to gold plugin makes some
> > +    variables IRONLY, which are indeed PREVAILING_DEF in
> > +    resolution file. These variables still need manual
> > +    externally_visible attribute
> > +    */
> 
> Full-stop at the end of the sentence, */ not on the next line.  Also
> two spaces after each '.'
> 
> >     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
> > -  else
> > -    prevailing->resolution = LDPR_PREVAILING_DEF;
> >  }
> >
> >  /* Merge all decls in the symbol table chain to the prevailing decl
> and
> > @@ -698,6 +708,25 @@
> >       && TREE_CODE (prevailing->decl) != VAR_DECL)
> >     prevailing->next = NULL;
> >
> > +
> 
> No extra vertical space please.
> 
> > +  /* Add externally_visible attribute for declaration of
> LDPR_PREVAILING_DEF */
> 
> Adjust the comment, to sth like "In whole-program mode mark
> LDPR_PREVAILING_DEFs as externally visible. "
> 
> > +  if (flag_whole_program)
> > +    {
> > +      if (prevailing->resolution == LDPR_PREVAILING_DEF)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = true;
> > +          else
> > +            prevailing->vnode->externally_visible = true;
> > +        }
> > +      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = false;
> > +          else
> > +            prevailing->vnode->externally_visible = false;
> > +        }
> > +    }
> 
> Honza will tell you if the above is correct, I am not 100% sure.
> 
> Did you verify we generate the same code with and without your
> patch when all symbols are resolved inside the IL?
> 
> Thanks,
> Richard.
> 
> >   return 1;
> >  }
> >
> > Index: ipa.c
> > ===================================================================
> > --- ipa.c       (revision 160529)
> > +++ ipa.c       (working copy)
> > @@ -665,13 +665,12 @@
> >        }
> >       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node-
> >decl))
> >                  || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node-
> >decl));
> > -      if (cgraph_externally_visible_p (node, whole_program))
> > +      if (!node->local.externally_visible
> > +          && cgraph_externally_visible_p (node, whole_program))
> >         {
> >          gcc_assert (!node->global.inlined_to);
> >          node->local.externally_visible = true;
> >        }
> > -      else
> > -       node->local.externally_visible = false;
> >       if (!node->local.externally_visible && node->analyzed
> >          && !DECL_EXTERNAL (node->decl))
> >        {
> > @@ -721,7 +720,8 @@
> >     {
> >       if (!vnode->finalized)
> >         continue;
> > -      if (vnode->needed
> > +      if (!vnode->externally_visible
> > +          && vnode->needed
> >          && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
> >          && (!whole_program
> >              /* We can privatize comdat readonly variables whose
> address is not taken,
> > @@ -732,8 +732,6 @@
> >              || lookup_attribute ("externally_visible",
> >                                   DECL_ATTRIBUTES (vnode->decl))))
> >        vnode->externally_visible = true;
> > -      else
> > -        vnode->externally_visible = false;
> >       if (!vnode->externally_visible)
> >        {
> >          gcc_assert (whole_program || !TREE_PUBLIC (vnode->decl));
> >
> >> -----Original Message-----
> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> Sent: 09 June 2010 16:26
> >> To: Bingfeng Mei
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> necessary with -fwhole-program and resolution file.
> >>
> >> On Wed, Jun 9, 2010 at 5:20 PM, Bingfeng Mei <bmei@broadcom.com>
> wrote:
> >> > I added an attribute because -fwhole-program/externally_visible is
> >> handled in ipa.c
> >> >
> >> > ...
> >> >  if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES
> (node-
> >> >decl)))
> >> >    return true;
> >> > ...
> >> >
> >> > Adding attribute seems cleaner than changing flags, otherwise I
> need
> >> to change
> >> > handling in ipa.c as well.
> >>
> >> True, but there is an externally_visible flag in cgraph_node,
> >> so I guess that attribute lookup is bogus.
> >>
> >> Richard.
> >>
> >> > Bingfeng
> >> >
> >> >> -----Original Message-----
> >> >> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> >> >> Sent: 09 June 2010 16:02
> >> >> To: Bingfeng Mei
> >> >> Cc: gcc-patches@gcc.gnu.org
> >> >> Subject: Re: [PATCH, LTO] add externally_visible attribute when
> >> >> necessary with -fwhole-program and resolution file.
> >> >>
> >> >> On Wed, Jun 9, 2010 at 4:46 PM, Bingfeng Mei <bmei@broadcom.com>
> >> wrote:
> >> >> > Hi,
> >> >> > This patch addresses issue discussed in
> >> >> > http://gcc.gnu.org/ml/gcc/2010-05/msg00560.html
> >> >> > http://gcc.gnu.org/ml/gcc/2010-06/msg00317.html
> >> >> >
> >> >> > With the patch, any declaration which is resolved as
> >> >> LDPR_PREVAILING_DEF
> >> >> > and compiled with -fwhole-program is annotated with
> >> >> > attribute "externally_visible" if it doesn't exist already.
> >> >> > This eliminates the error-prone process of manual annotation
> >> >> > of the attribute when compiling mixed LTO/non-LTO applications.
> >> >> >
> >> >> > For the following test files:
> >> >> > a.c
> >> >> >
> >> >> > #include <string.h>
> >> >> > #include <stdio.h>
> >> >> > extern int foo(int);
> >> >> > /* Called by b.c, should not be optimized by -fwhole-program */
> >> >> > void bar()
> >> >> > {
> >> >> >  printf("bar\n");
> >> >> > }
> >> >> > /* Not used by others, should be optimized out by -fwhole-
> >> program*/
> >> >> > void bar2()
> >> >> > {
> >> >> >  printf("bar2\n");
> >> >> > }
> >> >> > extern int src[], dst[];
> >> >> > int vvvvvv;
> >> >> > int main()
> >> >> > {
> >> >> >  int ret;
> >> >> >  vvvvvv = 12;
> >> >> >  ret = foo(20);
> >> >> >  bar2();
> >> >> >  memcpy(dst, src, 100);
> >> >> >  return ret + 3;
> >> >> > }
> >> >> >
> >> >> > b.c
> >> >> >
> >> >> > #include <stdio.h>
> >> >> > int src[100];
> >> >> > int dst[100];
> >> >> > extern int vvvvvv;
> >> >> > extern void bar();
> >> >> > int foo(int c)
> >> >> > {
> >> >> >  printf("Hello world: %d\n", c);
> >> >> >  bar();
> >> >> >  return 1000 + vvvvvv;
> >> >> > }
> >> >> >
> >> >> > ~/work/install-x86/bin/gcc  a.c -O2 -c  -flto
> >> >> > ~/work/install-x86/bin/gcc  b.c -O2 -c
> >> >> > ar cru libb.a b.o
> >> >> > ~/work/install-x86/bin/gcc -flto a.o -L. -lb -O2 -fuse-linker-
> >> plugin
> >> >> -o f -fwhole-program
> >> >> >
> >> >> > The code is compiled and linked correctly. bar & vvvvvv don't
> >> become
> >> >> static function
> >> >> > and cause link errors, whereas bar2 is inlined as expected.
> >> >> >
> >> >> >
> >> >> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >> >> >
> >> >> > Cheers,
> >> >> > Bingfeng Mei
> >> >> >
> >> >> > 2010-06-09  Bingfeng Mei <bmei@broadcom.com>
> >> >> >
> >> >> >        * lto-symbtab.c (lto_symtab_resolve_symbols): Add
> >> >> externally_visible
> >> >> >        attribute for declaration of LDPR_PREVAILING_DEF when
> >> >> compiling with
> >> >> >        -fwhole-program
> >> >> >
> >> >> >
> >> >> > Index: lto-symtab.c
> >> >> >
> >> ===================================================================
> >> >> > --- lto-symtab.c        (revision 160463)
> >> >> > +++ lto-symtab.c        (working copy)
> >> >> > @@ -476,7 +476,19 @@
> >> >> >
> >> >> >   /* If the chain is already resolved there is nothing else to
> >> >> do.  */
> >> >> >   if (e->resolution != LDPR_UNKNOWN)
> >> >> > -    return;
> >> >> > +    {
> >> >> > +      /* Add externally_visible attribute for declaration of
> >> >> LDPR_PREVAILING_DEF */
> >> >> > +      if (e->resolution == LDPR_PREVAILING_DEF &&
> >> flag_whole_program)
> >> >> > +        {
> >> >> > +          if (!lookup_attribute ("externally_visible",
> >> >> DECL_ATTRIBUTES (e->decl)))
> >> >> > +            {
> >> >> > +              DECL_ATTRIBUTES (e->decl)
> >> >> > +                = tree_cons (get_identifier
> >> ("externally_visible"),
> >> >> NULL_TREE,
> >> >> > +                             DECL_ATTRIBUTES (e->decl));
> >> >>
> >> >> I don't think we need to add an attribute here but we can play
> >> >> with some cgraph flags which is cheaper.
> >> >>
> >> >> Also I think this isn't really correct - not everything that
> >> prevails
> >> >> needs to be externally visible (in fact, you seem to simply
> >> >> remove the effect of -fwhole-program completely).
> >> >>
> >> >> A testcase that should still work is
> >> >>
> >> >> t1.c:
> >> >> void foo(void) { bar (); }
> >> >> t2.c
> >> >> extern void foo (void);
> >> >> void bar (void) {}
> >> >> void eliminate_me (void) {}
> >> >> int main()
> >> >> {
> >> >>    foo();
> >> >> }
> >> >>
> >> >> and eliminate_me should still be eliminated with -fwhole-program
> >> >> if you do
> >> >>
> >> >> gcc -c t1.c
> >> >> gcc -O2 t2.c t1.o -flto -fwhole-program -fuse-linker-plugin
> >> >>
> >> >> Thus, the resolution file probably does not have the information
> >> >> you need (a list of references from outside of the LTO file set).
> >> >>
> >> >> Richard.
> >> >
> >> >
> >> >
> >
> >
> >


[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 5569 bytes --]

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 160529)
+++ doc/extend.texi	(working copy)
@@ -2296,7 +2296,7 @@
 @cindex @code{externally_visible} attribute.
 This attribute, attached to a global variable or function, nullifies
 the effect of the @option{-fwhole-program} command-line option, so the
-object remains visible outside the current compilation unit.
+object remains visible outside the current compilation unit. If @option{-fwhole-program} is used together with @option{-flto} and @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 
 @item far
 @cindex functions which handle memory bank switching
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 160529)
+++ doc/invoke.texi	(working copy)
@@ -7294,7 +7294,7 @@
 Assume that the current compilation unit represents the whole program being
 compiled.  All public functions and variables with the exception of @code{main}
 and those merged by attribute @code{externally_visible} become static functions
-and in effect are optimized more aggressively by interprocedural optimizers.
+and in effect are optimized more aggressively by interprocedural optimizers. If @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 While this option is equivalent to proper use of the @code{static} keyword for
 programs consisting of a single file, in combination with option
 @option{-combine}, @option{-flto} or @option{-fwhopr} this flag can be used to
Index: lto-symtab.c
===================================================================
--- lto-symtab.c	(revision 160529)
+++ lto-symtab.c	(working copy)
@@ -530,11 +530,20 @@
     return;
 
 found:
-  if (TREE_CODE (prevailing->decl) == VAR_DECL
-      && TREE_READONLY (prevailing->decl))
+  /* If current lto files represent the whole program,
+    it is correct to use LDPR_PREVALING_DEF_IRONLY.
+    If current lto files are part of whole program, internal
+    resolver doesn't know if it is LDPR_PREVAILING_DEF
+    or LDPR_PREVAILING_DEF_IRONLY.  Use IRONLY conforms to
+    using -fwhole-program.  Otherwise, it doesn't
+    matter using either LDPR_PREVAILING_DEF or
+    LDPR_PREVAILING_DEF_IRONLY.
+    
+    FIXME: above workaround due to gold plugin makes some
+    variables IRONLY, which are indeed PREVAILING_DEF in
+    resolution file.  These variables still need manual
+    externally_visible attribute.  */
     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
-  else
-    prevailing->resolution = LDPR_PREVAILING_DEF;
 }
 
 /* Merge all decls in the symbol table chain to the prevailing decl and
@@ -698,6 +707,24 @@
       && TREE_CODE (prevailing->decl) != VAR_DECL)
     prevailing->next = NULL;
 
+  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
+  if (flag_whole_program)
+    {
+      if (prevailing->resolution == LDPR_PREVAILING_DEF)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = true;
+          else
+            prevailing->vnode->externally_visible = true;
+        }
+      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = false;
+          else
+            prevailing->vnode->externally_visible = false;
+        }
+    }    
   return 1;
 }
 
Index: ipa.c
===================================================================
--- ipa.c	(revision 160529)
+++ ipa.c	(working copy)
@@ -665,13 +665,12 @@
 	}
       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl))
       	          || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl));
-      if (cgraph_externally_visible_p (node, whole_program))
+      if (!node->local.externally_visible
+          && cgraph_externally_visible_p (node, whole_program))
         {
 	  gcc_assert (!node->global.inlined_to);
 	  node->local.externally_visible = true;
 	}
-      else
-	node->local.externally_visible = false;
       if (!node->local.externally_visible && node->analyzed
 	  && !DECL_EXTERNAL (node->decl))
 	{
@@ -721,7 +720,8 @@
     {
       if (!vnode->finalized)
         continue;
-      if (vnode->needed
+      if (!vnode->externally_visible
+          && vnode->needed
 	  && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
 	  && (!whole_program
 	      /* We can privatize comdat readonly variables whose address is not taken,
@@ -732,8 +732,6 @@
 	      || lookup_attribute ("externally_visible",
 				   DECL_ATTRIBUTES (vnode->decl))))
 	vnode->externally_visible = true;
-      else
-        vnode->externally_visible = false;
       if (!vnode->externally_visible)
 	{
 	  gcc_assert (whole_program || !TREE_PUBLIC (vnode->decl));

Re: PING: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Jan Hubicka]

Index: ipa.c
===================================================================
--- ipa.c	(revision 160529)
+++ ipa.c	(working copy)
@@ -665,13 +665,12 @@
 	}
       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl))
       	          || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl));
-      if (cgraph_externally_visible_p (node, whole_program))
+      if (!node->local.externally_visible
+          && cgraph_externally_visible_p (node, whole_program))
         {
 	  gcc_assert (!node->global.inlined_to);
 	  node->local.externally_visible = true;
 	}
-      else
-	node->local.externally_visible = false;

This is not correct: the externally visible attribute is computed twice; once
in pass_ipa_function_and_variable_visibility and second time in
pass_ipa_whole_program_visibility.

This is so to allow output from early IPA passes and early local optimization
to be used by both at the compile time and link time.  So variables/functions
are externally visible per their visiblities first and then in
pass_ipa_whole_program_visibility they might become static.

This is why the conditional sets externally_visible to false.  Not doing so you
effectively disbale -fwhole-program.

We might get around by not streaming externally_visible flag and not clearling
it when in LTO mode (since it would not be set for other reasons) but this is
bit fragile.

We already have flag used_from_other_partition, this situation is similar, just
it is not used from other partition but externally.  Perhaps we can just add specific
flag for this purpose and update cgraph_externally_visible_p to test for it?

Honza

RE: PING: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

> -----Original Message-----
> From: Jan Hubicka [mailto:hubicka@ucw.cz]
> Sent: 18 June 2010 15:49
> To: Bingfeng Mei
> Cc: Jan Hubicka; gcc-patches@gcc.gnu.org; Richard Guenther
> Subject: Re: PING: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> Index: ipa.c
> ===================================================================
> --- ipa.c	(revision 160529)
> +++ ipa.c	(working copy)
> @@ -665,13 +665,12 @@
>  	}
>        gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node-
> >decl))
>        	          || TREE_PUBLIC (node->decl) || DECL_EXTERNAL
> (node->decl));
> -      if (cgraph_externally_visible_p (node, whole_program))
> +      if (!node->local.externally_visible
> +          && cgraph_externally_visible_p (node, whole_program))
>          {
>  	  gcc_assert (!node->global.inlined_to);
>  	  node->local.externally_visible = true;
>  	}
> -      else
> -	node->local.externally_visible = false;
> 
> This is not correct: the externally visible attribute is computed twice;
> once
> in pass_ipa_function_and_variable_visibility and second time in
> pass_ipa_whole_program_visibility.
> 
> This is so to allow output from early IPA passes and early local
> optimization
> to be used by both at the compile time and link time.  So
> variables/functions
> are externally visible per their visiblities first and then in
> pass_ipa_whole_program_visibility they might become static.
> 
> This is why the conditional sets externally_visible to false.  Not
> doing so you
> effectively disbale -fwhole-program.

Actually, the externally_visible flags will always be set to either
true or false with -fwhole-program due to following code. The unpacked
values from LTO bytecode won't be passed though to effectively 
disable -fwhole-program.

+  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
+  if (flag_whole_program)
+    {
+      if (prevailing->resolution == LDPR_PREVAILING_DEF)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = true;
+          else
+            prevailing->vnode->externally_visible = true;
+        }
+      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible = false;
+          else
+            prevailing->vnode->externally_visible = false;
+        }
+    }

The difference from original code is that when no -fwhole-program specified, the true
value of externally_visible (from pass_ipa_function_and_variable_visibility)
will be passed on and not conditionally set to false in pass_ipa_whole_program_visibility.
But does that matter? Cgraph_externally_visible_p will always return true then.

I rely on the facts that externally_visible flags are initialized to false
at the beginning. Is this reasonable?

> 
> We might get around by not streaming externally_visible flag and not
> clearling
> it when in LTO mode (since it would not be set for other reasons) but
> this is
> bit fragile.
> 
> We already have flag used_from_other_partition, this situation is
> similar, just
> it is not used from other partition but externally.  Perhaps we can
> just add specific
> flag for this purpose and update cgraph_externally_visible_p to test
> for it?
> 

> Honza


Thanks,
Bingfeng

Re: PING: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Jan Hubicka]

> +  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
> +  if (flag_whole_program)
> +    {
> +      if (prevailing->resolution == LDPR_PREVAILING_DEF)
> +        {
> +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> +            prevailing->node->local.externally_visible = true;
> +          else
> +            prevailing->vnode->externally_visible = true;
> +        }
> +      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
> +        {
> +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> +            prevailing->node->local.externally_visible = false;
> +          else
> +            prevailing->vnode->externally_visible = false;
> +        }
> +    }

This is executed only with LTO and -fwhole-program, so you would make non-LTO
-fwhole-program noop.
> 
> The difference from original code is that when no -fwhole-program specified, the true
> value of externally_visible (from pass_ipa_function_and_variable_visibility)
> will be passed on and not conditionally set to false in pass_ipa_whole_program_visibility.
> But does that matter? Cgraph_externally_visible_p will always return true then.

Without -fwhole-program the visibilities should match, so this is not a
problem, but we need to keep non-LTO -fwhole-program working.  It seems that
having specific flag for this case is better than trying to play rather fragile
games with setting/preserving the flag based on compliation mode.

It is useful to know if the symbol is bound from non-LTO object at linktime
also to make hidden symbols to become local when building the final DSO even
without -fwhole-program.

Honza

RE: PING: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

[-- Attachment #1: Type: text/plain, Size: 3521 bytes --]

Honza, 

As you suggested, I added "externally_visible_by_resolver" flags only for this purpose. 
The relevant places in ipa.c are updated to check this flag. It passes tests
and is bootstrapped. OK for trunk?

Thanks,
Bingfeng

2010-06-21  Bingfeng Mei <bmei@broadcom.com>

        * lto-symbtab.c (lto_symtab_merge_decls_1): Set
        externally_visible_by_resolver flags for symbols of
        LDPR_PREVAILING_DEF when compiling with -fwhole-program.
        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for
	internal resolver.
        * ipa.c (function_and_variable_visibility): Set externally_visible
        flag of varpool_node if externally_visible_by_resolver flag is set.
        (cgraph_externally_visible_p): check externally_visible_by_resolver
        flag.
        * cgraph.h (struct varpool_node): new externally_visible_by_resolver
        flag.  (struct cgraph_local_info): new externally_visible_by_resolver
        flag.
        * cgraph.c (dump_cgraph_node): dump externally_visible_by_resolver
        flag.  (cgraph_clone_node): initialize externally_visible_by_resolver.
        (cgraph_create_virtual_clone): initialize externally_visible_by_resolver.
        * doc/invoke.texi (-fwhole-program option): Change description of
        externally_visible attribute accordingly.
        * doc/extend.texi (externally_visible): Ditto.

> -----Original Message-----
> From: Jan Hubicka [mailto:hubicka@ucw.cz]
> Sent: 18 June 2010 17:39
> To: Bingfeng Mei
> Cc: Jan Hubicka; gcc-patches@gcc.gnu.org; Richard Guenther
> Subject: Re: PING: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> > +  /* Set externally_visible flags for declaration of
> LDPR_PREVAILING_DEF */
> > +  if (flag_whole_program)
> > +    {
> > +      if (prevailing->resolution == LDPR_PREVAILING_DEF)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = true;
> > +          else
> > +            prevailing->vnode->externally_visible = true;
> > +        }
> > +      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
> > +        {
> > +          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
> > +            prevailing->node->local.externally_visible = false;
> > +          else
> > +            prevailing->vnode->externally_visible = false;
> > +        }
> > +    }
> 
> This is executed only with LTO and -fwhole-program, so you would make
> non-LTO
> -fwhole-program noop.
> >
> > The difference from original code is that when no -fwhole-program
> specified, the true
> > value of externally_visible (from
> pass_ipa_function_and_variable_visibility)
> > will be passed on and not conditionally set to false in
> pass_ipa_whole_program_visibility.
> > But does that matter? Cgraph_externally_visible_p will always return
> true then.
> 
> Without -fwhole-program the visibilities should match, so this is not a
> problem, but we need to keep non-LTO -fwhole-program working.  It seems
> that
> having specific flag for this case is better than trying to play rather
> fragile
> games with setting/preserving the flag based on compliation mode.
> 
> It is useful to know if the symbol is bound from non-LTO object at
> linktime
> also to make hidden symbols to become local when building the final DSO
> even
> without -fwhole-program.
> 
> Honza


[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 6937 bytes --]

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 161065)
+++ doc/extend.texi	(working copy)
@@ -2296,7 +2296,7 @@
 @cindex @code{externally_visible} attribute.
 This attribute, attached to a global variable or function, nullifies
 the effect of the @option{-fwhole-program} command-line option, so the
-object remains visible outside the current compilation unit.
+object remains visible outside the current compilation unit. If @option{-fwhole-program} is used together with @option{-flto} and @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 
 @item far
 @cindex functions which handle memory bank switching
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 161065)
+++ doc/invoke.texi	(working copy)
@@ -7294,7 +7294,7 @@
 Assume that the current compilation unit represents the whole program being
 compiled.  All public functions and variables with the exception of @code{main}
 and those merged by attribute @code{externally_visible} become static functions
-and in effect are optimized more aggressively by interprocedural optimizers.
+and in effect are optimized more aggressively by interprocedural optimizers. If @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 While this option is equivalent to proper use of the @code{static} keyword for
 programs consisting of a single file, in combination with option
 @option{-combine}, @option{-flto} or @option{-fwhopr} this flag can be used to
Index: lto-symtab.c
===================================================================
--- lto-symtab.c	(revision 161065)
+++ lto-symtab.c	(working copy)
@@ -530,11 +530,20 @@
     return;
 
 found:
-  if (TREE_CODE (prevailing->decl) == VAR_DECL
-      && TREE_READONLY (prevailing->decl))
+  /* If current lto files represent the whole program,
+    it is correct to use LDPR_PREVALING_DEF_IRONLY.
+    If current lto files are part of whole program, internal
+    resolver doesn't know if it is LDPR_PREVAILING_DEF
+    or LDPR_PREVAILING_DEF_IRONLY.  Use IRONLY conforms to
+    using -fwhole-program.  Otherwise, it doesn't
+    matter using either LDPR_PREVAILING_DEF or
+    LDPR_PREVAILING_DEF_IRONLY
+    
+    FIXME: above workaround due to gold plugin makes some
+    variables IRONLY, which are indeed PREVAILING_DEF in
+    resolution file.  These variables still need manual
+    externally_visible attribute.  */
     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
-  else
-    prevailing->resolution = LDPR_PREVAILING_DEF;
 }
 
 /* Merge all decls in the symbol table chain to the prevailing decl and
@@ -698,6 +707,24 @@
       && TREE_CODE (prevailing->decl) != VAR_DECL)
     prevailing->next = NULL;
 
+  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
+  if (flag_whole_program)
+    {
+      if (prevailing->resolution == LDPR_PREVAILING_DEF)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible_by_resolver = true;
+          else
+            prevailing->vnode->externally_visible_by_resolver = true;
+        }
+      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible_by_resolver = false;
+          else
+            prevailing->vnode->externally_visible_by_resolver = false;
+        }
+    }
   return 1;
 }
 
Index: cgraph.c
===================================================================
--- cgraph.c	(revision 161065)
+++ cgraph.c	(working copy)
@@ -1825,6 +1825,8 @@
     fprintf (f, " local");
   if (node->local.externally_visible)
     fprintf (f, " externally_visible");
+  if (node->local.externally_visible_by_resolver)
+    fprintf (f, " externally_visible_by_resolver");
   if (node->local.finalized)
     fprintf (f, " finalized");
   if (node->local.disregard_inline_limits)
@@ -2075,6 +2077,7 @@
   new_node->analyzed = n->analyzed;
   new_node->local = n->local;
   new_node->local.externally_visible = false;
+  new_node->local.externally_visible_by_resolver = false;
   new_node->local.local = true;
   new_node->local.vtable_method = false;
   new_node->global = n->global;
@@ -2266,6 +2269,7 @@
   else
     new_node->clone.combined_args_to_skip = args_to_skip;
   new_node->local.externally_visible = 0;
+  new_node->local.externally_visible_by_resolver = 0;
   new_node->local.local = 1;
   new_node->lowered = true;
   new_node->reachable = true;
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 161065)
+++ cgraph.h	(working copy)
@@ -102,6 +102,9 @@
   /* Set when function is visible by other units.  */
   unsigned externally_visible : 1;
 
+  /* Set when resolver determines that function is visible by other units.  */
+  unsigned externally_visible_by_resolver : 1;
+  
   /* Set once it has been finalized so we consider it to be output.  */
   unsigned finalized : 1;
 
@@ -487,6 +490,8 @@
   unsigned output : 1;
   /* Set when function is visible by other units.  */
   unsigned externally_visible : 1;
+  /* Set when resolver determines that variable is visible by other units.  */
+  unsigned externally_visible_by_resolver : 1;
   /* Set for aliases once they got through assemble_alias.  Also set for
      extra name aliases in varpool_extra_name_alias.  */
   unsigned alias : 1;
Index: ipa.c
===================================================================
--- ipa.c	(revision 161065)
+++ ipa.c	(working copy)
@@ -576,6 +576,8 @@
     return false;
   if (!whole_program)
     return true;
+  if (node->local.externally_visible_by_resolver)
+    return true;
   if (DECL_PRESERVE_P (node->decl))
     return true;
   /* COMDAT functions must be shared only if they have address taken,
@@ -729,6 +731,7 @@
 	         we start reordering datastructures.  */
 	      || DECL_COMDAT (vnode->decl)
 	      || DECL_WEAK (vnode->decl)
+              || vnode->externally_visible_by_resolver
 	      || lookup_attribute ("externally_visible",
 				   DECL_ATTRIBUTES (vnode->decl))))
 	vnode->externally_visible = true;

PING: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]

Honza, 

Could you comment on the modified patch? As you suggested, I added 
new "externally_visible_by_resolver" flags only for this purpose. 
The relevant places in ipa.c are updated to check this flag. It passes tests
and is bootstrapped. OK for trunk?

Thanks,
Bingfeng

2010-06-21  Bingfeng Mei <bmei@broadcom.com>

        * lto-symbtab.c (lto_symtab_merge_decls_1): Set
        externally_visible_by_resolver flags for symbols of
        LDPR_PREVAILING_DEF when compiling with -fwhole-program.
        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for
	internal resolver.
        * ipa.c (function_and_variable_visibility): Set externally_visible
        flag of varpool_node if externally_visible_by_resolver flag is set.
        (cgraph_externally_visible_p): check externally_visible_by_resolver
        flag.
        * cgraph.h (struct varpool_node): new externally_visible_by_resolver
        flag.  (struct cgraph_local_info): new externally_visible_by_resolver
        flag.
        * cgraph.c (dump_cgraph_node): dump externally_visible_by_resolver
        flag.  (cgraph_clone_node): initialize externally_visible_by_resolver.
        (cgraph_create_virtual_clone): initialize externally_visible_by_resolver.
        * doc/invoke.texi (-fwhole-program option): Change description of
        externally_visible attribute accordingly.
        * doc/extend.texi (externally_visible): Ditto.



[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 6937 bytes --]

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 161065)
+++ doc/extend.texi	(working copy)
@@ -2296,7 +2296,7 @@
 @cindex @code{externally_visible} attribute.
 This attribute, attached to a global variable or function, nullifies
 the effect of the @option{-fwhole-program} command-line option, so the
-object remains visible outside the current compilation unit.
+object remains visible outside the current compilation unit. If @option{-fwhole-program} is used together with @option{-flto} and @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 
 @item far
 @cindex functions which handle memory bank switching
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 161065)
+++ doc/invoke.texi	(working copy)
@@ -7294,7 +7294,7 @@
 Assume that the current compilation unit represents the whole program being
 compiled.  All public functions and variables with the exception of @code{main}
 and those merged by attribute @code{externally_visible} become static functions
-and in effect are optimized more aggressively by interprocedural optimizers.
+and in effect are optimized more aggressively by interprocedural optimizers. If @command{gold} is used as the linker plugin, @code{externally_visible} attributes are automatically added to functions (not variable yet due to a current @command{gold} issue) that are accessed outside of LTO objects according to resolution file produced by @command{gold}.  For other linkers that cannot generate resolution file, explicit @code{externally_visible} attributes are still necessary.
 While this option is equivalent to proper use of the @code{static} keyword for
 programs consisting of a single file, in combination with option
 @option{-combine}, @option{-flto} or @option{-fwhopr} this flag can be used to
Index: lto-symtab.c
===================================================================
--- lto-symtab.c	(revision 161065)
+++ lto-symtab.c	(working copy)
@@ -530,11 +530,20 @@
     return;
 
 found:
-  if (TREE_CODE (prevailing->decl) == VAR_DECL
-      && TREE_READONLY (prevailing->decl))
+  /* If current lto files represent the whole program,
+    it is correct to use LDPR_PREVALING_DEF_IRONLY.
+    If current lto files are part of whole program, internal
+    resolver doesn't know if it is LDPR_PREVAILING_DEF
+    or LDPR_PREVAILING_DEF_IRONLY.  Use IRONLY conforms to
+    using -fwhole-program.  Otherwise, it doesn't
+    matter using either LDPR_PREVAILING_DEF or
+    LDPR_PREVAILING_DEF_IRONLY
+    
+    FIXME: above workaround due to gold plugin makes some
+    variables IRONLY, which are indeed PREVAILING_DEF in
+    resolution file.  These variables still need manual
+    externally_visible attribute.  */
     prevailing->resolution = LDPR_PREVAILING_DEF_IRONLY;
-  else
-    prevailing->resolution = LDPR_PREVAILING_DEF;
 }
 
 /* Merge all decls in the symbol table chain to the prevailing decl and
@@ -698,6 +707,24 @@
       && TREE_CODE (prevailing->decl) != VAR_DECL)
     prevailing->next = NULL;
 
+  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
+  if (flag_whole_program)
+    {
+      if (prevailing->resolution == LDPR_PREVAILING_DEF)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible_by_resolver = true;
+          else
+            prevailing->vnode->externally_visible_by_resolver = true;
+        }
+      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
+        {
+          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+            prevailing->node->local.externally_visible_by_resolver = false;
+          else
+            prevailing->vnode->externally_visible_by_resolver = false;
+        }
+    }
   return 1;
 }
 
Index: cgraph.c
===================================================================
--- cgraph.c	(revision 161065)
+++ cgraph.c	(working copy)
@@ -1825,6 +1825,8 @@
     fprintf (f, " local");
   if (node->local.externally_visible)
     fprintf (f, " externally_visible");
+  if (node->local.externally_visible_by_resolver)
+    fprintf (f, " externally_visible_by_resolver");
   if (node->local.finalized)
     fprintf (f, " finalized");
   if (node->local.disregard_inline_limits)
@@ -2075,6 +2077,7 @@
   new_node->analyzed = n->analyzed;
   new_node->local = n->local;
   new_node->local.externally_visible = false;
+  new_node->local.externally_visible_by_resolver = false;
   new_node->local.local = true;
   new_node->local.vtable_method = false;
   new_node->global = n->global;
@@ -2266,6 +2269,7 @@
   else
     new_node->clone.combined_args_to_skip = args_to_skip;
   new_node->local.externally_visible = 0;
+  new_node->local.externally_visible_by_resolver = 0;
   new_node->local.local = 1;
   new_node->lowered = true;
   new_node->reachable = true;
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 161065)
+++ cgraph.h	(working copy)
@@ -102,6 +102,9 @@
   /* Set when function is visible by other units.  */
   unsigned externally_visible : 1;
 
+  /* Set when resolver determines that function is visible by other units.  */
+  unsigned externally_visible_by_resolver : 1;
+  
   /* Set once it has been finalized so we consider it to be output.  */
   unsigned finalized : 1;
 
@@ -487,6 +490,8 @@
   unsigned output : 1;
   /* Set when function is visible by other units.  */
   unsigned externally_visible : 1;
+  /* Set when resolver determines that variable is visible by other units.  */
+  unsigned externally_visible_by_resolver : 1;
   /* Set for aliases once they got through assemble_alias.  Also set for
      extra name aliases in varpool_extra_name_alias.  */
   unsigned alias : 1;
Index: ipa.c
===================================================================
--- ipa.c	(revision 161065)
+++ ipa.c	(working copy)
@@ -576,6 +576,8 @@
     return false;
   if (!whole_program)
     return true;
+  if (node->local.externally_visible_by_resolver)
+    return true;
   if (DECL_PRESERVE_P (node->decl))
     return true;
   /* COMDAT functions must be shared only if they have address taken,
@@ -729,6 +731,7 @@
 	         we start reordering datastructures.  */
 	      || DECL_COMDAT (vnode->decl)
 	      || DECL_WEAK (vnode->decl)
+              || vnode->externally_visible_by_resolver
 	      || lookup_attribute ("externally_visible",
 				   DECL_ATTRIBUTES (vnode->decl))))
 	vnode->externally_visible = true;

Re: PING: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Jan Hubicka]

> Honza, 
> 
> Could you comment on the modified patch? As you suggested, I added 
> new "externally_visible_by_resolver" flags only for this purpose. 
> The relevant places in ipa.c are updated to check this flag. It passes tests
> and is bootstrapped. OK for trunk?

The patch is OK.  Since we already have used_from_other_partition for similar purpose,
I guess it would be better to call the flag used_from_object_file.

Honza
> 
> Thanks,
> Bingfeng
> 
> 2010-06-21  Bingfeng Mei <bmei@broadcom.com>
> 
>         * lto-symbtab.c (lto_symtab_merge_decls_1): Set
>         externally_visible_by_resolver flags for symbols of
>         LDPR_PREVAILING_DEF when compiling with -fwhole-program.
>         (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for
> 	internal resolver.
>         * ipa.c (function_and_variable_visibility): Set externally_visible
>         flag of varpool_node if externally_visible_by_resolver flag is set.
>         (cgraph_externally_visible_p): check externally_visible_by_resolver
>         flag.
>         * cgraph.h (struct varpool_node): new externally_visible_by_resolver
>         flag.  (struct cgraph_local_info): new externally_visible_by_resolver
>         flag.
>         * cgraph.c (dump_cgraph_node): dump externally_visible_by_resolver
>         flag.  (cgraph_clone_node): initialize externally_visible_by_resolver.
>         (cgraph_create_virtual_clone): initialize externally_visible_by_resolver.
>         * doc/invoke.texi (-fwhole-program option): Change description of
>         externally_visible attribute accordingly.
>         * doc/extend.texi (externally_visible): Ditto.
> 
> 

Committed : [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.

[Bingfeng Mei]

Changed names of flags and committed  at r161483

2010-06-28  Bingfeng Mei <bmei@broadcom.com>

        * cgraph.h (struct varpool_node): new used_from_object_file flag.
        (struct cgraph_local_info): new used_from_object_file flag.
        * cgraph.c (dump_cgraph_node): dump used_from_object_file flag.
        (cgraph_clone_node): initialize used_from_object_file.
        (cgraph_create_virtual_clone): initialize used_from_object_file.
        * lto-symbtab.c (lto_symtab_merge_decls_1): Set
        used_from_object_file flags for symbols of LDPR_PREVAILING_DEF
        when compiling with -fwhole-program.
        (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY for
	internal resolver.
        * ipa.c (function_and_variable_visibility): Set externally_visible
        flag of varpool_node if used_from_object_file flag is set.
        (cgraph_externally_visible_p): check used_from_object_file flag.
        * doc/invoke.texi (-fwhole-program option): Change description of
        externally_visible attribute accordingly.
        * doc/extend.texi (externally_visible): Ditto.

> -----Original Message-----
> From: Jan Hubicka [mailto:hubicka@ucw.cz]
> Sent: 28 June 2010 10:47
> To: Bingfeng Mei
> Cc: Jan Hubicka; gcc-patches@gcc.gnu.org; Richard Guenther
> Subject: Re: PING: [PATCH, LTO] add externally_visible attribute when
> necessary with -fwhole-program and resolution file.
> 
> > Honza,
> >
> > Could you comment on the modified patch? As you suggested, I added
> > new "externally_visible_by_resolver" flags only for this purpose.
> > The relevant places in ipa.c are updated to check this flag. It
> passes tests
> > and is bootstrapped. OK for trunk?
> 
> The patch is OK.  Since we already have used_from_other_partition for
> similar purpose,
> I guess it would be better to call the flag used_from_object_file.
> 
> Honza
> >
> > Thanks,
> > Bingfeng
> >
> > 2010-06-21  Bingfeng Mei <bmei@broadcom.com>
> >
> >         * lto-symbtab.c (lto_symtab_merge_decls_1): Set
> >         externally_visible_by_resolver flags for symbols of
> >         LDPR_PREVAILING_DEF when compiling with -fwhole-program.
> >         (lto_symtab_resolve_symbols) Use LDPR_PREVAILING_DEF_IRONLY
> for
> > 	internal resolver.
> >         * ipa.c (function_and_variable_visibility): Set
> externally_visible
> >         flag of varpool_node if externally_visible_by_resolver flag
> is set.
> >         (cgraph_externally_visible_p): check
> externally_visible_by_resolver
> >         flag.
> >         * cgraph.h (struct varpool_node): new
> externally_visible_by_resolver
> >         flag.  (struct cgraph_local_info): new
> externally_visible_by_resolver
> >         flag.
> >         * cgraph.c (dump_cgraph_node): dump
> externally_visible_by_resolver
> >         flag.  (cgraph_clone_node): initialize
> externally_visible_by_resolver.
> >         (cgraph_create_virtual_clone): initialize
> externally_visible_by_resolver.
> >         * doc/invoke.texi (-fwhole-program option): Change
> description of
> >         externally_visible attribute accordingly.
> >         * doc/extend.texi (externally_visible): Ditto.
> >
> >
> 
>