Hello Michael,
Thanks for review. I've renamed test class from Class to TestClass in
revision 210305.
Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
5 Июнь 2014 г. 23:26:38, Michael Wong писал:
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):
static Class LocalClass(y); // expected-error {{variable with local
storage in initial value of threadprivate variable}}
#pragma omp threadprivate(LocalClass)
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.
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.
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.
On Wed, May 28, 2014 at 3:40 AM, Alexey Bataev <[email protected]
<mailto:[email protected]>> wrote:
Author: abataev
Date: Wed May 28 02:40:25 2014
New Revision: 209716
URL: http://llvm.org/viewvc/llvm-project?rev=209716&view=rev
Log:
[OPENMP] Additional checking for local vars in initial values for
threadprivate vars
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/threadprivate_messages.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=209716&r1=209715&r2=209716&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May
28 02:40:25 2014
@@ -6956,6 +6956,8 @@ def err_omp_linear_expected_int_or_ptr :
def warn_omp_linear_step_zero : Warning<
"zero linear step (%0 %select{|and other variables in clause
}1should probably be const)">,
InGroup<OpenMPClauses>;
+def err_omp_local_var_in_threadprivate_init : Error<
+ "variable with local storage in initial value of threadprivate
variable">;
} // end of OpenMP category
let CategoryName = "Related Result Type Issue" in {
Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=209716&r1=209715&r2=209716&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed May 28 02:40:25 2014
@@ -553,6 +553,35 @@ Sema::ActOnOpenMPThreadprivateDirective(
return DeclGroupPtrTy();
}
+namespace {
+class LocalVarRefChecker : public
ConstStmtVisitor<LocalVarRefChecker, bool> {
+ Sema &SemaRef;
+
+public:
+ bool VisitDeclRefExpr(const DeclRefExpr *E) {
+ if (auto VD = dyn_cast<VarDecl>(E->getDecl())) {
+ if (VD->hasLocalStorage()) {
+ SemaRef.Diag(E->getLocStart(),
+ diag::err_omp_local_var_in_threadprivate_init)
+ << E->getSourceRange();
+ SemaRef.Diag(VD->getLocation(), diag::note_defined_here)
+ << VD << VD->getSourceRange();
+ return true;
+ }
+ }
+ return false;
+ }
+ bool VisitStmt(const Stmt *S) {
+ for (auto Child : S->children()) {
+ if (Child && Visit(Child))
+ return true;
+ }
+ return false;
+ }
+ LocalVarRefChecker(Sema &SemaRef) : SemaRef(SemaRef) {}
+};
+} // namespace
+
OMPThreadPrivateDecl *
Sema::CheckOMPThreadPrivateDecl(SourceLocation Loc, ArrayRef<Expr
*> VarList) {
SmallVector<Expr *, 8> Vars;
@@ -592,6 +621,13 @@ Sema::CheckOMPThreadPrivateDecl(SourceLo
continue;
}
+ // Check if initial value of threadprivate variable reference
variable with
+ // local storage (it is not supported by runtime).
+ if (auto Init = VD->getAnyInitializer()) {
+ LocalVarRefChecker Checker(*this);
+ if (Checker.Visit(Init)) continue;
+ }
+
Vars.push_back(RefExpr);
DSAStack->addDSA(VD, DE, OMPC_threadprivate);
}
Modified: cfe/trunk/test/OpenMP/threadprivate_messages.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/threadprivate_messages.cpp?rev=209716&r1=209715&r2=209716&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/threadprivate_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/threadprivate_messages.cpp Wed May 28
02:40:25 2014
@@ -108,10 +108,12 @@ int o; // expected-note {{candidate foun
int main(int argc, char **argv) { // expected-note {{'argc'
defined here}}
- int x, y = argc; // expected-note {{'y' defined here}}
+ int x, y = argc; // expected-note 2 {{'y' defined here}}
static double d1;
static double d2;
static double d3; // expected-note {{'d3' defined here}}
+ static Class LocalClass(y); // expected-error {{variable with
local storage in initial value of threadprivate variable}}
+#pragma omp threadprivate(LocalClass)
d.a = a;
d2++;
_______________________________________________
cfe-commits mailing list
[email protected] <mailto:[email protected]>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
