[PATCH] [OpenMP] Add TLS-based implementation for threadprivate directive

[Samuel Antao]

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/