[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7956
     // Map other list items in the map clause which are not captured variables
-    // but "declare target link" global variables.,
+    // but "declare target link" global variables.
     for (const auto *C : this->CurDir.getClausesOfKind<OMPMapClause>()) {
----------------
ABataev wrote:
> Restore the original, this must be changed in a separate patch.
Restore


================
Comment at: lib/Sema/SemaOpenMP.cpp:196
       nullptr};
+  /// Vector of previously encountered target directives
+  SmallVector<SourceLocation, 2> TargetLocations;
----------------
I would suggest to split this patch into 2 parts: the sema checking part and the codegen part.


================
Comment at: lib/Sema/SemaOpenMP.cpp:468
+  void checkEncounteredTargets(SourceLocation Loc,
+                               ArrayRef<OMPClause *> ClauseList) {
+    for (OMPClause *CNew : ClauseList) {
----------------
Make function `const`


================
Comment at: lib/Sema/SemaOpenMP.cpp:469
+                               ArrayRef<OMPClause *> ClauseList) {
+    for (OMPClause *CNew : ClauseList) {
+      // Check if any of the requires clauses affect target regions.
----------------
`const OMPClause *`


================
Comment at: lib/Sema/SemaOpenMP.cpp:476
+           isa<OMPDynamicAllocatorsClause>(CNew))) {
+        SemaRef.Diag(Loc,
+                     diag::err_omp_target_before_requires)
----------------
DSAStack usually is not used for diagnostic emission, it serves as the container. Better to move the diagnostics to Act... functions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568