[PATCH] D110714: [Flang][openmp] Added semantic checks for atomic construct with update clause

[Peixin Qiao via Phabricator via llvm-commits]

peixin added a comment.

The following description is not correct.

  In atomic update statement, the statements must be of the form x = x operator expr or x = expr operator x

Please check my previous comments:
According to OpenMP 5.0 Spec 2.17.7 at page 238, the update-statement has one of the following forms:
x = x operator expr
x = expr operator x
x = intrinsic_procedure_name (x, expr_list)
x = intrinsic_procedure_name (expr_list, x)



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1291
+template <typename T, typename D>
+bool OmpStructureChecker::CheckOperatorValidity(
+    const T &node, const D &variable) {
----------------
CheckOperatorValidity -> IsOperatorValid considering that the "true" is returned when the operator is valid.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1311
+      context_.Say(variable.GetSource(),
+          "Atomic update variable '%s' not found in the RHS of the assignment statement in an ATOMIC UPDATE construct"_err_en_US,
+          variableName);
----------------
ATOMIC UPDATE -> ATOMIC (UPDATE)?


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1322
+  return true;
+}
+template <typename T>
----------------
Please add one blank line after line 1322.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1346
+              context_.Say(expr.source,
+                  "Invalid intrinsic procedure name in OpenMP ATOMIC construct UPDATE statement"_err_en_US);
+            } else if (name) {
----------------
ATOMIC construct UPDATE -> ATOMIC (UPDATE)?


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1374
+              context_.Say(expr.source,
+                  "Invalid operator in OpenMP ATOMIC construct UPDATE statement"_err_en_US);
+            }
----------------
ATOMIC construct UPDATE -> ATOMIC (UPDATE)?


================
Comment at: flang/test/Semantics/omp-atomic02.f90:22
+   a = a + (4*2)
+   !$omp atomic
+   a = a*(b + 1)
----------------
Can you add a blank line between every two test cases? It's hard to read these test cases.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110714/new/

https://reviews.llvm.org/D110714