[pr19844] Add thread local mode to aliases.

[Rafael Espíndola]

On 27 May 2014 16:43, Reid Kleckner <rnk at google.com> wrote:
> This adds some oversharing that we got rid of with the GlobalObject.
> Functions inherit from GlobalValue, but they don't have a thread local mode.
> Is that worth worrying about?

Not sure. It seems a case is harder to get wrong than the section and
alignment were. To fix this we would need to bring in Nicks idea of
using mixins.

> This also allows us to have thread local modes on aliases to functions,
> which is awkward.

Yes, but this one seems a fundamental issue with having a generic
ConstantExpr in a GlobalAlias. At the llvm level we cannot in general
know if an alias in an alias to a function.

> Do you think it's OK to add asserts to setThreadLocalMode() to make sure we
> don't get into these bad states?  FWIW I don't think it's worth having a
> GlobalVarAlias and GlobalFuncAlias.

It does'n seem to add much. I would be very opposed to a
GlobalVarAlias and GlobalFuncAlais. If we are going to constraint what
alias point to, lets just say it is a GlobalObject and offset and then
it becomes easy to check that the aliasee matches whatever constraints
we have.

> I'm OK with making frontends responsible for putting the thread local mode
> on any aliases produced.  Aliases to thread local GVs weren't very useful
> before.
>
> +variable). Not all targets support thread-local variables. Optionally, a
> +TLS model may be specified:
> +
> +``localdynamic``
> +    For variables that are only used within the current shared library.
> +``initialexec``
> +    For variables in modules that will not be loaded dynamically.
> +``localexec``
> +    For variables defined in the executable and only used within it.
> +
>
> I noticed this should probably describe the behavior when no model is
> specified, i.e. GeneralDynamic.

Good point. Added.

> +The models correspond to the ELF TLS models; see `ELF Handling For
>
> diff --git a/test/CodeGen/X86/2008-03-12-ThreadLocalAlias.ll
> b/test/CodeGen/X86/2008-03-12-ThreadLocalAlias.ll
> index e64375a..a0106d7 100644
> --- a/test/CodeGen/X86/2008-03-12-ThreadLocalAlias.ll
> +++ b/test/CodeGen/X86/2008-03-12-ThreadLocalAlias.ll
> @@ -8,7 +8,7 @@ target triple = "i386-pc-linux-gnu"
>  @__resp = thread_local global %struct.__res_state* @_res ;
> <%struct.__res_state**> [#uses=1]
>  @_res = global %struct.__res_state zeroinitializer, section ".bss" ;
> <%struct.__res_state*> [#uses=1]
>
> - at __libc_resp = hidden alias %struct.__res_state** @__resp ;
> <%struct.__res_state**> [#uses=2]
> + at __libc_resp = hidden thread_local alias %struct.__res_state** @__resp ;
> <%struct.__res_state**> [#uses=2]
>
> This test case looks like it was reduced from a real libc.  Do you have a
> clang-side patch ready to go for when this lands?

No, we now actually match gcc's behaviour of requiring the model on the alias.

The full declaration in glibc is

extern __thread struct __res_state *__libc_resp
  __attribute__ ((alias ("__resp"))) attribute_hidden;

There will be some changes needed to clang to fix pr19844 and PR19843,
but I haven't started coding those yet.

A Patch with the documentation update is attached.

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 26064 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140528/2e72d4dc/attachment.bin>