[PATCH] D17170: [OPENMP] Codegen for distribute directive

[Alexey Bataev via cfe-commits]

ABataev added inline comments.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:669-692
@@ -668,1 +668,26 @@
 
+  /// \brief Schedule types for 'omp for' loops (these enumerators are taken from
+   /// the enum sched_type in kmp.h).
+   enum OpenMPSchedType {
+     /// \brief Lower bound for default (unordered) versions.
+     OMP_sch_lower = 32,
+     OMP_sch_static_chunked = 33,
+     OMP_sch_static = 34,
+     OMP_sch_dynamic_chunked = 35,
+     OMP_sch_guided_chunked = 36,
+     OMP_sch_runtime = 37,
+     OMP_sch_auto = 38,
+     /// \brief Lower bound for 'ordered' versions.
+     OMP_ord_lower = 64,
+     OMP_ord_static_chunked = 65,
+     OMP_ord_static = 66,
+     OMP_ord_dynamic_chunked = 67,
+     OMP_ord_guided_chunked = 68,
+     OMP_ord_runtime = 69,
+     OMP_ord_auto = 70,
+     /// \brief dist_schedule types
+     OMP_dist_sch_static_chunked = 91,
+     OMP_dist_sch_static = 92,
+     OMP_sch_default = OMP_sch_static,
+   };
+
----------------
I don't like the idea of exposing this in the header file. getRuntimeSchedule() can be turned into static functions in .cpp file

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:717-725
@@ -682,2 +716,11 @@
 
+  /// \brief Check if the specified \a DistScheduleKind is static non-chunked.
+  /// This kind of distribute directive is emitted without outer loop.
+  /// \param ScheduleKind DistSchedule kind specified in the 'dist_schedule'
+  /// clause.
+  /// \param Chunked True if chunk is specified in the clause.
+  ///
+  virtual bool isDistStaticNonchunked(OpenMPDistScheduleClauseKind ScheduleKind,
+                                      bool Chunked) const;
+
   virtual void emitForDispatchInit(CodeGenFunction &CGF, SourceLocation Loc,
----------------
What's the difference between isStaticNonchunked() and isDistStaticNonchunked()? For me they must be the same.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:759
@@ +758,3 @@
+  ///
+  void emitForStaticInitWithKMPSchedule(CodeGenFunction &CGF,
+                                        SourceLocation Loc,
----------------
I don't think this function is required at all. You can just add an optional argument to emitForStaticInit() function.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:761
@@ +760,3 @@
+                                        SourceLocation Loc,
+                                        OpenMPSchedType Schedule,
+                                        unsigned IVSize, bool IVSigned,
----------------
It is a bad idea to expose runtime-specific type in interface functions. Use OpenMPDistScheduleClauseKind instead and translate it inside.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:1540-1541
@@ +1539,4 @@
+
+  RT.emitDistributeStaticInit(*this, S.getLocStart(), ScheduleKind,
+                         IVSize, IVSigned, IL, LB, UB, ST, Chunk);
+
----------------
Bad formatting

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2185-2216
@@ -2147,1 +2184,34 @@
 
+static std::pair<llvm::Value * /*Chunk*/, OpenMPDistScheduleClauseKind>
+emitDistScheduleClause(CodeGenFunction &CGF, const OMPDistributeDirective &S,
+                   bool OuterRegion) {
+  // Detect the distribute schedule kind and chunk.
+  auto ScheduleKind = OMPC_DIST_SCHEDULE_unknown;
+  llvm::Value *Chunk = nullptr;
+  if (const auto *C = S.getSingleClause<OMPDistScheduleClause>()) {
+    ScheduleKind = C->getDistScheduleKind();
+    if (const auto *Ch = C->getChunkSize()) {
+      if (auto *ImpRef = cast_or_null<DeclRefExpr>(C->getHelperChunkSize())) {
+        if (OuterRegion) {
+          const VarDecl *ImpVar = cast<VarDecl>(ImpRef->getDecl());
+          CGF.EmitVarDecl(*ImpVar);
+          CGF.EmitStoreThroughLValue(
+              CGF.EmitAnyExpr(Ch),
+              CGF.MakeAddrLValue(CGF.GetAddrOfLocalVar(ImpVar),
+                                 ImpVar->getType()));
+        } else {
+          Ch = ImpRef;
+        }
+      }
+      if (!C->getHelperChunkSize() || !OuterRegion) {
+        Chunk = CGF.EmitScalarExpr(Ch);
+        Chunk = CGF.EmitScalarConversion(Chunk, Ch->getType(),
+                                         S.getIterationVariable()->getType(),
+                                         S.getLocStart());
+      }
+    }
+  }
+  return std::make_pair(Chunk, ScheduleKind);
+}
+
+void CodeGenFunction::EmitOMPDistributeLoop(const OMPDistributeDirective &S) {
----------------
Wait few days for this code. I will commit improved code for schedule clause codegen, that simplifies all similar stuff

================
Comment at: lib/CodeGen/CodeGenFunction.h:2405
@@ -2397,2 +2404,3 @@
   OpenMPDirectiveKind EmitSections(const OMPExecutableDirective &S);
+  void EmitOMPDistributeLoop(const OMPDistributeDirective &S);
 
----------------
This must be public along with other codegen function for other directives


Repository:
  rL LLVM

http://reviews.llvm.org/D17170