[OPENMP] Additional processing of 'omp atomic read' directive.
authorAlexey Bataev <a.bataev@hotmail.com>
Tue, 18 Nov 2014 10:14:22 +0000 (10:14 +0000)
committerAlexey Bataev <a.bataev@hotmail.com>
Tue, 18 Nov 2014 10:14:22 +0000 (10:14 +0000)
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
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/Stmt.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/OpenMP/atomic_messages.c [new file with mode: 0644]
clang/test/OpenMP/atomic_messages.cpp

index a031d8b..b9402f4 100644 (file)
@@ -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<OMPClause *> Clauses, Stmt *AssociatedStmt);
+         ArrayRef<OMPClause *> 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<Expr>(*std::next(child_begin())); }
+  const Expr *getX() const {
+    return cast_or_null<Expr>(*std::next(child_begin()));
+  }
+  /// \brief Get 'v' part of the associated expression/statement.
+  Expr *getV() { return cast_or_null<Expr>(*std::next(child_begin(), 2)); }
+  const Expr *getV() const {
+    return cast_or_null<Expr>(*std::next(child_begin(), 2));
+  }
+  /// \brief Get 'expr' part of the associated expression/statement.
+  Expr *getExpr() { return cast_or_null<Expr>(*std::next(child_begin(), 3)); }
+  const Expr *getExpr() const {
+    return cast_or_null<Expr>(*std::next(child_begin(), 3));
+  }
+
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == OMPAtomicDirectiveClass;
   }
index c175b98..d01a2d6 100644 (file)
@@ -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">;
index c323d98..371a7a1 100644 (file)
@@ -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<OMPClause *> Clauses,
-                                               Stmt *AssociatedStmt) {
+OMPAtomicDirective *
+OMPAtomicDirective::Create(const ASTContext &C, SourceLocation StartLoc,
+                           SourceLocation EndLoc, ArrayRef<OMPClause *> Clauses,
+                           Stmt *AssociatedStmt, Expr *X, Expr *V, Expr *E) {
   unsigned Size = llvm::RoundUpToAlignment(sizeof(OMPAtomicDirective),
                                            llvm::alignOf<OMPClause *>());
-  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<OMPClause *>());
   void *Mem =
-      C.Allocate(Size + sizeof(OMPClause *) * NumClauses + sizeof(Stmt *));
+      C.Allocate(Size + sizeof(OMPClause *) * NumClauses + 4 * sizeof(Stmt *));
   return new (Mem) OMPAtomicDirective(NumClauses);
 }
 
index 9bdb092..e4838de 100644 (file)
@@ -3223,13 +3223,94 @@ StmtResult Sema::ActOnOpenMPAtomicDirective(ArrayRef<OMPClause *> 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<Expr>(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<Expr>(Body)) {
+      auto AtomicBinOp =
+          dyn_cast<BinaryOperator>(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<Expr>(Body)) {
       Diag(Body->getLocStart(),
@@ -3257,7 +3338,8 @@ StmtResult Sema::ActOnOpenMPAtomicDirective(ArrayRef<OMPClause *> 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<OMPClause *> Clauses,
index 0e4d994..e0308d7 100644 (file)
@@ -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) {
index 309e0a6..38996dd 100644 (file)
@@ -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 (file)
index 0000000..d2d3308
--- /dev/null
@@ -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;
+}
index b53133f..3965a77 100644 (file)
@@ -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 <class T>
 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<int>();
+  // expected-note@+1 {{in instantiation of function template specialization 'read<S>' requested here}}
+  return read<int>() + read<S>().a;
 }
 
 template <class T>