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/



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

Reply via email to