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

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a subscriber: yhegde.
kiranchandramohan added a comment.

Quick comments/questions before I have a final look.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:636
+          OmpClauseSet{llvm::omp::Clause::OMPC_reduction}));
+  CheckPrivateSymbolsInOuterCxt(
+      currSymbols, dirClauseTriple, llvm::omp::Clause::OMPC_firstprivate);
----------------
Nit: Add an empty line above.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1033-1040
+                       [&](const parser::OmpClause::Reduction &x) {
+                         if (enclosingClauseSet.test(
+                                 llvm::omp::Clause::OMPC_reduction)) {
+                           const auto &ompObjectList{
+                               std::get<parser::OmpObjectList>(x.v.t)};
+                           GetSymbolsInObjectList(
+                               ompObjectList, enclosingSymbols);
----------------
Will this clash with the reduction work that @yhegde is doing in https://reviews.llvm.org/D90697?


================
Comment at: flang/lib/Semantics/check-omp-structure.h:85-87
+using SymbolSourceMap = std::multimap<const Symbol *, parser::CharBlock>;
+using DirectivesClauseTriple = std::multimap<llvm::omp::Directive,
+    std::pair<llvm::omp::Directive, const OmpClauseSet>>;
----------------
Can you add comments on why these Data Structures are necessary either here or where they are used?


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

https://reviews.llvm.org/D91920