[PATCH] [OPENMP] CodeGen - 'omp for' with dynamic schedule kinds

[John McCall]

Generally looks good.  A couple organization suggestions to eliminate some of the code duplication.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:426
@@ -425,1 +425,3 @@
   }
+  case OMPRTL__kmpc_dispatch_init_4: {
+    auto ITy = CGM.Int32Ty;
----------------
Your only caller here pattern-matches on the integer size and signedness so that it can pass the right enum value down just so that this code can immediately switch out to cases that are identical except for slight differences in integer size and signedness.  This is kindof silly.

You don't *have* to shove everything through this one method; you can have a CreateDispatchRuntimeFunction method or something that returns the appropriate function from a named family for a given integer size and signedness.  Or you could be slightly less type-safe and just have CreateRuntimeFunction take (defaultable) arguments that pick the right function from a family, like how LLVM overloaded intrinsics work.

Please do something like that instead of having all this redundancy, though.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:383
@@ +382,3 @@
+  /// \param CGF Reference to current CodeGenFunction.
+  /// \param Loc Clang source location.
+  /// \param IVSize Size of the iteration variable in bits.
----------------
These are not useful documentation.  You can just leave off the comment about CGF.  For Loc, there's probably an existing discussion about the purpose of the source locations you can refer to.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:394
@@ +393,3 @@
+  /// returned.
+  virtual llvm::CallInst *EmitOMPForNext(CodeGenFunction &CGF,
+                                         SourceLocation Loc, unsigned IVSize,
----------------
Why does this specifically return a CallInst* instead of just a Value*?

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:591
@@ +590,3 @@
+                                  LB, UB, ST);
+    BoolCondVal = EmitScalarConversion(
+        Call, getContext().getIntTypeForBitwidth(32, /* Signed */ true),
----------------
Just have EmitOMPForNext do this conversion and return an i1.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:610
@@ -564,1 +609,3 @@
 
+  if (Dynamic)
+    EmitIgnoredExpr(S.getInit());
----------------
Is there no init otherwise?  Seems like something that should be asserted.

http://reviews.llvm.org/D7138

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/