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

[Samuel Antao]

Hi Alexey,

Thanks for the review!

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.

> 2. I don't like the idea that this feature is enabled only for a specific platform. It requires a new class, some complex code changes etc. Instead I prefer to have a driver/frontend option that makes the compiler to switch codegen from cached version to TLS. If you want to set this feature on by default for the PowerPC you can set this option in frontend if current architecture is PowerPC. In this case we don't need an additional CGOpenMPRuntime-based class.


Done! Let me know if the option name okay.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:996
@@ +995,3 @@
+        RetVal = CtorCGF.Builder.CreatePointerCast(
+            TLSGV, CtorCGF.ConvertTypeForMem(CGM.getContext().VoidPtrTy));
+        OrigVal = TLSGV;
----------------
ABataev wrote:
> Use CtorCGF.VoidPtrTy rather than CtorCGF.ConvertTypeForMem(CGM.getContext().VoidPtrTy)
Done!

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1038
@@ +1037,3 @@
+        OrigVal = DtorCGF.Builder.CreatePointerCast(
+            TLSGV, DtorCGF.ConvertTypeForMem(CGM.getContext().VoidPtrTy));
+      } else {
----------------
ABataev wrote:
> DtorCGF.VoidPtrTy
Done!

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:355
@@ +354,3 @@
+  //// variables. It is false by default.
+  bool useTLSForThreadPrivate;
+
----------------
ABataev wrote:
> Should start with an upper case letter, i.e. UseTLSForThreadPrivate
Done!

http://reviews.llvm.org/D10753

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