[PATCH] [OPENMP] Codegen for 'num_threads' clause in 'parallel' directive

[Alexey Bataev]

================
Comment at: include/clang/AST/StmtOpenMP.h:131
@@ -130,1 +130,3 @@
 
+  /// \brief Gets single clause of the specified kind \a K associated with the
+  /// current directive iff there is only one clause of this kind.
----------------
hfinkel wrote:
> Make it "Gets a single" or "Gets the single"
Ok

================
Comment at: include/clang/AST/StmtOpenMP.h:132
@@ +131,3 @@
+  /// \brief Gets single clause of the specified kind \a K associated with the
+  /// current directive iff there is only one clause of this kind.
+  const OMPClause *getSingleClause(OpenMPClauseKind K) const;
----------------
hfinkel wrote:
> Also, make it clear that this asserts if there is more than one clause of the specified kind.
Ok

================
Comment at: lib/AST/Stmt.cpp:1396
@@ +1395,3 @@
+    auto PrevI = I;
+    assert(!++I && "There is at least 2 clauses of the  specified kind");
+    return *PrevI;
----------------
hfinkel wrote:
> "is at least" -> "are at least"
> Extra space in "the  specified"
Fixed

================
Comment at: lib/Sema/SemaOpenMP.cpp:2592
@@ -2591,3 +2591,3 @@
   Expr *ValExpr = NumThreads;
   if (!NumThreads->isValueDependent() && !NumThreads->isTypeDependent() &&
       !NumThreads->isInstantiationDependent() &&
----------------
hfinkel wrote:
> Do you need all of these checks, or is value-dependent enough?
I think we need all of them, because num_threads argument is not a constant, but an expression.

================
Comment at: lib/Sema/SemaOpenMP.cpp:2613
@@ +2612,3 @@
+    // Convert to int32 for runtime call.
+    ValExpr = PerformImplicitConversion(
+        ValExpr,
----------------
hfinkel wrote:
> While it is true that omp_get/set_num_threads uses 'int' to represent the number of threads, I think this cast (trunc/ext) to int32_t should be done in CodeGen, not here (it is runtime specific -- the AST should retain the original type).
Ok, I'll fix it.

http://reviews.llvm.org/D5145