diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -9265,6 +9265,8 @@ def warn_omp_alignment_not_power_of_two : Warning< "aligned clause will be ignored because the requested alignment is not a power of 2">, InGroup; +def warn_omp_atomic_evaluation_times_undefined : Warning< + "the number of times the expression evaluated is unspecified">; def err_omp_invalid_target_decl : Error< "%0 used in declare target directive is not a variable or a function name">; def err_omp_declare_target_multiple : Error< diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -8493,6 +8493,8 @@ private: bool checkBinaryOperation(BinaryOperator *AtomicBinOp, unsigned DiagId = 0, unsigned NoteId = 0); + void traverseBinOp(const Expr *E); + void checkArraySubscriptSideEffect(const Expr *X); }; } // namespace @@ -8564,6 +8566,34 @@ return ErrorFound != NoError; } +void OpenMPAtomicUpdateChecker::traverseBinOp(const Expr *E) { + if (!E) + return; + if (auto *BinOp = dyn_cast(E)) { + traverseBinOp(BinOp->getLHS()); + traverseBinOp(BinOp->getRHS()); + } else { + if (auto *Call = dyn_cast(E)) { + SemaRef.Diag(E->getExprLoc(), + diag::warn_omp_atomic_evaluation_times_undefined); + } + } +} + +void OpenMPAtomicUpdateChecker::checkArraySubscriptSideEffect(const Expr *X) { + if (!X) + return; + // If X is an ArraySubscript and the expression has side effect, then we'll + // have wrong result since we only evaluate the expression once. + if (X->getStmtClass() == Expr::ArraySubscriptExprClass) { + auto E = cast(X); + if (E->getLHS() != E->getIdx()) { + assert(E->getRHS() == E->getIdx() && "index was neither LHS nor RHS"); + traverseBinOp(E->getIdx()); + } + } +} + bool OpenMPAtomicUpdateChecker::checkStatement(Stmt *S, unsigned DiagId, unsigned NoteId) { ExprAnalysisErrorCode ErrorFound = NoError; @@ -8653,6 +8683,7 @@ return true; UpdateExpr = Update.get(); } + checkArraySubscriptSideEffect(X); return ErrorFound != NoError; } diff --git a/clang/test/OpenMP/atomic_messages.cpp b/clang/test/OpenMP/atomic_messages.cpp --- a/clang/test/OpenMP/atomic_messages.cpp +++ b/clang/test/OpenMP/atomic_messages.cpp @@ -767,3 +767,42 @@ return mixed(); } +int cnt = 0; +int add() { + return ++cnt; +} + +template +T warning() { + T arr[100], b = T(); + +// expected-warning@+2 2 {{the number of times the expression evaluated is unspecified}} +#pragma omp atomic update + arr[add(), add(), 0] = arr[add(), add(), 0] - 10; + +// expected-warning@+3 4 {{expression result unused}} +// expected-warning@+2 2 {{the number of times the expression evaluated is unspecified}} +#pragma omp atomic capture + b = arr[add () + add (), 0] = arr[add () + add (), 0] + 3; + +// expected-warning@+3 {{the number of times the expression evaluated is unspecified}} +#pragma omp atomic capture + { + arr[foo (), 0] += 6; + b = arr[foo (), 0]; + } + +// expected-warning@+3 {{the number of times the expression evaluated is unspecified}} +#pragma omp atomic capture + { + arr[foo (), 0] = arr[foo (), 0] + 6; + b = arr[foo (), 0]; + } + + return T(); +} + +int warning() { + // expected-note@+1 {{in instantiation of function template specialization 'warning' requested here}} + return warning(); +} diff --git a/clang/test/OpenMP/atomic_update_codegen.cpp b/clang/test/OpenMP/atomic_update_codegen.cpp --- a/clang/test/OpenMP/atomic_update_codegen.cpp +++ b/clang/test/OpenMP/atomic_update_codegen.cpp @@ -27,6 +27,12 @@ _Complex int civ, cix; _Complex float cfv, cfx; _Complex double cdv, cdx; +int iarr[100]; +int cnt = 0; + +int foo() { + return cnt++; +} typedef int int4 __attribute__((__vector_size__(16))); int4 int4x;