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

[Alexey Bataev via cfe-commits]

ABataev added inline comments.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1876-1878
@@ +1875,5 @@
+                                  SourceLocation Loc,
+                                  OpenMPScheduleClauseKind ForSchedKind,
+                                  OpenMPDistScheduleClauseKind DistSchedKind,
+                                  bool isForSchedule,
+                                  unsigned IVSize, bool IVSigned, bool Ordered,
----------------
I think you can pass OpenMPSchedType here instead of OpenMPScheduleClauseKind  and OpenMPDistScheduleClauseKind .

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:187-208
@@ -185,30 +186,24 @@
+
+public:
   /// \brief Values for bit flags used in the ident_t to describe the fields.
   /// All enumeric elements are named and described in accordance with the code
   /// from http://llvm.org/svn/llvm-project/openmp/trunk/runtime/src/kmp.h
   enum OpenMPLocationFlags {
     /// \brief Use trampoline for internal microtask.
     OMP_IDENT_IMD = 0x01,
     /// \brief Use c-style ident structure.
     OMP_IDENT_KMPC = 0x02,
     /// \brief Atomic reduction option for kmpc_reduce.
     OMP_ATOMIC_REDUCE = 0x10,
     /// \brief Explicit 'barrier' directive.
     OMP_IDENT_BARRIER_EXPL = 0x20,
     /// \brief Implicit barrier in code.
     OMP_IDENT_BARRIER_IMPL = 0x40,
     /// \brief Implicit barrier in 'for' directive.
     OMP_IDENT_BARRIER_IMPL_FOR = 0x40,
     /// \brief Implicit barrier in 'sections' directive.
     OMP_IDENT_BARRIER_IMPL_SECTIONS = 0xC0,
     /// \brief Implicit barrier in 'single' directive.
     OMP_IDENT_BARRIER_IMPL_SINGLE = 0x140
   };
 
----------------
No, do not make it public. It must be private forever. Moreover, It would be good if could hide it at all in .cpp file. I will look at it.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:553-566
@@ -564,2 +552,16 @@
 
+  /// \brief Emits object of ident_t type with info for source location.
+  /// \param Flags Flags for OpenMP location.
+  ///
+  llvm::Value *emitUpdateLocation(CodeGenFunction &CGF, SourceLocation Loc,
+                                  OpenMPLocationFlags Flags = OMP_IDENT_KMPC);
+
+  /// \brief Gets thread id value for the current thread.
+  ///
+  llvm::Value *getThreadID(CodeGenFunction &CGF, SourceLocation Loc);
+
+  /// \brief Returns __kmpc_for_static_init_* runtime function for the specified
+  /// size \a IVSize and sign \a IVSigned.
+  llvm::Constant *createForStaticInitFunction(unsigned IVSize, bool IVSigned);
+
   /// \brief Emits outlined function for the specified OpenMP parallel directive
----------------
Keep them private, please

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:1892-1894
@@ -1853,4 +1891,5 @@
     // Emit static non-chunked loop.
-    CGF.CGM.getOpenMPRuntime().emitForStaticInit(
-        CGF, S.getLocStart(), OMPC_SCHEDULE_static, /*IVSize=*/32,
+    CGF.CGM.getOpenMPRuntime().emitForStaticInit(CGF, S.getLocStart(),
+        OMPC_SCHEDULE_static,
+        /*IVSize=*/32,
         /*IVSigned=*/true, /*Ordered=*/false, IL.getAddress(), LB.getAddress(),
----------------
Revert it back, no actual changes

================
Comment at: lib/Serialization/ASTReaderStmt.cpp:2303
@@ -2303,1 +2302,3 @@
+      isOpenMPTaskLoopDirective(D->getDirectiveKind()) ||
+	  isOpenMPDistributeDirective(D->getDirectiveKind())) {
     D->setIsLastIterVariable(Reader.ReadSubExpr());
----------------
Reformat it, please, indentation is wrong

================
Comment at: lib/Serialization/ASTWriterStmt.cpp:2091
@@ -2091,1 +2090,3 @@
+      isOpenMPTaskLoopDirective(D->getDirectiveKind()) ||
+	  isOpenMPDistributeDirective(D->getDirectiveKind())) {
     Writer.AddStmt(D->getIsLastIterVariable());
----------------
Indentation


Repository:
  rL LLVM

http://reviews.llvm.org/D17170