Re: Fix aliasing handling in varpool_remove_node and lto-symtab

[Richard Guenther <rguenther at suse dot de>]

On Tue, 4 May 2010, Jan Hubicka wrote:

> Hi,
> this patch fixes several issues in lto-symtab and varpool alias handling.
> 
> There is unnecesary loop in lto_cgraph_replace_node removing calles of node
> that will be removed anyway.
> 
> lto_varpool_replace_node is bit too active about enforcing that node is analyzed;
> when we have external references only, none of nodes will be analyzed.  This passes
> only because my varpool dumping is currently dumping only analyzed nodes that is
> wrong. We will need to dump all referenced nodes in order to output references.
> 
> Third problem is that the chode ignores existence of aliases.  WHen replacing
> node with aliases, it is neccesary to reling the alias linst to prevailing node,
> also varpool_remove_node should deal correctly with nodes with aliases.
> 
> I also made varpool_remove_node to free the node.  This is good to ensure that
> it does not remain references somewhere.  To conserve memory, I am also killing
> DECL_INTITIAL.  We currently dangle pointers to declarations that was rendered
> useless via merging that I will fix later, but even with this fix, we will want
> to kill initializers of variables we find unused that I will do in one of
> followup patches after ipa_ref infrastructure is at place.
> 
> Bootstrapped/regtsted x86_64-linux, OK?

Ok.

Thanks,
Richard.

> Honza
> 
> 	* lto-symtab.c (lto_cgraph_replace_node): Do not remove edges;
> 	node will be removed anyway.
> 	(lto_varpool_replace_node): Allow also unanalyzed nodes;
> 	relink aliases of node into prevailing node.
> 	* varpool.c (varpool_remove_node): Remove aliases properly;
> 	when removing node, remove all its aliases too; remove DECL_INITIAL
> 	of removed node; ggc_free the varpool node.
> Index: lto-symtab.c
> ===================================================================
> *** lto-symtab.c	(revision 158944)
> --- lto-symtab.c	(working copy)
> *************** lto_cgraph_replace_node (struct cgraph_n
> *** 215,229 ****
>         cgraph_redirect_edge_callee (e, prevailing_node);
>       }
>   
> -   /* There are not supposed to be any outgoing edges from a node we
> -      replace.  Still this can happen for multiple instances of weak
> -      functions.  */
> -   for (e = node->callees; e; e = next)
> -     {
> -       next = e->next_callee;
> -       cgraph_remove_edge (e);
> -     }
> - 
>     if (node->same_body)
>       {
>         struct cgraph_node *alias;
> --- 215,220 ----
> *************** lto_varpool_replace_node (struct varpool
> *** 257,265 ****
>     /* Merge node flags.  */
>     if (vnode->needed)
>       {
> !       gcc_assert (prevailing_node->analyzed);
>         varpool_mark_needed_node (prevailing_node);
>       }
>     gcc_assert (!vnode->finalized || prevailing_node->finalized);
>     gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
>   
> --- 248,275 ----
>     /* Merge node flags.  */
>     if (vnode->needed)
>       {
> !       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);
>   
> Index: varpool.c
> ===================================================================
> *** varpool.c	(revision 158941)
> --- varpool.c	(working copy)
> *************** varpool_remove_node (struct varpool_node
> *** 157,170 ****
>     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 if (node->next)
>       {
> !       gcc_assert (varpool_nodes == node);
> !       varpool_nodes = node->next;
>       }
>     if (varpool_first_unanalyzed_node == node)
>       varpool_first_unanalyzed_node = node->next_needed;
> --- 157,181 ----
>     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)
> ! 	{
> !           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;
> *************** varpool_remove_node (struct varpool_node
> *** 182,188 ****
>         gcc_assert (varpool_nodes_queue == node);
>         varpool_nodes_queue = node->next_needed;
>       }
> !   node->decl = NULL;
>   }
>   
>   /* Dump given cgraph node.  */
> --- 193,201 ----
>         gcc_assert (varpool_nodes_queue == node);
>         varpool_nodes_queue = node->next_needed;
>       }
> !   if (DECL_INITIAL (node->decl))
> !     DECL_INITIAL (node->decl) = error_mark_node;
> !   ggc_free (node);
>   }
>   
>   /* Dump given cgraph node.  */
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex