Relat TLS model merging in lto-symtab

[Jakub Jelinek]

On Wed, Jan 28, 2015 at 08:59:07PM +0100, Jan Hubicka wrote:
> --- lto-symtab.c	(revision 220212)
> +++ lto-symtab.c	(working copy)
> @@ -158,11 +158,44 @@ lto_varpool_replace_node (varpool_node *
>  
>    if (vnode->tls_model != prevailing_node->tls_model)
>      {
> -      error_at (DECL_SOURCE_LOCATION (vnode->decl),
> -		"%qD is defined as %s", vnode->decl, tls_model_names [vnode->tls_model]);
> -      inform (DECL_SOURCE_LOCATION (prevailing_node->decl),
> -	      "previously defined here as %s",
> -	      tls_model_names [prevailing_node->tls_model]);
> +      bool error = false;
> +
> +      /* Non-TLS and TLS never mix together.  Also emulated model is not
> +	 compatible with anything else.  */
> +      if (prevailing_node->tls_model == TLS_MODEL_NONE
> +	  || prevailing_node->tls_model == TLS_MODEL_EMULATED
> +	  || vnode->tls_model == TLS_MODEL_NONE
> +	  || vnode->tls_model == TLS_MODEL_EMULATED)
> +	error = true;
> +      /* Linked is silently supporting transitions

Linker instead of Linked.

> +	 GD -> IE, GD -> LE, LD -> LE, IE -> LE, LD -> IE.

Looking at linker, there is actually no LD -> IE transition, so
linker if there were LD and IE accesses to the same TLS var and no
LE accesses, then the linker would probably keep LD and IE accesses
untouched.  That said, this is before the RTL is generated, and IE is
cheaper, so I think it is fine to also transform LD -> IE in the compiler.

I think you could do similarly a GD -> LD transition, if anything is
accessed using LD model, then by definition it has to be defined in the
current shared library (or executable), not using a definition from
somewhere else, and if something else accesses it globally, it is fine to
access it locally too.  Just make it clear what transitions are done
by the linker and what you do on top of that.
Though, once you add the LD -> IE and GD -> LD transitions the linker
doesn't do, you can just do
prevailing_node->tls_model = MAX (prevailing_node->tls_model, vnode->tls_model);
after checking the NONE/EMULATED cases.

> +	 Do the same transitions and error out on others.  */
> +      else if ((prevailing_node->tls_model == TLS_MODEL_REAL

I'd use TLS_MODEL_GLOBAL_DYNAMIC instead of TLS_MODEL_REAL, it is the
same value, but the former makes it more obvious and will better match
the comment.

> +		|| prevailing_node->tls_model == TLS_MODEL_LOCAL_DYNAMIC)
> +	       && (vnode->tls_model == TLS_MODEL_INITIAL_EXEC
> +		   || vnode->tls_model == TLS_MODEL_LOCAL_EXEC))
> +	prevailing_node->tls_model = vnode->tls_model;
> +      else if ((vnode->tls_model == TLS_MODEL_REAL
> +		|| vnode->tls_model == TLS_MODEL_LOCAL_DYNAMIC)
> +	       && (prevailing_node->tls_model == TLS_MODEL_INITIAL_EXEC
> +		   || prevailing_node->tls_model == TLS_MODEL_LOCAL_EXEC))
> +	;
> +      else if (prevailing_node->tls_model == TLS_MODEL_INITIAL_EXEC
> +	       && vnode->tls_model == TLS_MODEL_LOCAL_EXEC)
> +	prevailing_node->tls_model = vnode->tls_model;
> +      else if (vnode->tls_model == TLS_MODEL_INITIAL_EXEC
> +	       && prevailing_node->tls_model == TLS_MODEL_LOCAL_EXEC)
> +	;
> +      else
> +	error = true;
> +      if (error)
> +	{
> +	  error_at (DECL_SOURCE_LOCATION (vnode->decl),
> +		    "%qD is defined with tls model %s", vnode->decl, tls_model_names [vnode->tls_model]);
> +	  inform (DECL_SOURCE_LOCATION (prevailing_node->decl),
> +		  "previously defined here as %s",
> +		  tls_model_names [prevailing_node->tls_model]);
> +	}
>      }
>    /* Finally remove the replaced node.  */
>    vnode->remove ();

	Jakub