<div dir="ltr"><div>Hi, nice patch. Fairly simple, just adding a check for the following case. This use case, that of initializing a static class with a local variable and then making it threadprivate has always been problematic in OpenMP (i.e. not clearly defined although I think where you are going is right):<br>
<table cellpadding="0" cellspacing="0"><tbody><tr><td class="">static Class LocalClass(y); // expected-error {{variable with local storage in initial value of threadprivate variable}}</td>
</tr>





<tr>
<td class="" id="l116"></td><td class=""><br></td>
<td class="">#pragma omp threadprivate(LocalClass)</td></tr></tbody></table><br> My comment is more about this situation. We had a call discussion on this amoong the OpenMP clang review group (weekly). Ajay Jayaraj of TI started the thought that when Threadprivate Static vars that are initialized with initial local values, the precise method of initialization is ambiguously defined by OpenMP. Hal Finkel added that there has always been question in the OpenMP spec on when do you call the constructor. Normally initializing a static variable is done with an atomic flag or some form of double checked locking on the static var when you are in a parallel context to ensure a once only initialization (similarly for a functional local static). Here we also need to have it initialized once for every thread  (threadprivate) and have it last the duration of the program. There is now question as to when this should happen (any time before first use) and what constructor is called for each thread, likely the Default constructor.<br>
<br>This is one of those C++'ism that OpenMP fails to fully specify and it should be brought back to the OpenMP committee as a defect. I have been collecting these cases where OpenMP does not serve C++ particularly well, and this is one of several that we can improve upon.<br>
<br></div>One other very nick-picky comment. The test cases all use Class as a class to hold an abstraction for  a test class. Even as a C++ Std committee member, I find having a user name entity that uses the same name as a C++ abstraction  (class) confusing. I mean, was this deliberate (it was), or was this an accidental typo that somehow worked? Why not stay far away from that kind of doubt and umambiguously call it TestClass or TempClass as you have done with another class. Your choice. <br>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 28, 2014 at 3:40 AM, Alexey Bataev <span dir="ltr"><<a href="mailto:a.bataev@hotmail.com" target="_blank">a.bataev@hotmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: abataev<br>
Date: Wed May 28 02:40:25 2014<br>
New Revision: 209716<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=209716&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=209716&view=rev</a><br>
Log:<br>
[OPENMP] Additional checking for local vars in initial values for threadprivate vars<br>
<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
    cfe/trunk/lib/Sema/SemaOpenMP.cpp<br>
    cfe/trunk/test/OpenMP/threadprivate_messages.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=209716&r1=209715&r2=209716&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=209716&r1=209715&r2=209716&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May 28 02:40:25 2014<br>
@@ -6956,6 +6956,8 @@ def err_omp_linear_expected_int_or_ptr :<br>
 def warn_omp_linear_step_zero : Warning<<br>
   "zero linear step (%0 %select{|and other variables in clause }1should probably be const)">,<br>
   InGroup<OpenMPClauses>;<br>
+def err_omp_local_var_in_threadprivate_init : Error<<br>
+  "variable with local storage in initial value of threadprivate variable">;<br>
 } // end of OpenMP category<br>
<br>
 let CategoryName = "Related Result Type Issue" in {<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=209716&r1=209715&r2=209716&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=209716&r1=209715&r2=209716&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed May 28 02:40:25 2014<br>
@@ -553,6 +553,35 @@ Sema::ActOnOpenMPThreadprivateDirective(<br>
   return DeclGroupPtrTy();<br>
 }<br>
<br>
+namespace {<br>
+class LocalVarRefChecker : public ConstStmtVisitor<LocalVarRefChecker, bool> {<br>
+  Sema &SemaRef;<br>
+<br>
+public:<br>
+  bool VisitDeclRefExpr(const DeclRefExpr *E) {<br>
+    if (auto VD = dyn_cast<VarDecl>(E->getDecl())) {<br>
+      if (VD->hasLocalStorage()) {<br>
+        SemaRef.Diag(E->getLocStart(),<br>
+                     diag::err_omp_local_var_in_threadprivate_init)<br>
+            << E->getSourceRange();<br>
+        SemaRef.Diag(VD->getLocation(), diag::note_defined_here)<br>
+            << VD << VD->getSourceRange();<br>
+        return true;<br>
+      }<br>
+    }<br>
+    return false;<br>
+  }<br>
+  bool VisitStmt(const Stmt *S) {<br>
+    for (auto Child : S->children()) {<br>
+      if (Child && Visit(Child))<br>
+        return true;<br>
+    }<br>
+    return false;<br>
+  }<br>
+  LocalVarRefChecker(Sema &SemaRef) : SemaRef(SemaRef) {}<br>
+};<br>
+} // namespace<br>
+<br>
 OMPThreadPrivateDecl *<br>
 Sema::CheckOMPThreadPrivateDecl(SourceLocation Loc, ArrayRef<Expr *> VarList) {<br>
   SmallVector<Expr *, 8> Vars;<br>
@@ -592,6 +621,13 @@ Sema::CheckOMPThreadPrivateDecl(SourceLo<br>
       continue;<br>
     }<br>
<br>
+    // Check if initial value of threadprivate variable reference variable with<br>
+    // local storage (it is not supported by runtime).<br>
+    if (auto Init = VD->getAnyInitializer()) {<br>
+      LocalVarRefChecker Checker(*this);<br>
+      if (Checker.Visit(Init)) continue;<br>
+    }<br>
+<br>
     Vars.push_back(RefExpr);<br>
     DSAStack->addDSA(VD, DE, OMPC_threadprivate);<br>
   }<br>
<br>
Modified: cfe/trunk/test/OpenMP/threadprivate_messages.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/threadprivate_messages.cpp?rev=209716&r1=209715&r2=209716&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/threadprivate_messages.cpp?rev=209716&r1=209715&r2=209716&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/test/OpenMP/threadprivate_messages.cpp (original)<br>
+++ cfe/trunk/test/OpenMP/threadprivate_messages.cpp Wed May 28 02:40:25 2014<br>
@@ -108,10 +108,12 @@ int o; // expected-note {{candidate foun<br>
<br>
 int main(int argc, char **argv) { // expected-note {{'argc' defined here}}<br>
<br>
-  int x, y = argc; // expected-note {{'y' defined here}}<br>
+  int x, y = argc; // expected-note 2 {{'y' defined here}}<br>
   static double d1;<br>
   static double d2;<br>
   static double d3; // expected-note {{'d3' defined here}}<br>
+  static Class LocalClass(y); // expected-error {{variable with local storage in initial value of threadprivate variable}}<br>
+#pragma omp threadprivate(LocalClass)<br>
<br>
   d.a = a;<br>
   d2++;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>