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

[hfinkel at anl.gov]

================
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.
----------------
Make it "Gets a single" or "Gets the single"

================
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;
----------------
Also, make it clear that this asserts if there is more than one clause of the specified kind.

================
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;
----------------
"is at least" -> "are at least"
Extra space in "the  specified"

================
Comment at: lib/Sema/SemaOpenMP.cpp:2592
@@ -2591,3 +2591,3 @@
   Expr *ValExpr = NumThreads;
   if (!NumThreads->isValueDependent() && !NumThreads->isTypeDependent() &&
       !NumThreads->isInstantiationDependent() &&
----------------
Do you need all of these checks, or is value-dependent enough?

================
Comment at: lib/Sema/SemaOpenMP.cpp:2613
@@ +2612,3 @@
+    // Convert to int32 for runtime call.
+    ValExpr = PerformImplicitConversion(
+        ValExpr,
----------------
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).

================
Comment at: test/OpenMP/parallel_num_threads_codegen.cpp:5
@@ +4,3 @@
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
----------------
Don't need include-guards here.

http://reviews.llvm.org/D5145