[PATCH] D91920: [Flang] [OpenMP] Add semantic checks for OpenMP firstprivate , lastprivate and copyprivate clauses

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.

A few comments.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:687
+
+  if (currDirSet.test(GetContext().directive)) {
+    if (auto *enclosingContext{GetEnclosingDirContext()}) {
----------------
If this section is common code with firstprivate then sharing code would be good.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:690
+      if (outerDirSet.test(enclosingContext->directive)) {
+        if (const auto *clause{FindClauseInContext(
+                llvm::omp::Clause::OMPC_reduction, *enclosingContext)}) {
----------------
"A list item that is private within a parallel region, or that appears in the reduction clause of a parallel construct, must not appear in a lastprivate clause on a worksharing construct if any of the corresponding worksharing regions ever binds to any of the corresponding parallel regions."

It seems you are checking for reduction only here. From the standard (quoted above) it appears that we have to check for all kind of privates and reduction.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:805
 
+void OmpStructureChecker::GetSymbolsInDesignatorList(
+    const std::list<parser::Designator> &designatorList,
----------------
Can this function go to the base class?


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

https://reviews.llvm.org/D91920