Use resolution info to get rid of weak symbols

Hi,
this patch makes GCC to use resolution info to turn COMDAT and WEAK
symbols into regular symbols based on feedback given by linker plugin.
If resolution says that given symbol is prevailing, it is possible
to turn them into normal symbols, while when resolution says it
is prevailed, it is possible to turn them into external symbols.

Doing so makes rest of the backend to work smoother on them.
We previously did this transformation partly for functions, this patch
just makes it to happen for variables too and implements the second
part (turning the symbol into external definition).

Bootstrapped/regtested x86_64-linux and tested with libreoffice
build.  Will commit it shortly.

	* ipa.c (update_visibility_by_resolution_info): New function.
	(function_and_variable_visibility): Use it.

On 18 May 2014 21:38:47 Jan Hubicka <hubicka@ucw.cz> wrote:

> Hi,
> this patch makes GCC to use resolution info to turn COMDAT and WEAK
> symbols into regular symbols based on feedback given by linker plugin.
> If resolution says that given symbol is prevailing, it is possible
> to turn them into normal symbols, while when resolution says it
> is prevailed, it is possible to turn them into external symbols.
>
> Doing so makes rest of the backend to work smoother on them.
> We previously did this transformation partly for functions, this patch
> just makes it to happen for variables too and implements the second
> part (turning the symbol into external definition).
>
> Bootstrapped/regtested x86_64-linux and tested with libreoffice
> build.  Will commit it shortly.
>
> 	* ipa.c (update_visibility_by_resolution_info): New function.
> 	(function_and_variable_visibility): Use it.
> Index: ipa.c
> ===================================================================
> --- ipa.c	(revision 210522)
> +++ ipa.c	(working copy)
> @@ -978,6 +978,50 @@ can_replace_by_local_alias (symtab_node
>  	  && !symtab_can_be_discarded (node));
>  }
>
> +/* In LTO we can remove COMDAT groups and weak symbols.
> +   Either turn them into normal symbols or external symbol depending on +  
>  resolution info.  */
> +
> +static void
> +update_visibility_by_resolution_info (symtab_node * node)
> +{
> +  bool define;
> +
> +  if (!node->externally_visible
> +      || (!DECL_WEAK (node->decl) && !DECL_ONE_ONLY (node->decl))
> +      || node->resolution == LDPR_UNKNOWN)
> +    return;
> +
> +  define = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> +	    || node->resolution == LDPR_PREVAILING_DEF
> +	    || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
> +
> +  /* The linker decisions ought to agree in the whole group.  */
> +  if (node->same_comdat_group)
> +    for (symtab_node *next = node->same_comdat_group;
> +	 next != node; next = next->same_comdat_group)
> +      gcc_assert (!node->externally_visible

really !node->externally_visible and not !next->externally_visible ?

Above you already returned if !node->externally_visible ...

Thanks,

> +		  || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
> +			        || next->resolution == LDPR_PREVAILING_DEF
> +			        || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
> +
> +  if (node->same_comdat_group)
> +    for (symtab_node *next = node->same_comdat_group;
> +	 next != node; next = next->same_comdat_group)
> +      {
> +	DECL_COMDAT_GROUP (next->decl) = NULL;
> +	DECL_WEAK (next->decl) = false;
> +	if (next->externally_visible
> +	    && !define)
> +	  DECL_EXTERNAL (next->decl) = true;
> +      }
> +  DECL_COMDAT_GROUP (node->decl) = NULL;
> +  DECL_WEAK (node->decl) = false;
> +  if (!define)
> +    DECL_EXTERNAL (node->decl) = true;
> +  symtab_dissolve_same_comdat_group_list (node);
> +}
> +
>  /* Mark visibility of all functions.
>
>     A local function is one whose calls can occur only in the current
> @@ -1116,38 +1160,7 @@ function_and_variable_visibility (bool w
>  	    DECL_EXTERNAL (node->decl) = 1;
>  	}
>
> -      /* If whole comdat group is used only within LTO code, we can 
> dissolve it,
> -	 we handle the unification ourselves.
> -	 We keep COMDAT and weak so visibility out of DSO does not change.
> -	 Later we may bring the symbols static if they are not exported.  */
> -      if (DECL_ONE_ONLY (node->decl)
> -	  && (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> -	      || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP))
> -	{
> -	  symtab_node *next = node;
> -
> -	  if (node->same_comdat_group)
> -	    for (next = node->same_comdat_group;
> -		 next != node;
> -		 next = next->same_comdat_group)
> -	      if (next->externally_visible
> -		  && (next->resolution != LDPR_PREVAILING_DEF_IRONLY
> -		      && next->resolution != LDPR_PREVAILING_DEF_IRONLY_EXP))
> -		break;
> -	  if (node == next)
> -	    {
> -	      if (node->same_comdat_group)
> -	        for (next = node->same_comdat_group;
> -		     next != node;
> -		     next = next->same_comdat_group)
> -		{
> -		  DECL_COMDAT_GROUP (next->decl) = NULL;
> -		  DECL_WEAK (next->decl) = false;
> -		}
> -	      DECL_COMDAT_GROUP (node->decl) = NULL;
> -	      symtab_dissolve_same_comdat_group_list (node);
> -	    }
> -	}
> +      update_visibility_by_resolution_info (node);
>      }
>    FOR_EACH_DEFINED_FUNCTION (node)
>      {
> @@ -1234,6 +1247,7 @@ function_and_variable_visibility (bool w
>  	    symtab_dissolve_same_comdat_group_list (vnode);
>  	  vnode->resolution = LDPR_PREVAILING_DEF_IRONLY;
>  	}
> +      update_visibility_by_resolution_info (vnode);
>      }
>
>    if (dump_file)


Sent with AquaMail for Android
http://www.aqua-mail.com

On Sun, May 18, 2014 at 12:38 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this patch makes GCC to use resolution info to turn COMDAT and WEAK
> symbols into regular symbols based on feedback given by linker plugin.
> If resolution says that given symbol is prevailing, it is possible
> to turn them into normal symbols, while when resolution says it
> is prevailed, it is possible to turn them into external symbols.
>
> Doing so makes rest of the backend to work smoother on them.
> We previously did this transformation partly for functions, this patch
> just makes it to happen for variables too and implements the second
> part (turning the symbol into external definition).
>
> Bootstrapped/regtested x86_64-linux and tested with libreoffice
> build.  Will commit it shortly.
>
>         * ipa.c (update_visibility_by_resolution_info): New function.
>         (function_and_variable_visibility): Use it.

This caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67548