diff mbox

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

Message ID 7FB04A5C213E9943A72EE127DB74F0ADA66675B3BB@SJEXCHCCR02.corp.ad.broadcom.com
State New
Headers show

Commit Message

Bingfeng Mei June 9, 2010, 2:46 p.m. UTC 
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

Comments

Richard Biener June 9, 2010, 3:01 p.m. UTC | #1 
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.
Bingfeng Mei June 9, 2010, 3:14 p.m. UTC | #2 
> -----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.
Bingfeng Mei June 9, 2010, 3:20 p.m. UTC | #3 
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.
Richard Biener June 9, 2010, 3:25 p.m. UTC | #4 
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.
>
>
>
Richard Biener June 9, 2010, 3:26 p.m. UTC | #5 
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.
>
>
>
Bingfeng Mei June 9, 2010, 3:32 p.m. UTC | #6 
> -----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.
> >
> >
> >
diff mbox

Patch

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.  */