[pr19844] Add thread local mode to aliases.

[Reid Kleckner]

LGTM

+  void setThreadLocalMode(ThreadLocalMode Val) {
+    assert(Val == NotThreadLocal || getValueID() != Value::FunctionVal);

I guess isa<> isn't available.

+    ThreadLocal = Val;
+  }
+  ThreadLocalMode getThreadLocalMode() const {
+    assert(ThreadLocal == NotThreadLocal || getValueID() !=
Value::FunctionVal);

This would trigger on MyGVThatIsAFunction->isThreadLocal().  It seems a bit
surprising that you can't call the getter on any GlobalValue.  I'd only
assert in the setter.

+    return static_cast<ThreadLocalMode>(ThreadLocal);
+  }


On Wed, May 28, 2014 at 7:55 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> 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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140528/1a3c22a1/attachment.html>