[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:52-74
 enum DefaultMapAttributes {
-  DMA_unspecified,   /// Default mapping is not specified.
-  DMA_tofrom_scalar, /// Default mapping is 'tofrom:scalar'.
+  DMA_unspecified,             /// Default mapping is not specified.
+  DMA_alloc_scalar,            /// Default mapping is 'alloc:scalar'.
+  DMA_to_scalar,               /// Default mapping is 'to:scalar'.
+  DMA_from_scalar,             /// Default mapping is 'from:scalar'.
+  DMA_tofrom_scalar,           /// Default mapping is 'tofrom:scalar'.
+  DMA_firstprivate_scalar,     /// Default mapping is 'firstprivate:scalar'.
----------------
Bad decision, better to add 2 new enums, one for behavior and one for the variable category.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:147-148
     SourceLocation DefaultAttrLoc;
-    DefaultMapAttributes DefaultMapAttr = DMA_unspecified;
-    SourceLocation DefaultMapAttrLoc;
+    llvm::SmallVector<std::pair<DefaultMapAttributes, SourceLocation>, 3>
+      DefaultmapMap{3, std::make_pair(DMA_unspecified, SourceLocation())};
     OpenMPDirectiveKind Directive = OMPD_unknown;
----------------
No need to use the vector here, use the regular array


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2983-2992
+      // OpenMP 5.0 [2.19.7.2, defaultmap clause, Description]
+      // If implicit-behavior is none, each variable referenced in the
+      // construct that does not have a predetermined data-sharing attribute
+      // and does not appear in a to or link clause on a declare target
+      // directive must be listed in a data-mapping attribute clause, a
+      // data-haring attribute clause (including a data-sharing attribute
+      // clause on a combined construct where target. is one of the
----------------
It must be active only for OpenMP >= 50.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4463-4890
+
   if (AStmt && !CurContext->isDependentContext()) {
     assert(isa<CapturedStmt>(AStmt) && "Captured statement expected");
 
     // Check default data sharing attributes for referenced variables.
     DSAAttrChecker DSAChecker(DSAStack, *this, cast<CapturedStmt>(AStmt));
+
----------------
The formatting changes better to put in a separate NFC patch if you think that this is required.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4899-4903
+    } else {
+      Diag(P.second->getExprLoc(), diag::err_omp_defaultmap_no_attr_for_variable)
+          << P.first << P.second->getSourceRange();
+      Diag(DSAStack->getDefaultDSALocation(), diag::note_omp_defaultmap_attr_none);
+    }
----------------
Add a check for OpenMP version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204