[PATCH] D15125: [OPENMP] 'omp distribute' directive basic support.

[Alexey Bataev via cfe-commits]

ABataev added inline comments.

================
Comment at: include/clang/AST/OpenMPClause.h:667
@@ -666,3 +666,3 @@
   OpenMPScheduleClauseKind Kind;
-  /// \brief Start location of the schedule ind in source code.
+  /// \brief Start location of the schedule kind in source code.
   SourceLocation KindLoc;
----------------
This must be in a separate patch

================
Comment at: include/clang/AST/OpenMPClause.h:708-709
@@ -707,4 +707,4 @@
 public:
-  /// \brief Build 'schedule' clause with schedule kind \a Kind and chunk size
-  /// expression \a ChunkSize.
+  /// \brief Build 'dist_schedule' clause with schedule kind \a Kind and chunk
+  /// size expression \a ChunkSize.
   ///
----------------
I think this is wrong, this is not for dist_schedule, but for schedule clause

================
Comment at: include/clang/AST/OpenMPClause.h:779
@@ -778,1 +778,3 @@
 
+/// \brief This represents 'dist_schedule' clause in the '#pragma omp ...'
+/// directive.
----------------
This must be in a separate patch

================
Comment at: include/clang/AST/RecursiveASTVisitor.h:2732-2738
@@ -2727,1 +2731,9 @@
 template <typename Derived>
+bool RecursiveASTVisitor<Derived>::VisitOMPDistScheduleClause(
+    OMPDistScheduleClause *C) {
+  TRY_TO(TraverseStmt(C->getChunkSize()));
+  TRY_TO(TraverseStmt(C->getHelperChunkSize()));
+  return true;
+}
+
+template <typename Derived>
----------------
separate patch

================
Comment at: include/clang/AST/StmtOpenMP.h:2279
@@ +2278,3 @@
+  /// \brief true if current directive has inner cancel directive.
+  bool HasCancel;
+
----------------
Distribute directive cannot have inner cancel

================
Comment at: include/clang/Basic/OpenMPKinds.h:89-95
@@ -88,2 +88,9 @@
 
+/// \brief OpenMP attributes for 'dist_schedule' clause.
+enum OpenMPDistScheduleClauseKind {
+#define OPENMP_DIST_SCHEDULE_KIND(Name) OMPC_DIST_SCHEDULE_##Name,
+#include "clang/Basic/OpenMPKinds.def"
+  OMPC_DIST_SCHEDULE_unknown
+};
+
 OpenMPDirectiveKind getOpenMPDirectiveKind(llvm::StringRef Str);
----------------
Separate patch

================
Comment at: lib/AST/StmtPrinter.cpp:882
@@ +881,3 @@
+void OMPClausePrinter::VisitOMPDistScheduleClause(OMPDistScheduleClause *Node) {
+  OS << "schedule(" << getOpenMPSimpleClauseTypeName(
+                           OMPC_dist_schedule, Node->getDistScheduleKind());
----------------
dist_schedule, not schedule

================
Comment at: lib/Basic/OpenMPKinds.cpp:475
@@ -452,1 +474,3 @@
+         DKind == OMPD_parallel_for_simd || DKind == OMPD_parallel_sections ||
+         DKind == OMPD_distribute; // TODO add next directives.
 }
----------------
distribute is not a worksharing directive

================
Comment at: lib/Parse/ParseOpenMP.cpp:162
@@ -160,3 +161,3 @@
 ///         'for simd' | 'parallel for simd' | 'target' | 'target data' |
-///         'taskgroup' | 'teams' {clause}
+///         'taskgroup' | 'teams' {clause} | 'distribute'
 ///         annot_pragma_openmp_end
----------------
'distribute' misses clauses, put your changes right after 'teams'

================
Comment at: lib/Parse/ParseOpenMP.cpp:239
@@ -238,1 +238,3 @@
+  case OMPD_distribute:
+  case OMPD_target_data: {
     ConsumeToken();
----------------
Restore the order of target_data and taskloop directives, put distribute right after taskloop

================
Comment at: lib/Sema/SemaOpenMP.cpp:1530
@@ -1521,2 +1529,3 @@
   // | Parent directive | Child directive | Closely (!), No-Closely(+), Both(*)|
+  // |                  |                 | Strictly (!!)                      |
   // +------------------+-----------------+------------------------------------+
----------------
It is not required, closely without no-closely means strictly

================
Comment at: lib/Sema/SemaOpenMP.cpp:1989
@@ -1979,2 +1988,3 @@
   // | teams            | taskloop        | +                                  |
+  // | teams            | distribute      | !!                                 |
   // +------------------+-----------------+------------------------------------+
----------------
Add rules for distribute to all other directives

================
Comment at: lib/Sema/SemaOpenMP.cpp:2026-2027
@@ -2015,3 +2025,4 @@
       ShouldBeInOrderedRegion,
-      ShouldBeInTargetRegion
+      ShouldBeInTargetRegion,
+      ShouldBeInTeamsRegion
     } Recommend = NoRecommend;
----------------
I don't think this change is required

================
Comment at: lib/Sema/SemaOpenMP.cpp:2062-2072
@@ -2050,2 +2061,13 @@
       return false;
+    if (isOpenMPDistributeDirective(CurrentRegion)) {
+      // OpenMP 4.5 [2.17 Nesting of Regions]
+      // The region associated with the distribute construct must be strictly
+      // nested inside a teams region
+      if (ParentRegion != OMPD_teams) {
+        SemaRef.Diag(StartLoc,
+                     diag::err_omp_distribute_strictly_nested_in_teams);
+        return true;
+      }
+      return false;
+    }
     if (CurrentRegion == OMPD_cancellation_point ||
----------------
This must be handled in lines 2173-2181

================
Comment at: tools/libclang/CIndex.cpp:4489
@@ -4477,1 +4488,3 @@
+  case CXCursor_OMPDistributeDirective:
+    return cxstring::createRef("OMPForDirective");
   case CXCursor_OverloadCandidate:
----------------
kkwli0 wrote:
> "OMPDistributeDirective"
"OMPDistributeDirective"


Repository:
  rL LLVM

http://reviews.llvm.org/D15125