[PATCH] D71294: [OpenMP] Emit warning message about undefined number of evaluation for atomic update/capture

[Chi Chun Chen via Phabricator via cfe-commits]

cchen created this revision.
cchen added a reviewer: ABataev.
Herald added subscribers: cfe-commits, jfb, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

The total number of evaluation of X is undefined for atomic construct,
therefore, we emit warning message for them. BTW, Clang is always
evaluate X once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71294

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.cpp
  clang/test/OpenMP/atomic_update_codegen.cpp


Index: clang/test/OpenMP/atomic_update_codegen.cpp
===================================================================
--- clang/test/OpenMP/atomic_update_codegen.cpp
+++ 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;
Index: clang/test/OpenMP/atomic_messages.cpp
===================================================================
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -767,3 +767,42 @@
   return mixed<int>();
 }
 
+int cnt = 0;
+int add() {
+  return ++cnt;
+}
+
+template <class T>
+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<int>' requested here}}
+  return warning<int>();
+}
Index: clang/lib/Sema/SemaOpenMP.cpp
===================================================================
--- clang/lib/Sema/SemaOpenMP.cpp
+++ 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<BinaryOperator>(E)) {
+    traverseBinOp(BinOp->getLHS());
+    traverseBinOp(BinOp->getRHS());
+  } else {
+    if (auto *Call = dyn_cast<CallExpr>(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<ArraySubscriptExpr>(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;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ 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<OpenMPClauses>;
+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<

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits