[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.
>
>
>