public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR lto/47497 (undefined symbol when building soplex)
@ 2011-03-02 10:07 Jan Hubicka
  2011-03-02 10:36 ` Richard Guenther
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Hubicka @ 2011-03-02 10:07 UTC (permalink / raw)
  To: gcc-patches


Hi,
the problem is with thunks referring thunks or aliases.

lto-symtab is wrong here and when moving thunks&aliases associated with one
node to the other node, it overwrites thunk.alias by the destination node. It
consequently turns thunk referring another thunk into thunk referring the
functoin itself.

I fixed this by adding extra loop setting alias decl to prevailing decl. Richi,
perhaps there is better place to put this?  I don't like it being in the loop
redirecting thunks&aliases from one node to another because

 1) I think that loop is not quite correct.  When one function is prevailed by
another, local static thunk from the first function should not be redirected to
another.  That happens correctly and is harmless (we end up with dead thunk)
 2) It may happen that thunks get prevailed other way than nodes they are
aliased with.  We use comdat groups that prevents this from happening, but I
would rather not 100% rely on this on all targets since not all targets
implements comdat groups.  So I think it is more robust to simply merge the
decls in alias field like we merge other decls.

Fixing this problem cause different problem with streaming.  When we have alias
A referring function F and alias B referring alias A and we are unlucky with
prevailing and other things, we might end up streaming alias B before alias A. 
This leads us to call cgraph_same_body_alias on decl of A before A is added to
cgraph as an alias.  Consequently cgraph_same_body_alias does nothing later
when we try to create alias A itself, because the node already exists.

This patch fixes it by adding node pointer into the cgraph_same_body_alias and
cgraph_add_thunk so the thunks&aliases can be added in random order w/o
problems as long as the function they are associated with is already in cgraph.

Bootstraped/regtested x86_64-linux, also tested with Mozilla and qt build.

Honza
	PR lto/47497
	* lto-symtab.c (lto_cgraph_replace_node): Do not set thunk.alias.
	(lto_symtab_merge_cgraph_nodes_1): Update thunk.alias pointers here.
	* cgraph.h (cgraph_same_body_alias, cgraph_add_thunk): Add node pointers.
	* cgraph.c (cgraph_same_body_alias_1, cgraph_same_body_alias,
	cgraph_add_thunk): Add node pointers.
	* lto-cgraph.c (lto_output_node): Verify that thunks&aliases are
	associated to right node.
	(input_node): Update use of cgraph_same_body_alias
	and cgraph_add_thunk.

	* optimize.c (maybe_clone_body): Update call of cgraph_same_body_alias
	and cgraph_add_thunk.
	* method.c (make_alias_for_thunk, use_thunk): Likewise.
	* mangle.c (mangle_decl): Likewise.
Index: lto-symtab.c
===================================================================
--- lto-symtab.c	(revision 170364)
+++ lto-symtab.c	(working copy)
@@ -273,7 +273,6 @@ lto_cgraph_replace_node (struct cgraph_n
 	  last = alias;
 	  gcc_assert (alias->same_body_alias);
 	  alias->same_body = prevailing_node;
-	  alias->thunk.alias = prevailing_node->decl;
 	}
       last->next = prevailing_node->same_body;
       /* Node with aliases is prevailed by alias.
@@ -828,8 +827,16 @@ lto_symtab_merge_cgraph_nodes_1 (void **
 void
 lto_symtab_merge_cgraph_nodes (void)
 {
+  struct cgraph_node *node, *alias, *next;
   lto_symtab_maybe_init_hash_table ();
   htab_traverse (lto_symtab_identifiers, lto_symtab_merge_cgraph_nodes_1, NULL);
+
+  for (node = cgraph_nodes; node; node = node->next)
+    for (alias = node->same_body; alias; alias = next)
+      {
+	next = alias->next;
+	alias->thunk.alias = lto_symtab_prevailing_decl (alias->thunk.alias);
+      }
 }
 
 /* Given the decl DECL, return the prevailing decl with the same name. */
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 170364)
+++ cgraph.h	(working copy)
@@ -559,8 +559,8 @@ struct cgraph_indirect_call_info *cgraph
 struct cgraph_node * cgraph_get_node (const_tree);
 struct cgraph_node * cgraph_get_node_or_alias (const_tree);
 struct cgraph_node * cgraph_node (tree);
-struct cgraph_node * cgraph_same_body_alias (tree, tree);
-struct cgraph_node * cgraph_add_thunk (tree, tree, bool, HOST_WIDE_INT,
+struct cgraph_node * cgraph_same_body_alias (struct cgraph_node *, tree, tree);
+struct cgraph_node * cgraph_add_thunk (struct cgraph_node *, tree, tree, bool, HOST_WIDE_INT,
 				       HOST_WIDE_INT, tree, tree);
 void cgraph_remove_same_body_alias (struct cgraph_node *);
 struct cgraph_node *cgraph_node_for_asm (tree);
Index: cgraph.c
===================================================================
--- cgraph.c	(revision 170364)
+++ cgraph.c	(working copy)
@@ -536,16 +536,16 @@ cgraph_node (tree decl)
   return node;
 }
 
-/* Mark ALIAS as an alias to DECL.  */
+/* Mark ALIAS as an alias to DECL.  DECL_NODE is cgraph node representing
+   the function body is associated with (not neccesarily cgraph_node (DECL).  */
 
 static struct cgraph_node *
-cgraph_same_body_alias_1 (tree alias, tree decl)
+cgraph_same_body_alias_1 (struct cgraph_node *decl_node, tree alias, tree decl)
 {
-  struct cgraph_node key, *alias_node, *decl_node, **slot;
+  struct cgraph_node key, *alias_node, **slot;
 
   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
   gcc_assert (TREE_CODE (alias) == FUNCTION_DECL);
-  decl_node = cgraph_node (decl);
 
   key.decl = alias;
 
@@ -575,7 +575,7 @@ cgraph_same_body_alias_1 (tree alias, tr
    and cgraph_node (ALIAS) transparently returns cgraph_node (DECL).   */
 
 struct cgraph_node *
-cgraph_same_body_alias (tree alias, tree decl)
+cgraph_same_body_alias (struct cgraph_node *decl_node, tree alias, tree decl)
 {
 #ifndef ASM_OUTPUT_DEF
   /* If aliases aren't supported by the assembler, fail.  */
@@ -584,7 +584,7 @@ cgraph_same_body_alias (tree alias, tree
 
   /*gcc_assert (!assembler_name_hash);*/
 
-  return cgraph_same_body_alias_1 (alias, decl);
+  return cgraph_same_body_alias_1 (decl_node, alias, decl);
 }
 
 /* Add thunk alias into callgraph.  The alias declaration is ALIAS and it
@@ -592,7 +592,8 @@ cgraph_same_body_alias (tree alias, tree
    See comments in thunk_adjust for detail on the parameters.  */
 
 struct cgraph_node *
-cgraph_add_thunk (tree alias, tree decl, bool this_adjusting,
+cgraph_add_thunk (struct cgraph_node *decl_node, tree alias, tree decl,
+		  bool this_adjusting,
 		  HOST_WIDE_INT fixed_offset, HOST_WIDE_INT virtual_value,
 		  tree virtual_offset,
 		  tree real_alias)
@@ -606,7 +607,7 @@ cgraph_add_thunk (tree alias, tree decl,
       cgraph_remove_node (node);
     }
   
-  node = cgraph_same_body_alias_1 (alias, decl);
+  node = cgraph_same_body_alias_1 (decl_node, alias, decl);
   gcc_assert (node);
   gcc_checking_assert (!virtual_offset
 		       || tree_int_cst_equal (virtual_offset,
@@ -2722,7 +2723,7 @@ cgraph_propagate_frequency (struct cgrap
 	case NODE_FREQUENCY_EXECUTED_ONCE:
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    fprintf (dump_file, "  Called by %s that is executed once\n",
-		     cgraph_node_name (node));
+		     cgraph_node_name (edge->caller));
 	  maybe_unlikely_executed = false;
 	  if (edge->loop_nest)
 	    {
@@ -2735,7 +2736,7 @@ cgraph_propagate_frequency (struct cgrap
 	case NODE_FREQUENCY_NORMAL:
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    fprintf (dump_file, "  Called by %s that is normal or hot\n",
-		     cgraph_node_name (node));
+		     cgraph_node_name (edge->caller));
 	  maybe_unlikely_executed = false;
 	  maybe_executed_once = false;
 	  break;
Index: cp/optimize.c
===================================================================
--- cp/optimize.c	(revision 170364)
+++ cp/optimize.c	(working copy)
@@ -309,7 +309,7 @@ maybe_clone_body (tree fn)
 	  && (!DECL_ONE_ONLY (fns[0])
 	      || (HAVE_COMDAT_GROUP
 		  && DECL_WEAK (fns[0])))
-	  && cgraph_same_body_alias (clone, fns[0]))
+	  && cgraph_same_body_alias (cgraph_node (fns[0]), clone, fns[0]))
 	{
 	  alias = true;
 	  if (DECL_ONE_ONLY (fns[0]))
Index: cp/method.c
===================================================================
--- cp/method.c	(revision 170364)
+++ cp/method.c	(working copy)
@@ -259,7 +259,8 @@ make_alias_for_thunk (tree function)
 
   if (!flag_syntax_only)
     {
-      struct cgraph_node *aliasn = cgraph_same_body_alias (alias, function);
+      struct cgraph_node *aliasn = cgraph_same_body_alias (cgraph_node (function),
+							   alias, function);
       DECL_ASSEMBLER_NAME (function);
       gcc_assert (aliasn != NULL);
     }
@@ -376,7 +377,7 @@ use_thunk (tree thunk_fndecl, bool emit_
   a = nreverse (t);
   DECL_ARGUMENTS (thunk_fndecl) = a;
   TREE_ASM_WRITTEN (thunk_fndecl) = 1;
-  cgraph_add_thunk (thunk_fndecl, function,
+  cgraph_add_thunk (cgraph_node (function), thunk_fndecl, function,
 		    this_adjusting, fixed_offset, virtual_value,
 		    virtual_offset, alias);
 
Index: cp/mangle.c
===================================================================
--- cp/mangle.c	(revision 170364)
+++ cp/mangle.c	(working copy)
@@ -3109,7 +3109,7 @@ mangle_decl (const tree decl)
       if (vague_linkage_p (decl))
 	DECL_WEAK (alias) = 1;
       if (TREE_CODE (decl) == FUNCTION_DECL)
-	cgraph_same_body_alias (alias, decl);
+	cgraph_same_body_alias (cgraph_node (decl), alias, decl);
       else
 	varpool_extra_name_alias (alias, decl);
 #endif
Index: lto-cgraph.c
===================================================================
--- lto-cgraph.c	(revision 170364)
+++ lto-cgraph.c	(working copy)
@@ -551,6 +551,7 @@ lto_output_node (struct lto_simple_outpu
 	      lto_output_fn_decl_index (ob->decl_state, ob->main_stream,
 					alias->thunk.alias);
 	    }
+	  gcc_assert (cgraph_node (alias->thunk.alias) == node);
 	  lto_output_uleb128_stream (ob->main_stream, alias->resolution);
 	  alias = alias->previous;
 	}
@@ -1094,7 +1095,7 @@ input_node (struct lto_file_decl_data *f
 	  tree real_alias;
 	  decl_index = lto_input_uleb128 (ib);
 	  real_alias = lto_file_decl_data_get_fn_decl (file_data, decl_index);
-	  alias = cgraph_same_body_alias (alias_decl, real_alias);
+	  alias = cgraph_same_body_alias (node, alias_decl, real_alias);
 	}
       else
         {
@@ -1103,12 +1104,13 @@ input_node (struct lto_file_decl_data *f
 	  tree real_alias;
 	  decl_index = lto_input_uleb128 (ib);
 	  real_alias = lto_file_decl_data_get_fn_decl (file_data, decl_index);
-	  alias = cgraph_add_thunk (alias_decl, fn_decl, type & 2, fixed_offset,
+	  alias = cgraph_add_thunk (node, alias_decl, fn_decl, type & 2, fixed_offset,
 				    virtual_value,
 				    (type & 4) ? size_int (virtual_value) : NULL_TREE,
 				    real_alias);
 	}
-       alias->resolution = (enum ld_plugin_symbol_resolution)lto_input_uleb128 (ib);
+      gcc_assert (alias);
+      alias->resolution = (enum ld_plugin_symbol_resolution)lto_input_uleb128 (ib);
     }
   return node;
 }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: PR lto/47497 (undefined symbol when building soplex)
  2011-03-02 10:07 PR lto/47497 (undefined symbol when building soplex) Jan Hubicka
@ 2011-03-02 10:36 ` Richard Guenther
  2011-03-02 13:23   ` Jan Hubicka
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Guenther @ 2011-03-02 10:36 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

On Wed, Mar 2, 2011 at 11:07 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>
> Hi,
> the problem is with thunks referring thunks or aliases.
>
> lto-symtab is wrong here and when moving thunks&aliases associated with one
> node to the other node, it overwrites thunk.alias by the destination node. It
> consequently turns thunk referring another thunk into thunk referring the
> functoin itself.
>
> I fixed this by adding extra loop setting alias decl to prevailing decl. Richi,
> perhaps there is better place to put this?  I don't like it being in the loop
> redirecting thunks&aliases from one node to another because
>
>  1) I think that loop is not quite correct.  When one function is prevailed by
> another, local static thunk from the first function should not be redirected to
> another.  That happens correctly and is harmless (we end up with dead thunk)
>  2) It may happen that thunks get prevailed other way than nodes they are
> aliased with.  We use comdat groups that prevents this from happening, but I
> would rather not 100% rely on this on all targets since not all targets
> implements comdat groups.  So I think it is more robust to simply merge the
> decls in alias field like we merge other decls.
>
> Fixing this problem cause different problem with streaming.  When we have alias
> A referring function F and alias B referring alias A and we are unlucky with
> prevailing and other things, we might end up streaming alias B before alias A.
> This leads us to call cgraph_same_body_alias on decl of A before A is added to
> cgraph as an alias.  Consequently cgraph_same_body_alias does nothing later
> when we try to create alias A itself, because the node already exists.
>
> This patch fixes it by adding node pointer into the cgraph_same_body_alias and
> cgraph_add_thunk so the thunks&aliases can be added in random order w/o
> problems as long as the function they are associated with is already in cgraph.
>
> Bootstraped/regtested x86_64-linux, also tested with Mozilla and qt build.

Comments inline

> Honza
>        PR lto/47497
>        * lto-symtab.c (lto_cgraph_replace_node): Do not set thunk.alias.
>        (lto_symtab_merge_cgraph_nodes_1): Update thunk.alias pointers here.
>        * cgraph.h (cgraph_same_body_alias, cgraph_add_thunk): Add node pointers.
>        * cgraph.c (cgraph_same_body_alias_1, cgraph_same_body_alias,
>        cgraph_add_thunk): Add node pointers.
>        * lto-cgraph.c (lto_output_node): Verify that thunks&aliases are
>        associated to right node.
>        (input_node): Update use of cgraph_same_body_alias
>        and cgraph_add_thunk.
>
>        * optimize.c (maybe_clone_body): Update call of cgraph_same_body_alias
>        and cgraph_add_thunk.
>        * method.c (make_alias_for_thunk, use_thunk): Likewise.
>        * mangle.c (mangle_decl): Likewise.
> Index: lto-symtab.c
> ===================================================================
> --- lto-symtab.c        (revision 170364)
> +++ lto-symtab.c        (working copy)
> @@ -273,7 +273,6 @@ lto_cgraph_replace_node (struct cgraph_n
>          last = alias;
>          gcc_assert (alias->same_body_alias);
>          alias->same_body = prevailing_node;
> -         alias->thunk.alias = prevailing_node->decl;
>        }
>       last->next = prevailing_node->same_body;
>       /* Node with aliases is prevailed by alias.
> @@ -828,8 +827,16 @@ lto_symtab_merge_cgraph_nodes_1 (void **
>  void
>  lto_symtab_merge_cgraph_nodes (void)
>  {
> +  struct cgraph_node *node, *alias, *next;
>   lto_symtab_maybe_init_hash_table ();
>   htab_traverse (lto_symtab_identifiers, lto_symtab_merge_cgraph_nodes_1, NULL);
> +
> +  for (node = cgraph_nodes; node; node = node->next)
> +    for (alias = node->same_body; alias; alias = next)
> +      {
> +       next = alias->next;
> +       alias->thunk.alias = lto_symtab_prevailing_decl (alias->thunk.alias);
> +      }

Not a very nice place but I guess ok.

>  }
>
>  /* Given the decl DECL, return the prevailing decl with the same name. */
> Index: cgraph.h
> ===================================================================
> --- cgraph.h    (revision 170364)
> +++ cgraph.h    (working copy)
> @@ -559,8 +559,8 @@ struct cgraph_indirect_call_info *cgraph
>  struct cgraph_node * cgraph_get_node (const_tree);
>  struct cgraph_node * cgraph_get_node_or_alias (const_tree);
>  struct cgraph_node * cgraph_node (tree);
> -struct cgraph_node * cgraph_same_body_alias (tree, tree);
> -struct cgraph_node * cgraph_add_thunk (tree, tree, bool, HOST_WIDE_INT,
> +struct cgraph_node * cgraph_same_body_alias (struct cgraph_node *, tree, tree);
> +struct cgraph_node * cgraph_add_thunk (struct cgraph_node *, tree, tree, bool, HOST_WIDE_INT,
>                                       HOST_WIDE_INT, tree, tree);
>  void cgraph_remove_same_body_alias (struct cgraph_node *);
>  struct cgraph_node *cgraph_node_for_asm (tree);
> Index: cgraph.c
> ===================================================================
> --- cgraph.c    (revision 170364)
> +++ cgraph.c    (working copy)
> @@ -536,16 +536,16 @@ cgraph_node (tree decl)
>   return node;
>  }
>
> -/* Mark ALIAS as an alias to DECL.  */
> +/* Mark ALIAS as an alias to DECL.  DECL_NODE is cgraph node representing
> +   the function body is associated with (not neccesarily cgraph_node (DECL).  */
>
>  static struct cgraph_node *
> -cgraph_same_body_alias_1 (tree alias, tree decl)
> +cgraph_same_body_alias_1 (struct cgraph_node *decl_node, tree alias, tree decl)
>  {
> -  struct cgraph_node key, *alias_node, *decl_node, **slot;
> +  struct cgraph_node key, *alias_node, **slot;
>
>   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
>   gcc_assert (TREE_CODE (alias) == FUNCTION_DECL);
> -  decl_node = cgraph_node (decl);
>
>   key.decl = alias;
>
> @@ -575,7 +575,7 @@ cgraph_same_body_alias_1 (tree alias, tr
>    and cgraph_node (ALIAS) transparently returns cgraph_node (DECL).   */
>
>  struct cgraph_node *
> -cgraph_same_body_alias (tree alias, tree decl)
> +cgraph_same_body_alias (struct cgraph_node *decl_node, tree alias, tree decl)
>  {
>  #ifndef ASM_OUTPUT_DEF
>   /* If aliases aren't supported by the assembler, fail.  */
> @@ -584,7 +584,7 @@ cgraph_same_body_alias (tree alias, tree
>
>   /*gcc_assert (!assembler_name_hash);*/
>
> -  return cgraph_same_body_alias_1 (alias, decl);
> +  return cgraph_same_body_alias_1 (decl_node, alias, decl);
>  }
>
>  /* Add thunk alias into callgraph.  The alias declaration is ALIAS and it
> @@ -592,7 +592,8 @@ cgraph_same_body_alias (tree alias, tree
>    See comments in thunk_adjust for detail on the parameters.  */
>
>  struct cgraph_node *
> -cgraph_add_thunk (tree alias, tree decl, bool this_adjusting,
> +cgraph_add_thunk (struct cgraph_node *decl_node, tree alias, tree decl,
> +                 bool this_adjusting,
>                  HOST_WIDE_INT fixed_offset, HOST_WIDE_INT virtual_value,
>                  tree virtual_offset,
>                  tree real_alias)
> @@ -606,7 +607,7 @@ cgraph_add_thunk (tree alias, tree decl,
>       cgraph_remove_node (node);
>     }
>
> -  node = cgraph_same_body_alias_1 (alias, decl);
> +  node = cgraph_same_body_alias_1 (decl_node, alias, decl);
>   gcc_assert (node);
>   gcc_checking_assert (!virtual_offset
>                       || tree_int_cst_equal (virtual_offset,
> @@ -2722,7 +2723,7 @@ cgraph_propagate_frequency (struct cgrap
>        case NODE_FREQUENCY_EXECUTED_ONCE:
>          if (dump_file && (dump_flags & TDF_DETAILS))
>            fprintf (dump_file, "  Called by %s that is executed once\n",
> -                    cgraph_node_name (node));
> +                    cgraph_node_name (edge->caller));

unrelated change?

>          maybe_unlikely_executed = false;
>          if (edge->loop_nest)
>            {
> @@ -2735,7 +2736,7 @@ cgraph_propagate_frequency (struct cgrap
>        case NODE_FREQUENCY_NORMAL:
>          if (dump_file && (dump_flags & TDF_DETAILS))
>            fprintf (dump_file, "  Called by %s that is normal or hot\n",
> -                    cgraph_node_name (node));
> +                    cgraph_node_name (edge->caller));

likewise.

>          maybe_unlikely_executed = false;
>          maybe_executed_once = false;
>          break;
> Index: cp/optimize.c
> ===================================================================
> --- cp/optimize.c       (revision 170364)
> +++ cp/optimize.c       (working copy)
> @@ -309,7 +309,7 @@ maybe_clone_body (tree fn)
>          && (!DECL_ONE_ONLY (fns[0])
>              || (HAVE_COMDAT_GROUP
>                  && DECL_WEAK (fns[0])))
> -         && cgraph_same_body_alias (clone, fns[0]))
> +         && cgraph_same_body_alias (cgraph_node (fns[0]), clone, fns[0]))

Not cgraph_get_node?

>        {
>          alias = true;
>          if (DECL_ONE_ONLY (fns[0]))
> Index: cp/method.c
> ===================================================================
> --- cp/method.c (revision 170364)
> +++ cp/method.c (working copy)
> @@ -259,7 +259,8 @@ make_alias_for_thunk (tree function)
>
>   if (!flag_syntax_only)
>     {
> -      struct cgraph_node *aliasn = cgraph_same_body_alias (alias, function);
> +      struct cgraph_node *aliasn = cgraph_same_body_alias (cgraph_node (function),
> +                                                          alias, function);

Likewise.

>       DECL_ASSEMBLER_NAME (function);
>       gcc_assert (aliasn != NULL);
>     }
> @@ -376,7 +377,7 @@ use_thunk (tree thunk_fndecl, bool emit_
>   a = nreverse (t);
>   DECL_ARGUMENTS (thunk_fndecl) = a;
>   TREE_ASM_WRITTEN (thunk_fndecl) = 1;
> -  cgraph_add_thunk (thunk_fndecl, function,
> +  cgraph_add_thunk (cgraph_node (function), thunk_fndecl, function,

Likewise.

>                    this_adjusting, fixed_offset, virtual_value,
>                    virtual_offset, alias);
>
> Index: cp/mangle.c
> ===================================================================
> --- cp/mangle.c (revision 170364)
> +++ cp/mangle.c (working copy)
> @@ -3109,7 +3109,7 @@ mangle_decl (const tree decl)
>       if (vague_linkage_p (decl))
>        DECL_WEAK (alias) = 1;
>       if (TREE_CODE (decl) == FUNCTION_DECL)
> -       cgraph_same_body_alias (alias, decl);
> +       cgraph_same_body_alias (cgraph_node (decl), alias, decl);

Likewise.

>       else
>        varpool_extra_name_alias (alias, decl);
>  #endif
> Index: lto-cgraph.c
> ===================================================================
> --- lto-cgraph.c        (revision 170364)
> +++ lto-cgraph.c        (working copy)
> @@ -551,6 +551,7 @@ lto_output_node (struct lto_simple_outpu
>              lto_output_fn_decl_index (ob->decl_state, ob->main_stream,
>                                        alias->thunk.alias);
>            }
> +         gcc_assert (cgraph_node (alias->thunk.alias) == node);

Likewise.

The patch looks ok with cgraph_node exchanged for cgraph_get_node
(and re-testing).

Thanks,
Richard.


>          lto_output_uleb128_stream (ob->main_stream, alias->resolution);
>          alias = alias->previous;
>        }
> @@ -1094,7 +1095,7 @@ input_node (struct lto_file_decl_data *f
>          tree real_alias;
>          decl_index = lto_input_uleb128 (ib);
>          real_alias = lto_file_decl_data_get_fn_decl (file_data, decl_index);
> -         alias = cgraph_same_body_alias (alias_decl, real_alias);
> +         alias = cgraph_same_body_alias (node, alias_decl, real_alias);
>        }
>       else
>         {
> @@ -1103,12 +1104,13 @@ input_node (struct lto_file_decl_data *f
>          tree real_alias;
>          decl_index = lto_input_uleb128 (ib);
>          real_alias = lto_file_decl_data_get_fn_decl (file_data, decl_index);
> -         alias = cgraph_add_thunk (alias_decl, fn_decl, type & 2, fixed_offset,
> +         alias = cgraph_add_thunk (node, alias_decl, fn_decl, type & 2, fixed_offset,
>                                    virtual_value,
>                                    (type & 4) ? size_int (virtual_value) : NULL_TREE,
>                                    real_alias);
>        }
> -       alias->resolution = (enum ld_plugin_symbol_resolution)lto_input_uleb128 (ib);
> +      gcc_assert (alias);
> +      alias->resolution = (enum ld_plugin_symbol_resolution)lto_input_uleb128 (ib);
>     }
>   return node;
>  }
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: PR lto/47497 (undefined symbol when building soplex)
  2011-03-02 10:36 ` Richard Guenther
@ 2011-03-02 13:23   ` Jan Hubicka
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Hubicka @ 2011-03-02 13:23 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Jan Hubicka, gcc-patches

> > @@ -2722,7 +2723,7 @@ cgraph_propagate_frequency (struct cgrap
> >        case NODE_FREQUENCY_EXECUTED_ONCE:
> >          if (dump_file && (dump_flags & TDF_DETAILS))
> >            fprintf (dump_file, "  Called by %s that is executed once\n",
> > -                    cgraph_node_name (node));
> > +                    cgraph_node_name (edge->caller));
> 
> unrelated change?

Uh, yes.  Bugfix, but I will commit it independently.
> > Index: cp/optimize.c
> > ===================================================================
> > --- cp/optimize.c       (revision 170364)
> > +++ cp/optimize.c       (working copy)
> > @@ -309,7 +309,7 @@ maybe_clone_body (tree fn)
> >          && (!DECL_ONE_ONLY (fns[0])
> >              || (HAVE_COMDAT_GROUP
> >                  && DECL_WEAK (fns[0])))
> > -         && cgraph_same_body_alias (clone, fns[0]))
> > +         && cgraph_same_body_alias (cgraph_node (fns[0]), clone, fns[0]))
> 
> Not cgraph_get_node?

Well, cgraph was originally designed with cgraph_node being the way to get
from decl to callgraph node.

Since callgraph node no longer is just an vertex of graph, the fact that
cgraph_node lazilly allocates the nodes on demand no longer makes sense and I
will change that for 4.7 requiring explicit cgraph_create_node calls.  (to
avoid funny bugs like this one).

At the moment I am not quite sure if using cgraph_get_node at random places makes more
sense, but I will update the patch and re-test.
(I am not even sure if the main function is already in cgraph at time frontend adds thunks
to it).

Honza

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-03-02 13:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-02 10:07 PR lto/47497 (undefined symbol when building soplex) Jan Hubicka
2011-03-02 10:36 ` Richard Guenther
2011-03-02 13:23   ` Jan Hubicka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).