In http://reviews.llvm.org/D10753#197058, @ABataev wrote:

> > In http://reviews.llvm.org/D10753#195190, @ABataev wrote:
>
> > 
>
> > > 1. Why you don't want to reuse an existing clang infrastructure for TLS 
> > > support? I think it would be much easier and portable solution to reuse 
> > > an existing code, rather than inventing a new one.
>
> > 
>
> > 
>
> > I am afraid I don't understand exactly what you mean by TLS infrastructure. 
> > In my understanding, creating a TLS global variable will make each thread 
> > to have different storage for the variable but will not cause the 
> > Ctors/Dtors to run when a thread is spawn by the OpenMP runtime. Therefore 
> > the OpenMP runtime has to be aware of which Ctors/Dtors to use to a given 
> > variable even if it is declared as TLS so it can run them when it creates a 
> > thread. So from a codegen viewpoint, the step of registering the Dtors and 
> > Ctors  is still required, only the cache creation and look-up can be 
> > skipped. That's the reason why I am reusing the existing code to do the 
> > registration.
>
> > 
>
> > It is possible, however, that I am missing some feature in clang that would 
> > enable the execution of the Cotrs/Dtors when threads are spawn without 
> > intervention of any runtime library. If so, please give me a pointer and I 
> > will update the patch to use that instead.
>
>
> Look at file Decl.cpp, method VarDecl::getTLSKind().
>  I think it would be enough to modify this method to enable codegen for 
> threadprivates just like for TLS:
>
>   case TSCS_unspecified:
>     if (!hasAttr<ThreadAttr>() && 
> (!getASTContext().getLangOpts().OpenMPUseTLS || 
> !hasAttr<OMPThreadPrivateAttr>))
>       return TLS_None;
>     return getASTContext().getLangOpts().isCompatibleWithMSVC(
>                LangOptions::MSVC2015)
>                ? TLS_Dynamic
>                : TLS_Static;
>   
>
> Plus you have to disable cached codegen for threadprivates if 
> getASTContext().getLangOpts().OpenMPUseTLS is true


Alexey, I tried to have the TLS global be created based on the attributes set 
in the way you suggested. The problem I see is that the attribute will be 
checked before SEMA takes places, so it will requires changes in the OpenMP 
threadprivate SEMA and also in the SEMA for thread _local. The latter requires 
understanding that some declaration is an OpenMP thread_local, not a user 
specified thread_local.

To give you an example, thread local variables can only have trivial 
Ctors/Dtors but OpenMP threadprivate variables can also have non-trivial ones. 
So if I add that attribute check in TLSKind() that will cause the thread local 
SEMA to fail.

I also see that the emission of the globals sometimes happen before the 
ThreadPrivate attribute is set. So, doing this would require some adjustments 
there, i.e. have the TLS property of the global being set during the 
threadprivate SEMA.

In my view, taking this approach seems a little more disruptive, but I am happy 
to do that if you think that is the right thing.

Let me know your thoughts.

Thanks!
Samuel


http://reviews.llvm.org/D10753

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to