From 62cec44ca45121206d611722f98b109ecbd6e11d Mon Sep 17 00:00:00 2001 From: Alexey Bataev Date: Tue, 18 Nov 2014 10:14:22 +0000 Subject: [PATCH] [OPENMP] Additional processing of 'omp atomic read' directive. According to OpenMP standard, Section 2.12.6, atomic Construct, '#pragma omp atomic read' is allowed to be used only for expression statements of form 'v = x;', where x and v (as applicable) are both l-value expressions with scalar type. Patch adds checks for it. llvm-svn: 222231 --- clang/include/clang/AST/StmtOpenMP.h | 37 ++++++++-- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 +- clang/lib/AST/Stmt.cpp | 18 ++--- clang/lib/Sema/SemaOpenMP.cpp | 92 ++++++++++++++++++++++-- clang/lib/Serialization/ASTReaderStmt.cpp | 3 + clang/lib/Serialization/ASTWriterStmt.cpp | 3 + clang/test/OpenMP/atomic_messages.c | 65 +++++++++++++++++ clang/test/OpenMP/atomic_messages.cpp | 63 +++++++++++++--- 8 files changed, 259 insertions(+), 26 deletions(-) create mode 100644 clang/test/OpenMP/atomic_messages.c diff --git a/clang/include/clang/AST/StmtOpenMP.h b/clang/include/clang/AST/StmtOpenMP.h index a031d8b..b9402f4 100644 --- a/clang/include/clang/AST/StmtOpenMP.h +++ b/clang/include/clang/AST/StmtOpenMP.h @@ -1439,7 +1439,7 @@ class OMPAtomicDirective : public OMPExecutableDirective { OMPAtomicDirective(SourceLocation StartLoc, SourceLocation EndLoc, unsigned NumClauses) : OMPExecutableDirective(this, OMPAtomicDirectiveClass, OMPD_atomic, - StartLoc, EndLoc, NumClauses, 1) {} + StartLoc, EndLoc, NumClauses, 4) {} /// \brief Build an empty directive. /// @@ -1448,20 +1448,33 @@ class OMPAtomicDirective : public OMPExecutableDirective { explicit OMPAtomicDirective(unsigned NumClauses) : OMPExecutableDirective(this, OMPAtomicDirectiveClass, OMPD_atomic, SourceLocation(), SourceLocation(), NumClauses, - 1) {} + 4) {} + + /// \brief Set 'x' part of the associated expression/statement. + void setX(Expr *X) { *std::next(child_begin()) = X; } + /// \brief Set 'v' part of the associated expression/statement. + void setV(Expr *V) { *std::next(child_begin(), 2) = V; } + /// \brief Set 'expr' part of the associated expression/statement. + void setExpr(Expr *E) { *std::next(child_begin(), 3) = E; } public: - /// \brief Creates directive with a list of \a Clauses. + /// \brief Creates directive with a list of \a Clauses and 'x', 'v' and 'expr' + /// parts of the atomic construct (see Section 2.12.6, atomic Construct, for + /// detailed description of 'x', 'v' and 'expr'). /// /// \param C AST context. /// \param StartLoc Starting location of the directive kind. /// \param EndLoc Ending Location of the directive. /// \param Clauses List of clauses. /// \param AssociatedStmt Statement, associated with the directive. + /// \param X 'x' part of the associated expression/statement. + /// \param V 'v' part of the associated expression/statement. + /// \param Expr 'expr' part of the associated expression/statement. /// static OMPAtomicDirective * Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, - ArrayRef Clauses, Stmt *AssociatedStmt); + ArrayRef Clauses, Stmt *AssociatedStmt, Expr *X, Expr *V, + Expr *E); /// \brief Creates an empty directive with the place for \a NumClauses /// clauses. @@ -1472,6 +1485,22 @@ public: static OMPAtomicDirective *CreateEmpty(const ASTContext &C, unsigned NumClauses, EmptyShell); + /// \brief Get 'x' part of the associated expression/statement. + Expr *getX() { return cast_or_null(*std::next(child_begin())); } + const Expr *getX() const { + return cast_or_null(*std::next(child_begin())); + } + /// \brief Get 'v' part of the associated expression/statement. + Expr *getV() { return cast_or_null(*std::next(child_begin(), 2)); } + const Expr *getV() const { + return cast_or_null(*std::next(child_begin(), 2)); + } + /// \brief Get 'expr' part of the associated expression/statement. + Expr *getExpr() { return cast_or_null(*std::next(child_begin(), 3)); } + const Expr *getExpr() const { + return cast_or_null(*std::next(child_begin(), 3)); + } + static bool classof(const Stmt *T) { return T->getStmtClass() == OMPAtomicDirectiveClass; } diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c175b98..d01a2d6 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7322,7 +7322,9 @@ def err_omp_parallel_reduction_in_task_firstprivate : Error< "argument of a reduction clause of a %0 construct must not appear in a firstprivate clause on a task construct">; def err_omp_atomic_read_not_expression_statement : Error< "the statement for 'atomic read' must be an expression statement of form 'v = x;'," - " where v and x are both l-value expressions with scalar type">; + " where v and x are both lvalue expressions with scalar type">; +def note_omp_atomic_read: Note< + "%select{expected an expression statement|expected built-in assignment operator|expected expression of scalar type|expected lvalue expression}0">; def err_omp_atomic_write_not_expression_statement : Error< "the statement for 'atomic write' must be an expression statement of form 'x = expr;'," " where x is an l-value expression with scalar type">; diff --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp index c323d98..371a7a1 100644 --- a/clang/lib/AST/Stmt.cpp +++ b/clang/lib/AST/Stmt.cpp @@ -1924,19 +1924,21 @@ OMPOrderedDirective *OMPOrderedDirective::CreateEmpty(const ASTContext &C, return new (Mem) OMPOrderedDirective(); } -OMPAtomicDirective *OMPAtomicDirective::Create(const ASTContext &C, - SourceLocation StartLoc, - SourceLocation EndLoc, - ArrayRef Clauses, - Stmt *AssociatedStmt) { +OMPAtomicDirective * +OMPAtomicDirective::Create(const ASTContext &C, SourceLocation StartLoc, + SourceLocation EndLoc, ArrayRef Clauses, + Stmt *AssociatedStmt, Expr *X, Expr *V, Expr *E) { unsigned Size = llvm::RoundUpToAlignment(sizeof(OMPAtomicDirective), llvm::alignOf()); - void *Mem = - C.Allocate(Size + sizeof(OMPClause *) * Clauses.size() + sizeof(Stmt *)); + void *Mem = C.Allocate(Size + sizeof(OMPClause *) * Clauses.size() + + 4 * sizeof(Stmt *)); OMPAtomicDirective *Dir = new (Mem) OMPAtomicDirective(StartLoc, EndLoc, Clauses.size()); Dir->setClauses(Clauses); Dir->setAssociatedStmt(AssociatedStmt); + Dir->setX(X); + Dir->setV(V); + Dir->setExpr(E); return Dir; } @@ -1946,7 +1948,7 @@ OMPAtomicDirective *OMPAtomicDirective::CreateEmpty(const ASTContext &C, unsigned Size = llvm::RoundUpToAlignment(sizeof(OMPAtomicDirective), llvm::alignOf()); void *Mem = - C.Allocate(Size + sizeof(OMPClause *) * NumClauses + sizeof(Stmt *)); + C.Allocate(Size + sizeof(OMPClause *) * NumClauses + 4 * sizeof(Stmt *)); return new (Mem) OMPAtomicDirective(NumClauses); } diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 9bdb092..e4838de 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -3223,13 +3223,94 @@ StmtResult Sema::ActOnOpenMPAtomicDirective(ArrayRef Clauses, } } } + auto Body = CS->getCapturedStmt(); + Expr *X = nullptr; + Expr *V = nullptr; + Expr *E = nullptr; + // OpenMP [2.12.6, atomic Construct] + // In the next expressions: + // * x and v (as applicable) are both l-value expressions with scalar type. + // * During the execution of an atomic region, multiple syntactic + // occurrences of x must designate the same storage location. + // * Neither of v and expr (as applicable) may access the storage location + // designated by x. + // * Neither of x and expr (as applicable) may access the storage location + // designated by v. + // * expr is an expression with scalar type. + // * binop is one of +, *, -, /, &, ^, |, <<, or >>. + // * binop, binop=, ++, and -- are not overloaded operators. + // * The expression x binop expr must be numerically equivalent to x binop + // (expr). This requirement is satisfied if the operators in expr have + // precedence greater than binop, or by using parentheses around expr or + // subexpressions of expr. + // * The expression expr binop x must be numerically equivalent to (expr) + // binop x. This requirement is satisfied if the operators in expr have + // precedence equal to or greater than binop, or by using parentheses around + // expr or subexpressions of expr. + // * For forms that allow multiple occurrences of x, the number of times + // that x is evaluated is unspecified. if (AtomicKind == OMPC_read) { - if (!isa(Body)) { - Diag(Body->getLocStart(), - diag::err_omp_atomic_read_not_expression_statement); - return StmtError(); + enum { + NotAnExpression, + NotAnAssignmentOp, + NotAScalarType, + NotAnLValue, + NoError + } ErrorFound = NoError; + SourceLocation ErrorLoc, NoteLoc; + SourceRange ErrorRange, NoteRange; + // If clause is read: + // v = x; + if (auto AtomicBody = dyn_cast(Body)) { + auto AtomicBinOp = + dyn_cast(AtomicBody->IgnoreParenImpCasts()); + if (AtomicBinOp && AtomicBinOp->getOpcode() == BO_Assign) { + X = AtomicBinOp->getRHS()->IgnoreParenImpCasts(); + V = AtomicBinOp->getLHS()->IgnoreParenImpCasts(); + if ((X->isInstantiationDependent() || X->getType()->isScalarType()) && + (V->isInstantiationDependent() || V->getType()->isScalarType())) { + if (!X->isLValue() || !V->isLValue()) { + auto NotLValueExpr = X->isLValue() ? V : X; + ErrorFound = NotAnLValue; + ErrorLoc = AtomicBinOp->getExprLoc(); + ErrorRange = AtomicBinOp->getSourceRange(); + NoteLoc = NotLValueExpr->getExprLoc(); + NoteRange = NotLValueExpr->getSourceRange(); + } + } else if (!X->isInstantiationDependent() || + !V->isInstantiationDependent()) { + auto NotScalarExpr = + (X->isInstantiationDependent() || X->getType()->isScalarType()) + ? V + : X; + ErrorFound = NotAScalarType; + ErrorLoc = AtomicBinOp->getExprLoc(); + ErrorRange = AtomicBinOp->getSourceRange(); + NoteLoc = NotScalarExpr->getExprLoc(); + NoteRange = NotScalarExpr->getSourceRange(); + } + } else { + ErrorFound = NotAnAssignmentOp; + ErrorLoc = AtomicBody->getExprLoc(); + ErrorRange = AtomicBody->getSourceRange(); + NoteLoc = AtomicBinOp ? AtomicBinOp->getOperatorLoc() + : AtomicBody->getExprLoc(); + NoteRange = AtomicBinOp ? AtomicBinOp->getSourceRange() + : AtomicBody->getSourceRange(); + } + } else { + ErrorFound = NotAnExpression; + NoteLoc = ErrorLoc = Body->getLocStart(); + NoteRange = ErrorRange = SourceRange(NoteLoc, NoteLoc); } + if (ErrorFound != NoError) { + Diag(ErrorLoc, diag::err_omp_atomic_read_not_expression_statement) + << ErrorRange; + Diag(NoteLoc, diag::note_omp_atomic_read) << ErrorFound << NoteRange; + return StmtError(); + } else if (CurContext->isDependentContext()) + V = X = nullptr; } else if (AtomicKind == OMPC_write) { if (!isa(Body)) { Diag(Body->getLocStart(), @@ -3257,7 +3338,8 @@ StmtResult Sema::ActOnOpenMPAtomicDirective(ArrayRef Clauses, getCurFunction()->setHasBranchProtectedScope(); - return OMPAtomicDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt); + return OMPAtomicDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt, + X, V, E); } StmtResult Sema::ActOnOpenMPTargetDirective(ArrayRef Clauses, diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index 0e4d994..e0308d7 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -2123,6 +2123,9 @@ void ASTStmtReader::VisitOMPAtomicDirective(OMPAtomicDirective *D) { // The NumClauses field was read in ReadStmtFromStream. ++Idx; VisitOMPExecutableDirective(D); + D->setX(Reader.ReadSubExpr()); + D->setV(Reader.ReadSubExpr()); + D->setExpr(Reader.ReadSubExpr()); } void ASTStmtReader::VisitOMPTargetDirective(OMPTargetDirective *D) { diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index 309e0a6..38996dd 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -1978,6 +1978,9 @@ void ASTStmtWriter::VisitOMPAtomicDirective(OMPAtomicDirective *D) { VisitStmt(D); Record.push_back(D->getNumClauses()); VisitOMPExecutableDirective(D); + Writer.AddStmt(D->getX()); + Writer.AddStmt(D->getV()); + Writer.AddStmt(D->getExpr()); Code = serialization::STMT_OMP_ATOMIC_DIRECTIVE; } diff --git a/clang/test/OpenMP/atomic_messages.c b/clang/test/OpenMP/atomic_messages.c new file mode 100644 index 0000000..d2d3308 --- /dev/null +++ b/clang/test/OpenMP/atomic_messages.c @@ -0,0 +1,65 @@ +// RUN: %clang_cc1 -verify -fopenmp=libiomp5 -ferror-limit 100 %s + +int foo() { +L1: + foo(); +#pragma omp atomic + // expected-error@+1 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an l-value expression with scalar type}} + { + foo(); + goto L1; // expected-error {{use of undeclared label 'L1'}} + } + goto L2; // expected-error {{use of undeclared label 'L2'}} +#pragma omp atomic + // expected-error@+1 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an l-value expression with scalar type}} + { + foo(); + L2: + foo(); + } + + return 0; +} + +struct S { + int a; +}; + +int readint() { + int a = 0, b = 0; +// Test for atomic read +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected an expression statement}} + ; +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} + foo(); +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} + a += b; +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected lvalue expression}} + a = 0; +#pragma omp atomic read + a = b; + // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'read' clause}} +#pragma omp atomic read read + a = b; + + return 0; +} + +int readS() { + struct S a, b; + // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'read' clause}} +#pragma omp atomic read read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected expression of scalar type}} + a = b; + + return a.a; +} diff --git a/clang/test/OpenMP/atomic_messages.cpp b/clang/test/OpenMP/atomic_messages.cpp index b53133f..3965a77 100644 --- a/clang/test/OpenMP/atomic_messages.cpp +++ b/clang/test/OpenMP/atomic_messages.cpp @@ -21,31 +21,78 @@ L1: return 0; } +struct S { + int a; + S &operator=(int v) { + a = v; + return *this; + } + S &operator+=(const S &s) { + a += s.a; + return *this; + } +}; + template T read() { - T a, b = 0; + T a = T(), b = T(); // Test for atomic read #pragma omp atomic read - // expected-error@+1 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both l-value expressions with scalar type}} + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected an expression statement}} ; -// expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'read' clause}} +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} + foo(); +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} + a += b; +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected lvalue expression}} + a = 0; +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} + a = b; + // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'read' clause}} #pragma omp atomic read read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} a = b; - return T(); + return a; } int read() { - int a, b = 0; + int a = 0, b = 0; // Test for atomic read #pragma omp atomic read - // expected-error@+1 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both l-value expressions with scalar type}} + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected an expression statement}} ; -// expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'read' clause}} +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} + foo(); +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected built-in assignment operator}} + a += b; +#pragma omp atomic read + // expected-error@+2 {{the statement for 'atomic read' must be an expression statement of form 'v = x;', where v and x are both lvalue expressions with scalar type}} + // expected-note@+1 {{expected lvalue expression}} + a = 0; +#pragma omp atomic read + a = b; + // expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'read' clause}} #pragma omp atomic read read a = b; - return read(); + // expected-note@+1 {{in instantiation of function template specialization 'read' requested here}} + return read() + read().a; } template -- 2.7.4