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

Alexey Bataev a.bataev at hotmail.com
Thu Jun 25 23:03:40 PDT 2015


Samuel,
Thanks for the patch!

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.
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.


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

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

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

http://reviews.llvm.org/D10753

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






More information about the cfe-commits mailing list