Re: varpool alias reorg

[Richard Guenther <rguenther at suse dot de>]

On Thu, 23 Jun 2011, Jan Hubicka wrote:

> > On Sat, Jun 18, 2011 at 7:19 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Sat, Jun 18, 2011 at 1:32 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > >> Hi,
> > >> this patch makes symetric changes to varpool as did the prevoius series to cgraph.
> > >> Basically the aliases are now represented as separate varpool nodes with alias reference
> > >> to the variable they refer to, with some infrastructure to walk the alias references
> > >> as needed.
> > >>
> > >> Bootstrapped/regtested x86_64-linux, comitted.
> > >>
> > >> Honza
> > >>
> > >> ? ? ? ?* lto-symtab.c (lto_varpool_replace_node): Remove code handling
> > >> ? ? ? ?extra name aliases.
> > >> ? ? ? ?(lto_symtab_resolve_can_prevail_p): Likewise.
> > >> ? ? ? ?(lto_symtab_merge_cgraph_nodes): Update alias_of pointers..
> > >> ? ? ? ?* cgraphbuild.c (record_reference): Remove extra body alias code.
> > >> ? ? ? ?(mark_load): Likewise.
> > >> ? ? ? ?(mark_store): Likewise.
> > >> ? ? ? ?* cgraph.h (varpool_node): Remove extra_name filed;
> > >> ? ? ? ?add alias_of and extraname_alias.
> > >> ? ? ? ?(varpool_create_variable_alias, varpool_for_node_and_aliases): Declare.
> > >> ? ? ? ?(varpool_alias_aliased_node): New inline function.
> > >> ? ? ? ?(varpool_variable_node): New function.
> > >> ? ? ? ?* cgraphunit.c (handle_alias_pairs): Handle also variable aliases.
> > >> ? ? ? ?* ipa-ref.c (ipa_record_reference): Allow aliases on variables.
> > >> ? ? ? ?* lto-cgraph.c (lto_output_varpool_node): Update streaming.
> > >> ? ? ? ?(input_varpool_node): Likewise.
> > >> ? ? ? ?* lto-streamer-out.c (produce_symtab): Remove extra name aliases.
> > >> ? ? ? ?(varpool_externally_visible_p): Remove extra body alias code.
> > >> ? ? ? ?(function_and_variable_visibility): Likewise.
> > >> ? ? ? ?* tree-ssa-structalias.c (associate_varinfo_to_alias_1): New function.
> > >> ? ? ? ?(ipa_pta_execute): Use it.
> > >> ? ? ? ?* varpool.c (varpool_remove_node): Remove extra name alias code.
> > >> ? ? ? ?(varpool_mark_needed_node): Likewise.
> > >> ? ? ? ?(varpool_analyze_pending_decls): Analyze aliases.
> > >> ? ? ? ?(assemble_aliases): New functoin.
> > >> ? ? ? ?(varpool_assemble_decl): Use it.
> > >> ? ? ? ?(varpool_create_variable_alias): New function.
> > >> ? ? ? ?(varpool_extra_name_alias): Rewrite.
> > >> ? ? ? ?(varpool_for_node_and_aliases): New function.
> > >
> > > This caused:
> > >
> > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49463
> > >
> > 
> > This patch is incorrect as shown in the PR above.
> 
> The builtins/strstr-asm.c is the same issue as I patched some time ago in 
> http://gcc.gnu.org/ml/gcc-patches/2010-12/msg00031.html
> just in this case the problem remained hidden until now.
> 
> Again the problem needs plugin to manifest.
> 
> There are two problems here
>   1) We do not stream builtin decls and merge them outside lto-symtab (by just
>      streaming references to builtins with their asm names).  There is at least
>      one extra PR related to this and on my TODO is to simply remove the code.
>   2) Aliases within single unit works only when both the alias and the target use
>      asm name.  This is because internally we store mangled DECL_ASSEMBLER_NAME and the
>      alias_pair code.
>      With LTO this breaks existing code simply because what used to be multiple units
>      and thus safe is now single LTO unit.
> 
>      Dave Korn fixed part of the problem by introducing mangling code into lto-symtab
>      His code solve similar problems with aliases from the asm code, but it
>      did not solve the problem with aliases from LTO units, like here, simply
>      because alias pair code bypass the lto-symtab.  One of goals of the incorrect patch
>      above is to make lto-symtab to do the merging and thus fix this issue (for now at
>      all decls except for builtins).
> 
>      Still it would be good to solve the problem on non-LTO compilation, too.
>      We discussed introduction of proper symbol table into GCC at the GCC gathering last
>      weekend.  It is where I am heading but it will take some time.
> 
> Until that happens, I suggest fixing the testcase same was as we already fixed
> the memops-asm-lib.c in 4.6 timeframe.
> 
> Bootstrapped/regtested x86_64-linux, OK?

Ok.

Thanks,
Richard.

> Index: strstr-asm-lib.c
> ===================================================================
> --- strstr-asm-lib.c	(revision 175183)
> +++ strstr-asm-lib.c	(working copy)
> @@ -7,6 +7,7 @@
>  extern int inside_main;
>  extern const char *p;
>  
> +__attribute__ ((used))
>  char *
>  my_strstr (const char *s1, const char *s2)
>  {