varpool alias reorg

varpool alias reorg

[Jan Hubicka]

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.
Index: lto-symtab.c
===================================================================
*** lto-symtab.c	(revision 175079)
--- lto-symtab.c	(working copy)
*************** lto_varpool_replace_node (struct varpool
*** 268,299 ****
        gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
        varpool_mark_needed_node (prevailing_node);
      }
-   /* Relink aliases.  */
-   if (vnode->extra_name && !vnode->alias)
-     {
-       struct varpool_node *alias, *last;
-       for (alias = vnode->extra_name;
- 	   alias; alias = alias->next)
- 	{
- 	  last = alias;
- 	  alias->extra_name = prevailing_node;
- 	}
- 
-       if (prevailing_node->extra_name)
- 	{
- 	  last->next = prevailing_node->extra_name;
- 	  prevailing_node->extra_name->prev = last;
- 	}
-       prevailing_node->extra_name = vnode->extra_name;
-       vnode->extra_name = NULL;
-     }
    gcc_assert (!vnode->finalized || prevailing_node->finalized);
    gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
  
-   /* When replacing by an alias, the references goes to the original
-      variable.  */
-   if (prevailing_node->alias && prevailing_node->extra_name)
-     prevailing_node = prevailing_node->extra_name;
    ipa_clone_refering (NULL, prevailing_node, &vnode->ref_list);
  
    /* Be sure we can garbage collect the initializer.  */
--- 268,276 ----
*************** lto_symtab_resolve_can_prevail_p (lto_sy
*** 445,451 ****
  	return false;
        if (e->vnode->finalized)
  	return true;
-       return e->vnode->alias && e->vnode->extra_name->finalized;
      }
  
    gcc_unreachable ();
*************** void
*** 779,784 ****
--- 755,761 ----
  lto_symtab_merge_cgraph_nodes (void)
  {
    struct cgraph_node *node;
+   struct varpool_node *vnode;
    lto_symtab_maybe_init_hash_table ();
    htab_traverse (lto_symtab_identifiers, lto_symtab_merge_cgraph_nodes_1, NULL);
  
*************** lto_symtab_merge_cgraph_nodes (void)
*** 786,791 ****
--- 763,771 ----
      if ((node->thunk.thunk_p || node->alias)
  	&& node->thunk.alias)
        node->thunk.alias = lto_symtab_prevailing_decl (node->thunk.alias);
+   for (vnode = varpool_nodes; vnode; vnode = vnode->next)
+     if (vnode->alias_of)
+       vnode->alias_of = lto_symtab_prevailing_decl (vnode->alias_of);
  }
  
  /* Given the decl DECL, return the prevailing decl with the same name. */
Index: cgraphbuild.c
===================================================================
*** cgraphbuild.c	(revision 175079)
--- cgraphbuild.c	(working copy)
*************** record_reference (tree *tp, int *walk_su
*** 87,94 ****
  	  if (lang_hooks.callgraph.analyze_expr)
  	    lang_hooks.callgraph.analyze_expr (&decl, walk_subtrees);
  	  varpool_mark_needed_node (vnode);
- 	  if (vnode->alias && vnode->extra_name)
- 	    vnode = vnode->extra_name;
  	  ipa_record_reference (NULL, ctx->varpool_node,
  				NULL, vnode,
  				IPA_REF_ADDR, NULL);
--- 87,92 ----
*************** mark_address (gimple stmt, tree addr, vo
*** 261,268 ****
        if (lang_hooks.callgraph.analyze_expr)
  	lang_hooks.callgraph.analyze_expr (&addr, &walk_subtrees);
        varpool_mark_needed_node (vnode);
-       if (vnode->alias && vnode->extra_name)
- 	vnode = vnode->extra_name;
        ipa_record_reference ((struct cgraph_node *)data, NULL,
  			    NULL, vnode,
  			    IPA_REF_ADDR, stmt);
--- 259,264 ----
*************** mark_load (gimple stmt, tree t, void *da
*** 296,303 ****
        if (lang_hooks.callgraph.analyze_expr)
  	lang_hooks.callgraph.analyze_expr (&t, &walk_subtrees);
        varpool_mark_needed_node (vnode);
-       if (vnode->alias && vnode->extra_name)
- 	vnode = vnode->extra_name;
        ipa_record_reference ((struct cgraph_node *)data, NULL,
  			    NULL, vnode,
  			    IPA_REF_LOAD, stmt);
--- 292,297 ----
*************** mark_store (gimple stmt, tree t, void *d
*** 320,327 ****
        if (lang_hooks.callgraph.analyze_expr)
  	lang_hooks.callgraph.analyze_expr (&t, &walk_subtrees);
        varpool_mark_needed_node (vnode);
-       if (vnode->alias && vnode->extra_name)
- 	vnode = vnode->extra_name;
        ipa_record_reference ((struct cgraph_node *)data, NULL,
  			    NULL, vnode,
  			    IPA_REF_STORE, stmt);
--- 314,319 ----
Index: cgraph.h
===================================================================
*** cgraph.h	(revision 175079)
--- cgraph.h	(working copy)
*************** DEF_VEC_ALLOC_P(cgraph_edge_p,heap);
*** 380,392 ****
  
  struct GTY((chain_next ("%h.next"), chain_prev ("%h.prev"))) varpool_node {
    tree decl;
    /* Pointer to the next function in varpool_nodes.  */
    struct varpool_node *next, *prev;
    /* Pointer to the next function in varpool_nodes_queue.  */
    struct varpool_node *next_needed, *prev_needed;
-   /* For normal nodes a pointer to the first extra name alias.  For alias
-      nodes a pointer to the normal node.  */
-   struct varpool_node *extra_name;
    /* Circular list of nodes in the same comdat group if non-NULL.  */
    struct varpool_node *same_comdat_group;
    struct ipa_ref_list ref_list;
--- 380,391 ----
  
  struct GTY((chain_next ("%h.next"), chain_prev ("%h.prev"))) varpool_node {
    tree decl;
+   /* For aliases points to declaration DECL is alias of.  */
+   tree alias_of;
    /* Pointer to the next function in varpool_nodes.  */
    struct varpool_node *next, *prev;
    /* Pointer to the next function in varpool_nodes_queue.  */
    struct varpool_node *next_needed, *prev_needed;
    /* Circular list of nodes in the same comdat group if non-NULL.  */
    struct varpool_node *same_comdat_group;
    struct ipa_ref_list ref_list;
*************** struct GTY((chain_next ("%h.next"), chai
*** 415,420 ****
--- 414,420 ----
    /* Set for aliases once they got through assemble_alias.  Also set for
       extra name aliases in varpool_extra_name_alias.  */
    unsigned alias : 1;
+   unsigned extra_name_alias : 1;
    /* Set when variable is used from other LTRANS partition.  */
    unsigned used_from_other_partition : 1;
    /* Set when variable is available in the other LTRANS partition.
*************** bool varpool_analyze_pending_decls (void
*** 665,673 ****
--- 665,677 ----
  void varpool_remove_unreferenced_decls (void);
  void varpool_empty_needed_queue (void);
  struct varpool_node * varpool_extra_name_alias (tree, tree);
+ struct varpool_node * varpool_create_variable_alias (tree, tree);
  const char * varpool_node_name (struct varpool_node *node);
  void varpool_reset_queue (void);
  bool const_value_known_p (tree);
+ bool varpool_for_node_and_aliases (struct varpool_node *,
+ 		                   bool (*) (struct varpool_node *, void *),
+ 			           void *, bool);
  
  /* Walk all reachable static variables.  */
  #define FOR_EACH_STATIC_VARIABLE(node) \
*************** cgraph_alias_aliased_node (struct cgraph
*** 968,973 ****
--- 972,991 ----
    return NULL;
  }
  
+ /* Return node that alias N is aliasing.  */
+ 
+ static inline struct varpool_node *
+ varpool_alias_aliased_node (struct varpool_node *n)
+ {
+   struct ipa_ref *ref;
+ 
+   ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
+   gcc_checking_assert (ref->use == IPA_REF_ALIAS);
+   if (ref->refered_type == IPA_REF_CGRAPH)
+     return ipa_ref_varpool_node (ref);
+   return NULL;
+ }
+ 
  /* Given NODE, walk the alias chain to return the function NODE is alias of.
     Walk through thunk, too.
     When AVAILABILITY is non-NULL, get minimal availablity in the chain.  */
*************** cgraph_function_or_thunk_node (struct cg
*** 1020,1025 ****
--- 1038,1071 ----
  	  if (a < *availability)
  	    *availability = a;
  	}
+     }
+   if (*availability)
+     *availability = AVAIL_NOT_AVAILABLE;
+   return NULL;
+ }
+ 
+ /* Given NODE, walk the alias chain to return the function NODE is alias of.
+    Do not walk through thunks.
+    When AVAILABILITY is non-NULL, get minimal availablity in the chain.  */
+ 
+ static inline struct varpool_node *
+ varpool_variable_node (struct varpool_node *node, enum availability *availability)
+ {
+   if (availability)
+     *availability = cgraph_variable_initializer_availability (node);
+   while (node)
+     {
+       if (node->alias && node->analyzed)
+ 	node = varpool_alias_aliased_node (node);
+       else
+ 	return node;
+       if (node && availability)
+ 	{
+ 	  enum availability a;
+ 	  a = cgraph_variable_initializer_availability (node);
+ 	  if (a < *availability)
+ 	    *availability = a;
+ 	}
      }
    if (*availability)
      *availability = AVAIL_NOT_AVAILABLE;
Index: cgraphunit.c
===================================================================
*** cgraphunit.c	(revision 175079)
--- cgraphunit.c	(working copy)
*************** handle_alias_pairs (void)
*** 1186,1191 ****
--- 1186,1192 ----
    unsigned i;
    struct cgraph_node *target_node;
    struct cgraph_node *src_node;
+   struct varpool_node *target_vnode;
    
    for (i = 0; VEC_iterate (alias_pair, alias_pairs, i, p);)
      {
*************** handle_alias_pairs (void)
*** 1206,1211 ****
--- 1207,1226 ----
  	  cgraph_create_function_alias (p->decl, target_node->decl);
  	  VEC_unordered_remove (alias_pair, alias_pairs, i);
  	}
+       else if (TREE_CODE (p->decl) == VAR_DECL
+ 	       && !lookup_attribute ("weakref", DECL_ATTRIBUTES (p->decl))
+ 	       && (target_vnode = varpool_node_for_asm (p->target)) != NULL)
+ 	{
+ 	  /* Normally EXTERNAL flag is used to mark external inlines,
+ 	     however for aliases it seems to be allowed to use it w/o
+ 	     any meaning. See gcc.dg/attr-alias-3.c  
+ 	     However for weakref we insist on EXTERNAL flag being set.
+ 	     See gcc.dg/attr-alias-5.c  */
+ 	  if (DECL_EXTERNAL (p->decl))
+ 	    DECL_EXTERNAL (p->decl) = 0;
+ 	  varpool_create_variable_alias (p->decl, target_vnode->decl);
+ 	  VEC_unordered_remove (alias_pair, alias_pairs, i);
+ 	}
        else
  	{
  	  if (dump_file)
Index: ipa-ref.c
===================================================================
*** ipa-ref.c	(revision 175079)
--- ipa-ref.c	(working copy)
*************** ipa_record_reference (struct cgraph_node
*** 68,74 ****
      {
        ref->refering.varpool_node = refering_varpool_node;
        ref->refering_type = IPA_REF_VARPOOL;
!       gcc_assert (use_type == IPA_REF_ADDR);
      }
    if (refered_node)
      {
--- 68,74 ----
      {
        ref->refering.varpool_node = refering_varpool_node;
        ref->refering_type = IPA_REF_VARPOOL;
!       gcc_assert (use_type == IPA_REF_ADDR || use_type == IPA_REF_ALIAS);
      }
    if (refered_node)
      {
Index: lto-cgraph.c
===================================================================
*** lto-cgraph.c	(revision 175079)
--- lto-cgraph.c	(working copy)
*************** lto_output_varpool_node (struct lto_simp
*** 544,551 ****
  {
    bool boundary_p = !varpool_node_in_set_p (node, vset) && node->analyzed;
    struct bitpack_d bp;
-   struct varpool_node *alias;
-   int count = 0;
    int ref;
  
    lto_output_var_decl_index (ob->decl_state, ob->main_stream, node->decl);
--- 544,549 ----
*************** lto_output_varpool_node (struct lto_simp
*** 554,560 ****
    bp_pack_value (&bp, node->force_output, 1);
    bp_pack_value (&bp, node->finalized, 1);
    bp_pack_value (&bp, node->alias, 1);
!   gcc_assert (!node->alias || !node->extra_name);
    gcc_assert (node->finalized || !node->analyzed);
    gcc_assert (node->needed);
    /* Constant pool initializers can be de-unified into individual ltrans units.
--- 552,558 ----
    bp_pack_value (&bp, node->force_output, 1);
    bp_pack_value (&bp, node->finalized, 1);
    bp_pack_value (&bp, node->alias, 1);
!   bp_pack_value (&bp, node->alias_of != NULL, 1);
    gcc_assert (node->finalized || !node->analyzed);
    gcc_assert (node->needed);
    /* Constant pool initializers can be de-unified into individual ltrans units.
*************** lto_output_varpool_node (struct lto_simp
*** 573,583 ****
  							   set, vset), 1);
        bp_pack_value (&bp, boundary_p, 1);  /* in_other_partition.  */
      }
-   /* Also emit any extra name aliases.  */
-   for (alias = node->extra_name; alias; alias = alias->next)
-     count++;
-   bp_pack_value (&bp, count != 0, 1);
    lto_output_bitpack (&bp);
    if (node->same_comdat_group && !boundary_p)
      {
        ref = lto_varpool_encoder_lookup (varpool_encoder, node->same_comdat_group);
--- 571,579 ----
  							   set, vset), 1);
        bp_pack_value (&bp, boundary_p, 1);  /* in_other_partition.  */
      }
    lto_output_bitpack (&bp);
+   if (node->alias_of)
+     lto_output_var_decl_index (ob->decl_state, ob->main_stream, node->alias_of);
    if (node->same_comdat_group && !boundary_p)
      {
        ref = lto_varpool_encoder_lookup (varpool_encoder, node->same_comdat_group);
*************** lto_output_varpool_node (struct lto_simp
*** 588,604 ****
    lto_output_sleb128_stream (ob->main_stream, ref);
    lto_output_enum (ob->main_stream, ld_plugin_symbol_resolution,
  		   LDPR_NUM_KNOWN, node->resolution);
- 
-   if (count)
-     {
-       lto_output_uleb128_stream (ob->main_stream, count);
-       for (alias = node->extra_name; alias; alias = alias->next)
- 	{
- 	  lto_output_var_decl_index (ob->decl_state, ob->main_stream, alias->decl);
- 	  lto_output_enum (ob->main_stream, ld_plugin_symbol_resolution,
- 			   LDPR_NUM_KNOWN, alias->resolution);
- 	}
-     }
  }
  
  /* Output the varpool NODE to OB. 
--- 584,589 ----
*************** input_varpool_node (struct lto_file_decl
*** 1054,1062 ****
    tree var_decl;
    struct varpool_node *node;
    struct bitpack_d bp;
-   bool aliases_p;
-   int count;
    int ref = LCC_NOT_FOUND;
  
    decl_index = lto_input_uleb128 (ib);
    var_decl = lto_file_decl_data_get_var_decl (file_data, decl_index);
--- 1039,1046 ----
    tree var_decl;
    struct varpool_node *node;
    struct bitpack_d bp;
    int ref = LCC_NOT_FOUND;
+   bool non_null_aliasof;
  
    decl_index = lto_input_uleb128 (ib);
    var_decl = lto_file_decl_data_get_var_decl (file_data, decl_index);
*************** input_varpool_node (struct lto_file_decl
*** 1068,1073 ****
--- 1052,1058 ----
    node->force_output = bp_unpack_value (&bp, 1);
    node->finalized = bp_unpack_value (&bp, 1);
    node->alias = bp_unpack_value (&bp, 1);
+   non_null_aliasof = bp_unpack_value (&bp, 1);
    node->analyzed = node->finalized; 
    node->used_from_other_partition = bp_unpack_value (&bp, 1);
    node->in_other_partition = bp_unpack_value (&bp, 1);
*************** input_varpool_node (struct lto_file_decl
*** 1076,1082 ****
        DECL_EXTERNAL (node->decl) = 1;
        TREE_STATIC (node->decl) = 0;
      }
-   aliases_p = bp_unpack_value (&bp, 1);
    if (node->finalized)
      varpool_mark_needed_node (node);
    ref = lto_input_sleb128 (ib);
--- 1061,1066 ----
*************** input_varpool_node (struct lto_file_decl
*** 1084,1101 ****
    node->same_comdat_group = (struct varpool_node *) (intptr_t) ref;
    node->resolution = lto_input_enum (ib, ld_plugin_symbol_resolution,
  				     LDPR_NUM_KNOWN);
!   if (aliases_p)
      {
!       count = lto_input_uleb128 (ib);
!       for (; count > 0; count --)
! 	{
! 	  tree decl = lto_file_decl_data_get_var_decl (file_data,
! 						       lto_input_uleb128 (ib));
! 	  struct varpool_node *alias;
! 	  alias = varpool_extra_name_alias (decl, var_decl);
! 	  alias->resolution = lto_input_enum (ib, ld_plugin_symbol_resolution,
! 					      LDPR_NUM_KNOWN);
! 	}
      }
    return node;
  }
--- 1068,1078 ----
    node->same_comdat_group = (struct varpool_node *) (intptr_t) ref;
    node->resolution = lto_input_enum (ib, ld_plugin_symbol_resolution,
  				     LDPR_NUM_KNOWN);
! 
!   if (non_null_aliasof)
      {
!       decl_index = lto_input_uleb128 (ib);
!       node->alias_of = lto_file_decl_data_get_var_decl (file_data, decl_index);
      }
    return node;
  }
Index: lto-streamer-out.c
===================================================================
*** lto-streamer-out.c	(revision 175079)
--- lto-streamer-out.c	(working copy)
*************** produce_symtab (struct output_block *ob,
*** 2557,2563 ****
    char *section_name = lto_get_section_name (LTO_section_symtab, NULL, NULL);
    struct pointer_set_t *seen;
    struct cgraph_node *node;
!   struct varpool_node *vnode, *valias;
    struct lto_output_stream stream;
    lto_varpool_encoder_t varpool_encoder = ob->decl_state->varpool_node_encoder;
    lto_cgraph_encoder_t encoder = ob->decl_state->cgraph_node_encoder;
--- 2557,2563 ----
    char *section_name = lto_get_section_name (LTO_section_symtab, NULL, NULL);
    struct pointer_set_t *seen;
    struct cgraph_node *node;
!   struct varpool_node *vnode;
    struct lto_output_stream stream;
    lto_varpool_encoder_t varpool_encoder = ob->decl_state->varpool_node_encoder;
    lto_cgraph_encoder_t encoder = ob->decl_state->cgraph_node_encoder;
*************** produce_symtab (struct output_block *ob,
*** 2617,2627 ****
  	  && vnode->finalized 
  	  && DECL_VIRTUAL_P (vnode->decl))
  	continue;
!       if (vnode->alias)
  	continue;
        write_symbol (cache, &stream, vnode->decl, seen, false);
-       for (valias = vnode->extra_name; valias; valias = valias->next)
-         write_symbol (cache, &stream, valias->decl, seen, true);
      }
    for (i = 0; i < lto_varpool_encoder_size (varpool_encoder); i++)
      {
--- 2617,2625 ----
  	  && vnode->finalized 
  	  && DECL_VIRTUAL_P (vnode->decl))
  	continue;
!       if (vnode->alias && !vnode->alias_of)
  	continue;
        write_symbol (cache, &stream, vnode->decl, seen, false);
      }
    for (i = 0; i < lto_varpool_encoder_size (varpool_encoder); i++)
      {
*************** produce_symtab (struct output_block *ob,
*** 2633,2643 ****
  	  && vnode->finalized 
  	  && DECL_VIRTUAL_P (vnode->decl))
  	continue;
!       if (vnode->alias)
  	continue;
        write_symbol (cache, &stream, vnode->decl, seen, false);
-       for (valias = vnode->extra_name; valias; valias = valias->next)
-         write_symbol (cache, &stream, valias->decl, seen, true);
      }
  
    /* Write all aliases.  */
--- 2631,2639 ----
  	  && vnode->finalized 
  	  && DECL_VIRTUAL_P (vnode->decl))
  	continue;
!       if (vnode->alias && !vnode->alias_of)
  	continue;
        write_symbol (cache, &stream, vnode->decl, seen, false);
      }
  
    /* Write all aliases.  */
Index: ipa.c
===================================================================
*** ipa.c	(revision 175079)
--- ipa.c	(working copy)
*************** cgraph_externally_visible_p (struct cgra
*** 655,661 ****
  static bool
  varpool_externally_visible_p (struct varpool_node *vnode, bool aliased)
  {
-   struct varpool_node *alias;
    if (!DECL_COMDAT (vnode->decl) && !TREE_PUBLIC (vnode->decl))
      return false;
  
--- 648,653 ----
*************** varpool_externally_visible_p (struct var
*** 694,704 ****
       This is needed for i.e. references from asm statements.   */
    if (varpool_used_from_object_file_p (vnode))
      return true;
-   for (alias = vnode->extra_name; alias; alias = alias->next)
-     if (alias->resolution != LDPR_PREVAILING_DEF_IRONLY)
-       break;
-   if (!alias && vnode->resolution == LDPR_PREVAILING_DEF_IRONLY)
-     return false;
  
    /* As a special case, the COMDAT virutal tables can be unshared.
       In LTO mode turn vtables into static variables.  The variable is readonly,
--- 686,691 ----
*************** function_and_variable_visibility (bool w
*** 782,794 ****
          {
  	  if (!node->analyzed)
  	    continue;
! 	  /* Weakrefs alias symbols from other compilation unit.  In the case
! 	     the destination of weakref became available because of LTO, we must
! 	     mark it as needed.  */
! 	  if (in_lto_p
! 	      && lookup_attribute ("weakref", DECL_ATTRIBUTES (p->decl))
! 	      && !node->needed)
! 	    cgraph_mark_needed_node (node);
  	  gcc_assert (node->needed);
  	  pointer_set_insert (aliased_nodes, node);
  	  if (dump_file)
--- 769,775 ----
          {
  	  if (!node->analyzed)
  	    continue;
! 	  cgraph_mark_needed_node (node);
  	  gcc_assert (node->needed);
  	  pointer_set_insert (aliased_nodes, node);
  	  if (dump_file)
*************** function_and_variable_visibility (bool w
*** 798,810 ****
        else if ((vnode = varpool_node_for_asm (p->target)) != NULL
  	       && !DECL_EXTERNAL (vnode->decl))
          {
! 	  /* Weakrefs alias symbols from other compilation unit.  In the case
! 	     the destination of weakref became available because of LTO, we must
! 	     mark it as needed.  */
! 	  if (in_lto_p
! 	      && lookup_attribute ("weakref", DECL_ATTRIBUTES (p->decl))
! 	      && !vnode->needed)
! 	    varpool_mark_needed_node (vnode);
  	  gcc_assert (vnode->needed);
  	  pointer_set_insert (aliased_vnodes, vnode);
  	  if (dump_file)
--- 779,785 ----
        else if ((vnode = varpool_node_for_asm (p->target)) != NULL
  	       && !DECL_EXTERNAL (vnode->decl))
          {
! 	  varpool_mark_needed_node (vnode);
  	  gcc_assert (vnode->needed);
  	  pointer_set_insert (aliased_vnodes, vnode);
  	  if (dump_file)
Index: tree-ssa-structalias.c
===================================================================
*** tree-ssa-structalias.c	(revision 175079)
--- tree-ssa-structalias.c	(working copy)
*************** associate_varinfo_to_alias (struct cgrap
*** 6685,6690 ****
--- 6685,6700 ----
    return false;
  }
  
+ /* Associate node with varinfo DATA. Worker for
+    varpool_for_node_and_aliases.  */
+ static bool
+ associate_varinfo_to_alias_1 (struct varpool_node *node, void *data)
+ {
+   if (node->alias)
+     insert_vi_for_tree (node->decl, (varinfo_t)data);
+   return false;
+ }
+ 
  /* Execute the driver for IPA PTA.  */
  static unsigned int
  ipa_pta_execute (void)
*************** ipa_pta_execute (void)
*** 6716,6729 ****
    /* Create constraints for global variables and their initializers.  */
    for (var = varpool_nodes; var; var = var->next)
      {
-       struct varpool_node *alias;
        varinfo_t vi;
  
        vi = get_vi_for_tree (var->decl);
! 
!       /* Associate the varinfo node with all aliases.  */
!       for (alias = var->extra_name; alias; alias = alias->next)
! 	insert_vi_for_tree (alias->decl, vi);
      }
  
    if (dump_file)
--- 6726,6737 ----
    /* Create constraints for global variables and their initializers.  */
    for (var = varpool_nodes; var; var = var->next)
      {
        varinfo_t vi;
+       if (var->alias)
+ 	continue;
  
        vi = get_vi_for_tree (var->decl);
!       varpool_for_node_and_aliases (var, associate_varinfo_to_alias_1, vi, true);
      }
  
    if (dump_file)
Index: varpool.c
===================================================================
*** varpool.c	(revision 175079)
--- varpool.c	(working copy)
*************** varpool_remove_node (struct varpool_node
*** 162,186 ****
    gcc_assert (*slot == node);
    htab_clear_slot (varpool_hash, slot);
    gcc_assert (!varpool_assembled_nodes_queue);
-   if (!node->alias)
-     while (node->extra_name)
-       varpool_remove_node (node->extra_name);
    if (node->next)
      node->next->prev = node->prev;
    if (node->prev)
      node->prev->next = node->next;
    else
      {
!       if (node->alias && node->extra_name)
! 	{
!           gcc_assert (node->extra_name->extra_name == node);
! 	  node->extra_name->extra_name = node->next;
! 	}
!       else
! 	{
!           gcc_assert (varpool_nodes == node);
!           varpool_nodes = node->next;
! 	}
      }
    if (varpool_first_unanalyzed_node == node)
      varpool_first_unanalyzed_node = node->next_needed;
--- 162,175 ----
    gcc_assert (*slot == node);
    htab_clear_slot (varpool_hash, slot);
    gcc_assert (!varpool_assembled_nodes_queue);
    if (node->next)
      node->next->prev = node->prev;
    if (node->prev)
      node->prev->next = node->next;
    else
      {
!       gcc_assert (varpool_nodes == node);
!       varpool_nodes = node->next;
      }
    if (varpool_first_unanalyzed_node == node)
      varpool_first_unanalyzed_node = node->next_needed;
*************** varpool_enqueue_needed_node (struct varp
*** 311,318 ****
  void
  varpool_mark_needed_node (struct varpool_node *node)
  {
-   if (node->alias && node->extra_name)
-     node = node->extra_name;
    if (!node->needed && node->finalized
        && !TREE_ASM_WRITTEN (node->decl))
      varpool_enqueue_needed_node (node);
--- 300,305 ----
*************** varpool_analyze_pending_decls (void)
*** 473,479 ****
  	     already informed about increased alignment.  */
            align_variable (decl, 0);
  	}
!       if (DECL_INITIAL (decl))
  	record_references_in_initializer (decl, analyzed);
        if (node->same_comdat_group)
  	{
--- 460,499 ----
  	     already informed about increased alignment.  */
            align_variable (decl, 0);
  	}
!       if (node->alias && node->alias_of)
! 	{
! 	  struct varpool_node *tgt = varpool_node (node->alias_of);
! 	  if (!VEC_length (ipa_ref_t, node->ref_list.references))
! 	    ipa_record_reference (NULL, node, NULL, tgt, IPA_REF_ALIAS, NULL);
! 	  /* C++ FE sometimes change linkage flags after producing same body aliases.  */
! 	  if (node->extra_name_alias)
! 	    {
! 	      DECL_WEAK (node->decl) = DECL_WEAK (node->alias_of);
! 	      TREE_PUBLIC (node->decl) = TREE_PUBLIC (node->alias_of);
! 	      DECL_VISIBILITY (node->decl) = DECL_VISIBILITY (node->alias_of);
! 	      if (TREE_PUBLIC (node->decl))
! 		{
! 		  DECL_COMDAT (node->decl) = DECL_COMDAT (node->alias_of);
! 		  DECL_COMDAT_GROUP (node->decl) = DECL_COMDAT_GROUP (node->alias_of);
! 		  if (DECL_ONE_ONLY (node->alias_of) && !node->same_comdat_group)
! 		    {
! 		      node->same_comdat_group = tgt;
! 		      if (!tgt->same_comdat_group)
! 			tgt->same_comdat_group = node;
! 		      else
! 			{
! 			  struct varpool_node *n;
! 			  for (n = tgt->same_comdat_group;
! 			       n->same_comdat_group != tgt;
! 			       n = n->same_comdat_group)
! 			    ;
! 			  n->same_comdat_group = node;
! 			}
! 		    }
! 		}
! 	    }
! 	}
!       else if (DECL_INITIAL (decl))
  	record_references_in_initializer (decl, analyzed);
        if (node->same_comdat_group)
  	{
*************** varpool_analyze_pending_decls (void)
*** 488,493 ****
--- 508,530 ----
    return changed;
  }
  
+ /* Assemble thunks and aliases asociated to NODE.  */
+ 
+ static void
+ assemble_aliases (struct varpool_node *node)
+ {
+   int i;
+   struct ipa_ref *ref;
+   for (i = 0; ipa_ref_list_refering_iterate (&node->ref_list, i, ref); i++)
+     if (ref->use == IPA_REF_ALIAS)
+       {
+ 	struct varpool_node *alias = ipa_ref_refering_varpool_node (ref);
+ 	assemble_alias (alias->decl,
+ 			DECL_ASSEMBLER_NAME (alias->alias_of));
+ 	assemble_aliases (alias);
+       }
+ }
+ 
  /* Output one variable, if necessary.  Return whether we output it.  */
  bool
  varpool_assemble_decl (struct varpool_node *node)
*************** varpool_assemble_decl (struct varpool_no
*** 503,527 ****
        assemble_variable (decl, 0, 1, 0);
        if (TREE_ASM_WRITTEN (decl))
  	{
- 	  struct varpool_node *alias;
- 
  	  node->next_needed = varpool_assembled_nodes_queue;
  	  node->prev_needed = NULL;
  	  if (varpool_assembled_nodes_queue)
  	    varpool_assembled_nodes_queue->prev_needed = node;
  	  varpool_assembled_nodes_queue = node;
  	  node->finalized = 1;
! 
! 	  /* Also emit any extra name aliases.  */
! 	  for (alias = node->extra_name; alias; alias = alias->next)
! 	    {
! 	      /* Update linkage fields in case they've changed.  */
! 	      DECL_WEAK (alias->decl) = DECL_WEAK (decl);
! 	      TREE_PUBLIC (alias->decl) = TREE_PUBLIC (decl);
! 	      DECL_VISIBILITY (alias->decl) = DECL_VISIBILITY (decl);
! 	      assemble_alias (alias->decl, DECL_ASSEMBLER_NAME (decl));
! 	    }
! 
  	  return true;
  	}
      }
--- 540,552 ----
        assemble_variable (decl, 0, 1, 0);
        if (TREE_ASM_WRITTEN (decl))
  	{
  	  node->next_needed = varpool_assembled_nodes_queue;
  	  node->prev_needed = NULL;
  	  if (varpool_assembled_nodes_queue)
  	    varpool_assembled_nodes_queue->prev_needed = node;
  	  varpool_assembled_nodes_queue = node;
  	  node->finalized = 1;
! 	  assemble_aliases (node);
  	  return true;
  	}
      }
*************** add_new_static_var (tree type)
*** 670,707 ****
     Extra name aliases are output whenever DECL is output.  */
  
  struct varpool_node *
  varpool_extra_name_alias (tree alias, tree decl)
  {
!   struct varpool_node key, *alias_node, *decl_node, **slot;
  
  #ifndef ASM_OUTPUT_DEF
    /* If aliases aren't supported by the assembler, fail.  */
    return NULL;
  #endif
! 
    gcc_assert (TREE_CODE (decl) == VAR_DECL);
    gcc_assert (TREE_CODE (alias) == VAR_DECL);
!   /* Make sure the hash table has been created.  */
!   decl_node = varpool_node (decl);
! 
!   key.decl = alias;
! 
!   slot = (struct varpool_node **) htab_find_slot (varpool_hash, &key, INSERT);
! 
!   /* If the varpool_node has been already created, fail.  */
!   if (*slot)
!     return NULL;
! 
!   alias_node = ggc_alloc_cleared_varpool_node ();
!   alias_node->decl = alias;
    alias_node->alias = 1;
!   alias_node->extra_name = decl_node;
!   alias_node->next = decl_node->extra_name;
!   ipa_empty_ref_list (&alias_node->ref_list);
!   if (decl_node->extra_name)
!     decl_node->extra_name->prev = alias_node;
!   decl_node->extra_name = alias_node;
!   *slot = alias_node;
    return alias_node;
  }
  
--- 695,739 ----
     Extra name aliases are output whenever DECL is output.  */
  
  struct varpool_node *
+ varpool_create_variable_alias (tree alias, tree decl)
+ {
+   struct varpool_node *alias_node;
+ 
+   gcc_assert (TREE_CODE (decl) == VAR_DECL);
+   gcc_assert (TREE_CODE (alias) == VAR_DECL);
+   alias_node = varpool_node (alias);
+   alias_node->alias = 1;
+   alias_node->finalized = 1;
+   alias_node->alias_of = decl;
+   if (decide_is_variable_needed (alias_node, alias)
+       || alias_node->needed)
+     varpool_mark_needed_node (alias_node);
+   return alias_node;
+ }
+ 
+ /* Attempt to mark ALIAS as an alias to DECL.  Return TRUE if successful.
+    Extra name aliases are output whenever DECL is output.  */
+ 
+ struct varpool_node *
  varpool_extra_name_alias (tree alias, tree decl)
  {
!   struct varpool_node *alias_node;
  
  #ifndef ASM_OUTPUT_DEF
    /* If aliases aren't supported by the assembler, fail.  */
    return NULL;
  #endif
!   alias_node = varpool_create_variable_alias (alias, decl);
!   alias_node->extra_name_alias = true;
    gcc_assert (TREE_CODE (decl) == VAR_DECL);
    gcc_assert (TREE_CODE (alias) == VAR_DECL);
!   alias_node = varpool_node (alias);
    alias_node->alias = 1;
!   alias_node->finalized = 1;
!   alias_node->alias_of = decl;
!   if (decide_is_variable_needed (alias_node, alias)
!       || alias_node->needed)
!     varpool_mark_needed_node (alias_node);
    return alias_node;
  }
  
*************** varpool_extra_name_alias (tree alias, tr
*** 711,727 ****
  bool
  varpool_used_from_object_file_p (struct varpool_node *node)
  {
-   struct varpool_node *alias;
- 
    if (!TREE_PUBLIC (node->decl))
      return false;
    if (resolution_used_from_other_file_p (node->resolution))
      return true;
-   for (alias = node->extra_name; alias; alias = alias->next)
-     if (TREE_PUBLIC (alias->decl)
- 	&& resolution_used_from_other_file_p (alias->resolution))
-       return true;
    return false;
  }
  
  #include "gt-varpool.h"
--- 743,780 ----
  bool
  varpool_used_from_object_file_p (struct varpool_node *node)
  {
    if (!TREE_PUBLIC (node->decl))
      return false;
    if (resolution_used_from_other_file_p (node->resolution))
      return true;
    return false;
  }
  
+ /* Call calback on NODE and aliases asociated to NODE. 
+    When INCLUDE_OVERWRITABLE is false, overwritable aliases and thunks are
+    skipped. */
+ 
+ bool
+ varpool_for_node_and_aliases (struct varpool_node *node,
+ 			      bool (*callback) (struct varpool_node *, void *),
+ 			      void *data,
+ 			      bool include_overwritable)
+ {
+   int i;
+   struct ipa_ref *ref;
+ 
+   if (callback (node, data))
+     return true;
+   for (i = 0; ipa_ref_list_refering_iterate (&node->ref_list, i, ref); i++)
+     if (ref->use == IPA_REF_ALIAS)
+       {
+ 	struct varpool_node *alias = ipa_ref_refering_varpool_node (ref);
+ 	if (include_overwritable
+ 	    || cgraph_variable_initializer_availability (alias) > AVAIL_OVERWRITABLE)
+           if (varpool_for_node_and_aliases (alias, callback, data,
+ 					   include_overwritable))
+ 	    return true;
+       }
+   return false;
+ }
  #include "gt-varpool.h"

Re: varpool alias reorg

[H.J. Lu]

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

-- 
H.J.

Regression with "varpool alias reorg"

[Hans-Peter Nilsson]

On Sat, 18 Jun 2011, Jan Hubicka wrote:

> 	* 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 a regression for emutls targets, see PR49500.

brgds, H-P

Re: varpool alias reorg

[H.J. Lu]

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.

-- 
H.J.

Re: varpool alias reorg

[Jan Hubicka]

> 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?

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)
 {

Re: varpool alias reorg

[Jan Hubicka]

Hi,
I also tested the attached variant that simply disable the builtins streaming.
It fixes the testcase, too, bootstraps/regtestes x86_64 with and without plugin
and builds mozilla. It also solves the decl sharing problems that leads to
debug info confussion.
However it breaks memops-asm.c testcase.  What testcase does is giving builtlins
asm name (i.e. my_memcpy instead of memcpy) and then it tests that the new calls
to the builtilns introduced via folding actually use these asm names.

The code this patch remove was probably invented specifically for this testcase:
instead of streaming builtin decl like we do all other streaming, we just stream
reference to the builtin + an asm name and the single builtin decl in the
LTO unit gets the name.

The problem is that this won't work when LTOing multiple such units each giving a
different asm name (i.e. one of units will win). We can't quite fix this because
we don't want to keep track from where the code we are folding is comming.

So I wonder, do we really need to preserve this behaviour?  
I do not think it is documented anywhere and it seems to me that both variants:
ignoring the asm names or honoring them are sane.

Honza

Index: testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c
===================================================================
*** testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c	(revision 175350)
--- testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c	(working copy)
*************** typedef __SIZE_TYPE__ size_t;
*** 6,12 ****
  
  /* LTO code is at the present to able to track that asm alias my_bcopy on builtin
     actually refers to this function.  See PR47181. */
- __attribute__ ((used))
  void *
  my_memcpy (void *d, const void *s, size_t n)
  {
--- 6,11 ----
*************** my_memcpy (void *d, const void *s, size_
*** 19,25 ****
  
  /* LTO code is at the present to able to track that asm alias my_bcopy on builtin
     actually refers to this function.  See PR47181. */
- __attribute__ ((used))
  void
  my_bcopy (const void *s, void *d, size_t n)
  {
--- 18,23 ----
*************** my_bcopy (const void *s, void *d, size_t
*** 39,45 ****
  
  /* LTO code is at the present to able to track that asm alias my_bcopy on builtin
     actually refers to this function.  See PR47181. */
- __attribute__ ((used))
  void *
  my_memset (void *d, int c, size_t n)
  {
--- 37,42 ----
*************** my_memset (void *d, int c, size_t n)
*** 51,57 ****
  
  /* LTO code is at the present to able to track that asm alias my_bcopy on builtin
     actually refers to this function.  See PR47181. */
- __attribute__ ((used))
  void
  my_bzero (void *d, size_t n)
  {
--- 48,53 ----
Index: lto-streamer-out.c
===================================================================
*** lto-streamer-out.c	(revision 175350)
--- lto-streamer-out.c	(working copy)
*************** pack_ts_function_decl_value_fields (stru
*** 484,491 ****
  {
    /* For normal/md builtins we only write the class and code, so they
       should never be handled here.  */
-   gcc_assert (!lto_stream_as_builtin_p (expr));
- 
    bp_pack_enum (bp, built_in_class, BUILT_IN_LAST,
  		DECL_BUILT_IN_CLASS (expr));
    bp_pack_value (bp, DECL_STATIC_CONSTRUCTOR (expr), 1);
--- 484,489 ----
*************** lto_output_tree_header (struct output_bl
*** 1306,1346 ****
  }
  
  
- /* Write the code and class of builtin EXPR to output block OB.  IX is
-    the index into the streamer cache where EXPR is stored.*/
- 
- static void
- lto_output_builtin_tree (struct output_block *ob, tree expr)
- {
-   gcc_assert (lto_stream_as_builtin_p (expr));
- 
-   if (DECL_BUILT_IN_CLASS (expr) == BUILT_IN_MD
-       && !targetm.builtin_decl)
-     sorry ("gimple bytecode streams do not support machine specific builtin "
- 	   "functions on this target");
- 
-   output_record_start (ob, LTO_builtin_decl);
-   lto_output_enum (ob->main_stream, built_in_class, BUILT_IN_LAST,
- 		   DECL_BUILT_IN_CLASS (expr));
-   output_uleb128 (ob, DECL_FUNCTION_CODE (expr));
- 
-   if (DECL_ASSEMBLER_NAME_SET_P (expr))
-     {
-       /* When the assembler name of a builtin gets a user name,
- 	 the new name is always prefixed with '*' by
- 	 set_builtin_user_assembler_name.  So, to prevent the
- 	 reader side from adding a second '*', we omit it here.  */
-       const char *str = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (expr));
-       if (strlen (str) > 1 && str[0] == '*')
- 	lto_output_string (ob, ob->main_stream, &str[1], true);
-       else
- 	lto_output_string (ob, ob->main_stream, NULL, true);
-     }
-   else
-     lto_output_string (ob, ob->main_stream, NULL, true);
- }
- 
- 
  /* Write a physical representation of tree node EXPR to output block
     OB.  If REF_P is true, the leaves of EXPR are emitted as references
     via lto_output_tree_ref.  IX is the index into the streamer cache
--- 1304,1309 ----
*************** lto_output_tree (struct output_block *ob
*** 1456,1470 ****
        lto_output_enum (ob->main_stream, LTO_tags, LTO_NUM_TAGS,
  		       lto_tree_code_to_tag (TREE_CODE (expr)));
      }
-   else if (lto_stream_as_builtin_p (expr))
-     {
-       /* MD and NORMAL builtins do not need to be written out
- 	 completely as they are always instantiated by the
- 	 compiler on startup.  The only builtins that need to
- 	 be written out are BUILT_IN_FRONTEND.  For all other
- 	 builtins, we simply write the class and code.  */
-       lto_output_builtin_tree (ob, expr);
-     }
    else
      {
        /* This is the first time we see EXPR, write its fields
--- 1419,1424 ----
Index: lto-streamer-in.c
===================================================================
*** lto-streamer-in.c	(revision 175350)
--- lto-streamer-in.c	(working copy)
*************** lto_get_pickled_tree (struct lto_input_b
*** 2434,2481 ****
  }
  
  
- /* Read a code and class from input block IB and return the
-    corresponding builtin.  DATA_IN is as in lto_input_tree.  */
- 
- static tree
- lto_get_builtin_tree (struct lto_input_block *ib, struct data_in *data_in)
- {
-   enum built_in_class fclass;
-   enum built_in_function fcode;
-   const char *asmname;
-   tree result;
- 
-   fclass = lto_input_enum (ib, built_in_class, BUILT_IN_LAST);
-   gcc_assert (fclass == BUILT_IN_NORMAL || fclass == BUILT_IN_MD);
- 
-   fcode = (enum built_in_function) lto_input_uleb128 (ib);
- 
-   if (fclass == BUILT_IN_NORMAL)
-     {
-       if (fcode >= END_BUILTINS)
- 	fatal_error ("machine independent builtin code out of range");
-       result = built_in_decls[fcode];
-       gcc_assert (result);
-     }
-   else if (fclass == BUILT_IN_MD)
-     {
-       result = targetm.builtin_decl (fcode, true);
-       if (!result || result == error_mark_node)
- 	fatal_error ("target specific builtin not available");
-     }
-   else
-     gcc_unreachable ();
- 
-   asmname = lto_input_string (data_in, ib);
-   if (asmname)
-     set_builtin_user_assembler_name (result, asmname);
- 
-   lto_streamer_cache_append (data_in->reader_cache, result);
- 
-   return result;
- }
- 
- 
  /* Read the physical representation of a tree node with tag TAG from
     input block IB using the per-file context in DATA_IN.  */
  
--- 2434,2439 ----
*************** lto_read_tree (struct lto_input_block *i
*** 2495,2504 ****
    if (streamer_hooks.read_tree)
      streamer_hooks.read_tree (ib, data_in, result);
  
-   /* We should never try to instantiate an MD or NORMAL builtin here.  */
-   if (TREE_CODE (result) == FUNCTION_DECL)
-     gcc_assert (!lto_stream_as_builtin_p (result));
- 
    /* end_marker = */ lto_input_1_unsigned (ib);
  
  #ifdef LTO_STREAMER_DEBUG
--- 2453,2458 ----
*************** lto_input_tree (struct lto_input_block *
*** 2582,2593 ****
  	 the reader cache.  */
        result = lto_get_pickled_tree (ib, data_in);
      }
-   else if (tag == LTO_builtin_decl)
-     {
-       /* If we are going to read a built-in function, all we need is
- 	 the code and class.  */
-       result = lto_get_builtin_tree (ib, data_in);
-     }
    else if (tag == lto_tree_code_to_tag (INTEGER_CST))
      {
        /* For integer constants we only need the type and its hi/low
--- 2536,2541 ----
Index: lto-streamer.h
===================================================================
*** lto-streamer.h	(revision 175350)
--- lto-streamer.h	(working copy)
*************** enum LTO_tags
*** 198,206 ****
    /* EH region holding the previous statement.  */
    LTO_eh_region,
  
-   /* An MD or NORMAL builtin.  Only the code and class are streamed out.  */
-   LTO_builtin_decl,
- 
    /* Function body.  */
    LTO_function,
  
--- 198,203 ----
*************** emit_label_in_global_context_p (tree lab
*** 1141,1157 ****
    return DECL_NONLOCAL (label) || FORCED_LABEL (label);
  }
  
- /* Return true if tree node EXPR should be streamed as a builtin.  For
-    these nodes, we just emit the class and function code.  */
- static inline bool
- lto_stream_as_builtin_p (tree expr)
- {
-   return (TREE_CODE (expr) == FUNCTION_DECL
- 	  && DECL_IS_BUILTIN (expr)
- 	  && (DECL_BUILT_IN_CLASS (expr) == BUILT_IN_NORMAL
- 	      || DECL_BUILT_IN_CLASS (expr) == BUILT_IN_MD));
- }
- 
  DEFINE_DECL_STREAM_FUNCS (TYPE, type)
  DEFINE_DECL_STREAM_FUNCS (FIELD_DECL, field_decl)
  DEFINE_DECL_STREAM_FUNCS (FN_DECL, fn_decl)
--- 1138,1143 ----

Re: varpool alias reorg

[Jan Hubicka]

Hi,
this is yet another variant of the fix.  This time we stream builtins decls as
usually, but at fixup time we copy the assembler names (if set) into the
builtin decls used by folders.  Not sure if it is any better than breaking
memops-asm, but I can imagine that things like glibc actually rename string
functions into their internal variants (and thus with this version of patch we
would be able to LTO such library, but still we won't be able to LTO such
library into something else because something else would end up referncing the
internal versions of builtins).  I doubt we could do any better, however.

__attribute__ ((used)) is still needed in memops-asm-lib.c because LTO symtab
of course doesn't see the future references to builtins that we will emit
later via folding.  I think it is resonable requirement, as discussed at the
time enabling the plugin.

Honza

Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 175350)
+++ lto-streamer-out.c	(working copy)
@@ -484,8 +484,6 @@ pack_ts_function_decl_value_fields (stru
 {
   /* For normal/md builtins we only write the class and code, so they
      should never be handled here.  */
-  gcc_assert (!lto_stream_as_builtin_p (expr));
-
   bp_pack_enum (bp, built_in_class, BUILT_IN_LAST,
 		DECL_BUILT_IN_CLASS (expr));
   bp_pack_value (bp, DECL_STATIC_CONSTRUCTOR (expr), 1);
@@ -1121,7 +1119,7 @@ lto_output_ts_binfo_tree_pointers (struc
      together large portions of programs making it harder to partition.  Becuase
      devirtualization is interesting before inlining, only, there is no real
      need to ship it into ltrans partition.  */
-  lto_output_tree_or_ref (ob, flag_wpa ? NULL : BINFO_VIRTUALS (expr), ref_p);
+  lto_output_tree_or_ref (ob, flag_wpa || 1 ? NULL : BINFO_VIRTUALS (expr), ref_p);
   lto_output_tree_or_ref (ob, BINFO_VPTR_FIELD (expr), ref_p);
 
   output_uleb128 (ob, VEC_length (tree, BINFO_BASE_ACCESSES (expr)));
@@ -1306,41 +1304,6 @@ lto_output_tree_header (struct output_bl
 }
 
 
-/* Write the code and class of builtin EXPR to output block OB.  IX is
-   the index into the streamer cache where EXPR is stored.*/
-
-static void
-lto_output_builtin_tree (struct output_block *ob, tree expr)
-{
-  gcc_assert (lto_stream_as_builtin_p (expr));
-
-  if (DECL_BUILT_IN_CLASS (expr) == BUILT_IN_MD
-      && !targetm.builtin_decl)
-    sorry ("gimple bytecode streams do not support machine specific builtin "
-	   "functions on this target");
-
-  output_record_start (ob, LTO_builtin_decl);
-  lto_output_enum (ob->main_stream, built_in_class, BUILT_IN_LAST,
-		   DECL_BUILT_IN_CLASS (expr));
-  output_uleb128 (ob, DECL_FUNCTION_CODE (expr));
-
-  if (DECL_ASSEMBLER_NAME_SET_P (expr))
-    {
-      /* When the assembler name of a builtin gets a user name,
-	 the new name is always prefixed with '*' by
-	 set_builtin_user_assembler_name.  So, to prevent the
-	 reader side from adding a second '*', we omit it here.  */
-      const char *str = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (expr));
-      if (strlen (str) > 1 && str[0] == '*')
-	lto_output_string (ob, ob->main_stream, &str[1], true);
-      else
-	lto_output_string (ob, ob->main_stream, NULL, true);
-    }
-  else
-    lto_output_string (ob, ob->main_stream, NULL, true);
-}
-
-
 /* Write a physical representation of tree node EXPR to output block
    OB.  If REF_P is true, the leaves of EXPR are emitted as references
    via lto_output_tree_ref.  IX is the index into the streamer cache
@@ -1456,15 +1419,6 @@ lto_output_tree (struct output_block *ob
       lto_output_enum (ob->main_stream, LTO_tags, LTO_NUM_TAGS,
 		       lto_tree_code_to_tag (TREE_CODE (expr)));
     }
-  else if (lto_stream_as_builtin_p (expr))
-    {
-      /* MD and NORMAL builtins do not need to be written out
-	 completely as they are always instantiated by the
-	 compiler on startup.  The only builtins that need to
-	 be written out are BUILT_IN_FRONTEND.  For all other
-	 builtins, we simply write the class and code.  */
-      lto_output_builtin_tree (ob, expr);
-    }
   else
     {
       /* This is the first time we see EXPR, write its fields
Index: lto-streamer-in.c
===================================================================
--- lto-streamer-in.c	(revision 175350)
+++ lto-streamer-in.c	(working copy)
@@ -1736,18 +1736,7 @@ unpack_ts_function_decl_value_fields (st
   DECL_PURE_P (expr) = (unsigned) bp_unpack_value (bp, 1);
   DECL_LOOPING_CONST_OR_PURE_P (expr) = (unsigned) bp_unpack_value (bp, 1);
   if (DECL_BUILT_IN_CLASS (expr) != NOT_BUILT_IN)
-    {
-      DECL_FUNCTION_CODE (expr) = (enum built_in_function) bp_unpack_value (bp, 11);
-      if (DECL_BUILT_IN_CLASS (expr) == BUILT_IN_NORMAL
-	  && DECL_FUNCTION_CODE (expr) >= END_BUILTINS)
-	fatal_error ("machine independent builtin code out of range");
-      else if (DECL_BUILT_IN_CLASS (expr) == BUILT_IN_MD)
-	{
-          tree result = targetm.builtin_decl (DECL_FUNCTION_CODE (expr), true);
-	  if (!result || result == error_mark_node)
-	    fatal_error ("target specific builtin not available");
-	}
-    }
+    DECL_FUNCTION_CODE (expr) = (enum built_in_function) bp_unpack_value (bp, 11);
   if (DECL_STATIC_DESTRUCTOR (expr))
     {
       priority_type p;
@@ -2434,48 +2423,6 @@ lto_get_pickled_tree (struct lto_input_b
 }
 
 
-/* Read a code and class from input block IB and return the
-   corresponding builtin.  DATA_IN is as in lto_input_tree.  */
-
-static tree
-lto_get_builtin_tree (struct lto_input_block *ib, struct data_in *data_in)
-{
-  enum built_in_class fclass;
-  enum built_in_function fcode;
-  const char *asmname;
-  tree result;
-
-  fclass = lto_input_enum (ib, built_in_class, BUILT_IN_LAST);
-  gcc_assert (fclass == BUILT_IN_NORMAL || fclass == BUILT_IN_MD);
-
-  fcode = (enum built_in_function) lto_input_uleb128 (ib);
-
-  if (fclass == BUILT_IN_NORMAL)
-    {
-      if (fcode >= END_BUILTINS)
-	fatal_error ("machine independent builtin code out of range");
-      result = built_in_decls[fcode];
-      gcc_assert (result);
-    }
-  else if (fclass == BUILT_IN_MD)
-    {
-      result = targetm.builtin_decl (fcode, true);
-      if (!result || result == error_mark_node)
-	fatal_error ("target specific builtin not available");
-    }
-  else
-    gcc_unreachable ();
-
-  asmname = lto_input_string (data_in, ib);
-  if (asmname)
-    set_builtin_user_assembler_name (result, asmname);
-
-  lto_streamer_cache_append (data_in->reader_cache, result);
-
-  return result;
-}
-
-
 /* Read the physical representation of a tree node with tag TAG from
    input block IB using the per-file context in DATA_IN.  */
 
@@ -2495,10 +2442,6 @@ lto_read_tree (struct lto_input_block *i
   if (streamer_hooks.read_tree)
     streamer_hooks.read_tree (ib, data_in, result);
 
-  /* We should never try to instantiate an MD or NORMAL builtin here.  */
-  if (TREE_CODE (result) == FUNCTION_DECL)
-    gcc_assert (!lto_stream_as_builtin_p (result));
-
   /* end_marker = */ lto_input_1_unsigned (ib);
 
 #ifdef LTO_STREAMER_DEBUG
@@ -2582,12 +2525,6 @@ lto_input_tree (struct lto_input_block *
 	 the reader cache.  */
       result = lto_get_pickled_tree (ib, data_in);
     }
-  else if (tag == LTO_builtin_decl)
-    {
-      /* If we are going to read a built-in function, all we need is
-	 the code and class.  */
-      result = lto_get_builtin_tree (ib, data_in);
-    }
   else if (tag == lto_tree_code_to_tag (INTEGER_CST))
     {
       /* For integer constants we only need the type and its hi/low
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 175350)
+++ lto/lto.c	(working copy)
@@ -669,6 +669,41 @@ uniquify_nodes (struct data_in *data_in,
       if (!t)
 	continue;
 
+  if (TREE_CODE (t) == FUNCTION_DECL
+      && DECL_BUILT_IN_CLASS (t) != NOT_BUILT_IN)
+    {
+      tree decl = NULL;
+      if (DECL_BUILT_IN_CLASS (t) == BUILT_IN_NORMAL)
+	{
+	  if (DECL_FUNCTION_CODE (t) >= END_BUILTINS)
+	    fatal_error ("machine independent builtin code out of range");
+	  else
+	    decl = built_in_decls[DECL_FUNCTION_CODE (t)];
+	}
+      else if (DECL_BUILT_IN_CLASS (t) == BUILT_IN_MD)
+	{
+          decl = targetm.builtin_decl (DECL_FUNCTION_CODE (t), true);
+	  if (!decl || decl == error_mark_node)
+	    fatal_error ("target specific builtin not available");
+	}
+      /* When user assembler name is set, change the bultin decl used by
+	 folder and rtl expansion.
+	 ??? this is not always quite correct: when multiple units are merged
+	 together, one of assembler names will win.  However this solve at
+	 least the memops-asm testcase.  */
+      if (decl
+	  && DECL_ASSEMBLER_NAME_SET_P (t))
+	{
+	   /* When the assembler name of a builtin gets a user name,
+	     the new name is always prefixed with '*' by
+	     set_builtin_user_assembler_name.  So, to prevent the
+	     reader side from adding a second '*', we omit it here.  */
+	   const char *str = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (t));
+	   if (strlen (str) > 1 && str[0] == '*')
+	     set_builtin_user_assembler_name (decl, &str[1]);
+	}
+    }
+
       /* First fixup the fields of T.  */
       lto_fixup_types (t);
 
Index: lto-streamer.h
===================================================================
--- lto-streamer.h	(revision 175350)
+++ lto-streamer.h	(working copy)
@@ -198,9 +198,6 @@ enum LTO_tags
   /* EH region holding the previous statement.  */
   LTO_eh_region,
 
-  /* An MD or NORMAL builtin.  Only the code and class are streamed out.  */
-  LTO_builtin_decl,
-
   /* Function body.  */
   LTO_function,
 
@@ -1141,17 +1138,6 @@ emit_label_in_global_context_p (tree lab
   return DECL_NONLOCAL (label) || FORCED_LABEL (label);
 }
 
-/* Return true if tree node EXPR should be streamed as a builtin.  For
-   these nodes, we just emit the class and function code.  */
-static inline bool
-lto_stream_as_builtin_p (tree expr)
-{
-  return (TREE_CODE (expr) == FUNCTION_DECL
-	  && DECL_IS_BUILTIN (expr)
-	  && (DECL_BUILT_IN_CLASS (expr) == BUILT_IN_NORMAL
-	      || DECL_BUILT_IN_CLASS (expr) == BUILT_IN_MD));
-}
-
 DEFINE_DECL_STREAM_FUNCS (TYPE, type)
 DEFINE_DECL_STREAM_FUNCS (FIELD_DECL, field_decl)
 DEFINE_DECL_STREAM_FUNCS (FN_DECL, fn_decl)

Re: varpool alias reorg

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4751 bytes --]

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)
>  {

Re: varpool alias reorg

[Jan Hubicka]

> > 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.
Hi,
thanks!  Could you, please, also consider the second two variants I sent?  In
general attribute used is needed when implementing libfuncs since we never know
when folding will intoduce new libcall (like in memops-asm).  However this
particular case don't need use because the builtin is called directly.
They also solve the problem of BLOCK lists being messed up.

In meantime I tested both variants and they pass.  I will add a changelog, of course.

Honza

Re: varpool alias reorg

[Richard Guenther]

On Fri, 24 Jun 2011, Jan Hubicka wrote:

> Hi,
> this is yet another variant of the fix.  This time we stream builtins decls as
> usually, but at fixup time we copy the assembler names (if set) into the
> builtin decls used by folders.  Not sure if it is any better than breaking
> memops-asm, but I can imagine that things like glibc actually rename string
> functions into their internal variants (and thus with this version of patch we
> would be able to LTO such library, but still we won't be able to LTO such
> library into something else because something else would end up referncing the
> internal versions of builtins).  I doubt we could do any better, however.

Not stream builtins with adjusted assembler names (I guess we'd need
a flag for this, DECL_USER_ASSEMBLER_NAME_SET_P?  Or just check for
attributes?) as builtins but as new decls.  Let lto symbol merging
then register those as aliases.  But which way around?  probably
similar to how we should handle re-defined extern inlines, the
extern inline being the GCC builtin and the re-definition being
the aliased one.

> __attribute__ ((used)) is still needed in memops-asm-lib.c because LTO symtab
> of course doesn't see the future references to builtins that we will emit
> later via folding.  I think it is resonable requirement, as discussed at the
> time enabling the plugin.

Yes, I think the testcase fix sounds reasonable.

I suppose you can come up with a simpler testcase for this "feature"
for gcc.dg/lto highlighting the different issues?  I'm not sure
if we are talking about my_memcpy () alias("memcpy") or
memcpy () alias("my_memcpy").

I still like to stream unmodified builtins as builtins, as that is
similar to pre-loading the streamer caches with things like
void_type_node or sizetype.

Richard.

Re: varpool alias reorg

[Jan Hubicka]

> On Fri, 24 Jun 2011, Jan Hubicka wrote:
> 
> > Hi,
> > this is yet another variant of the fix.  This time we stream builtins decls as
> > usually, but at fixup time we copy the assembler names (if set) into the
> > builtin decls used by folders.  Not sure if it is any better than breaking
> > memops-asm, but I can imagine that things like glibc actually rename string
> > functions into their internal variants (and thus with this version of patch we
> > would be able to LTO such library, but still we won't be able to LTO such
> > library into something else because something else would end up referncing the
> > internal versions of builtins).  I doubt we could do any better, however.
> 
> Not stream builtins with adjusted assembler names (I guess we'd need
> a flag for this, DECL_USER_ASSEMBLER_NAME_SET_P?  Or just check for

Most of code just checks for '*' on begginign of assembler name. I suppose it is safe.

> attributes?) as builtins but as new decls.  Let lto symbol merging
> then register those as aliases.  But which way around?  probably
> similar to how we should handle re-defined extern inlines, the
> extern inline being the GCC builtin and the re-definition being
> the aliased one.

I don't quite get your answer here.  What we do now is:

 1) stream in builtin as special kind of reference with decl assembler name associated to it
 2) at streaming in time always resolve builtlin to the "official" builtin decls (no matter
 what types and other stuff builtin had at stream out time) and overwritting the official builtin
 assembler name into one specified.

What i suggest is

 1) Stream out builtins as usual decls just with the extra function code
 2) Stream in builtins as usually
 3) optionally set the assembler name of the "official" decl

I see there are problems with i.e. one decl rule, but we do have same problems
with normal frontends that also do use different decl for explicit builtin
calls than for implicit, sadly.

I am not quite sure what the proper fix for this problem is - it is very handy
to have builtin decl in middle end where I know it is sane (i.e. it has the
right types etc.). Since C allows to declare the builtins arbitrarily, it gets
bit tricky to preserve one decl rule here.
> 
> > __attribute__ ((used)) is still needed in memops-asm-lib.c because LTO symtab
> > of course doesn't see the future references to builtins that we will emit
> > later via folding.  I think it is resonable requirement, as discussed at the
> > time enabling the plugin.
> 
> Yes, I think the testcase fix sounds reasonable.
> 
> I suppose you can come up with a simpler testcase for this "feature"
> for gcc.dg/lto highlighting the different issues?  I'm not sure
> if we are talking about my_memcpy () alias("memcpy") or
> memcpy () alias("my_memcpy").
> 
> I still like to stream unmodified builtins as builtins, as that is
> similar to pre-loading the streamer caches with things like
> void_type_node or sizetype.

Doing so will need us to solve the other one decl rules probly.
I didn't really got what the preloading is useful for after all?

Honza

> 
> Richard.

Re: varpool alias reorg

[Michael Matz]

Hi,

On Mon, 27 Jun 2011, Jan Hubicka wrote:

> > I still like to stream unmodified builtins as builtins, as that is 
> > similar to pre-loading the streamer caches with things like 
> > void_type_node or sizetype.
> 
> Doing so will need us to solve the other one decl rules probly. I didn't 
> really got what the preloading is useful for after all?

One important thing that really affects correctness which preloading does 
is to guarantee pointer equality for things like void_type_node or 
error_mark_node, which are used sometimes.  If we weren't doing preloading 
we would have to forcibly merge all these trees over different units, and 
what's worse, also fill out the global tree arrays with that merged 
variant.  And for that we'd need to note somehow which array slot a 
certain tree is coming from (and deal with the fact that different 
language frontends fill this array differently, sometimes with 
pointer-eqal tree nodes, sometimes only with semantically equal tree 
nodes, sometimes not at all).

Or we could fix all places where we use pointer equality with some of the 
global trees, which I wouldn't like, even for abstract considerations.  
There's really no point in having different but equal void_type nodes, or 
error_mark nodes.

preloading really is the easiest way to solve all this.  It's just 
important that all .o files have the same idea about "tree at slot 
so-and-so" (e.g. meaning "error_mark_node"), which I fixed some weeks ago.

And with early-debug-info we don't even then have the issue of e.g. the 
base integer types not being named like the frontend emitted them.


Ciao,
Michael.

Re: varpool alias reorg

[Richard Guenther]

On Mon, 27 Jun 2011, Jan Hubicka wrote:

> > On Fri, 24 Jun 2011, Jan Hubicka wrote:
> > 
> > > Hi,
> > > this is yet another variant of the fix.  This time we stream builtins decls as
> > > usually, but at fixup time we copy the assembler names (if set) into the
> > > builtin decls used by folders.  Not sure if it is any better than breaking
> > > memops-asm, but I can imagine that things like glibc actually rename string
> > > functions into their internal variants (and thus with this version of patch we
> > > would be able to LTO such library, but still we won't be able to LTO such
> > > library into something else because something else would end up referncing the
> > > internal versions of builtins).  I doubt we could do any better, however.
> > 
> > Not stream builtins with adjusted assembler names (I guess we'd need
> > a flag for this, DECL_USER_ASSEMBLER_NAME_SET_P?  Or just check for
> 
> Most of code just checks for '*' on begginign of assembler name. I suppose it is safe.
> 
> > attributes?) as builtins but as new decls.  Let lto symbol merging
> > then register those as aliases.  But which way around?  probably
> > similar to how we should handle re-defined extern inlines, the
> > extern inline being the GCC builtin and the re-definition being
> > the aliased one.
> 
> I don't quite get your answer here.  What we do now is:
> 
>  1) stream in builtin as special kind of reference with decl assembler name associated to it
>  2) at streaming in time always resolve builtlin to the "official" builtin decls (no matter
>  what types and other stuff builtin had at stream out time) and overwritting the official builtin
>  assembler name into one specified.
> 
> What i suggest is
> 
>  1) Stream out builtins as usual decls just with the extra function code
>  2) Stream in builtins as usually
>  3) optionally set the assembler name of the "official" decl
> 
> I see there are problems with i.e. one decl rule, but we do have same problems
> with normal frontends that also do use different decl for explicit builtin
> calls than for implicit, sadly.
> 
> I am not quite sure what the proper fix for this problem is - it is very handy
> to have builtin decl in middle end where I know it is sane (i.e. it has the
> right types etc.). Since C allows to declare the builtins arbitrarily, it gets
> bit tricky to preserve one decl rule here.

Hm.  I would suggest to do as now, stream in builtin specially if it
does not have an assembler name attribute.  If it does have it, stream
it as usually and let lto symtab do its job (I suppose we need to
register builtin functions with the symtab as well).

> > > __attribute__ ((used)) is still needed in memops-asm-lib.c because LTO symtab
> > > of course doesn't see the future references to builtins that we will emit
> > > later via folding.  I think it is resonable requirement, as discussed at the
> > > time enabling the plugin.
> > 
> > Yes, I think the testcase fix sounds reasonable.
> > 
> > I suppose you can come up with a simpler testcase for this "feature"
> > for gcc.dg/lto highlighting the different issues?  I'm not sure
> > if we are talking about my_memcpy () alias("memcpy") or
> > memcpy () alias("my_memcpy").
> > 
> > I still like to stream unmodified builtins as builtins, as that is
> > similar to pre-loading the streamer caches with things like
> > void_type_node or sizetype.
> 
> Doing so will need us to solve the other one decl rules probly.
> I didn't really got what the preloading is useful for after all?

Saving memory mostly, apart from the special singletons we have
(as Micha already hinted).

Richard.