This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH, LTO] add externally_visible attribute when necessary with -fwhole-program and resolution file.
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.
> >
> >
> >