r209716 - [OPENMP] Additional checking for local vars in initial values for threadprivate vars
Alexey Bataev
a.bataev at hotmail.com
Thu Jun 5 20:49:49 PDT 2014
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 <a.bataev at hotmail.com
> <mailto:a.bataev at hotmail.com>> 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
> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
More information about the cfe-commits
mailing list