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

[ABataev]

Summary

This patch generates call to "kmpc_push_num_threads(ident_t *loc, kmp_int32 global_tid, kmp_int32 num_threads);" library function before calling "kmpc_fork_call" each time there is an associated "num_threads" clause in the "omp parallel" directive.

[ABataev]

Ping

[hfinkel]

include/clang/AST/StmtOpenMP.h

131

Make it "Gets a single" or "Gets the single"

132

Also, make it clear that this asserts if there is more than one clause of the specified kind.

lib/AST/Stmt.cpp

1396

"is at least" -> "are at least"
Extra space in "the specified"

lib/Sema/SemaOpenMP.cpp

2726

Do you need all of these checks, or is value-dependent enough?

2746

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).

test/OpenMP/parallel_num_threads_codegen.cpp

6

Don't need include-guards here.

[hfinkel]

test/OpenMP/parallel_num_threads_codegen.cpp

6

n/m -- as you pointed out in the other review, these are needed because of the PCH testing.

[ABataev]

include/clang/AST/StmtOpenMP.h

131

Ok

132

Ok

lib/AST/Stmt.cpp

1396

Fixed

lib/Sema/SemaOpenMP.cpp

2726

I think we need all of them, because num_threads argument is not a constant, but an expression.

2746

Ok, I'll fix it.

[hfinkel]

lib/Sema/SemaOpenMP.cpp

2726

I'm not sure; I think this is worth testing. A similar issue same up with the patches for the assume_aligned attribute, and it turned out that only the ValueDependent check was needed. Note that sizeof(T), for example, is value-dependent, not type-dependent.

[ABataev]

lib/Sema/SemaOpenMP.cpp

2726

Here are some excerpts from the header:
/ template<int Size, char (&Chars)[Size]> struct meta_string - value depend bound of Chars
/ template<typename T> void add(T x) { return x;} - x is type dependent - it can be used as num_threads(x)
/ template<typename T> void add() { return sizeof(sizeof(T()));} - return is instantiation-dependent - I think this check can be excluded
/ template<typename F, typename ...Types> void forward(const F &f, Types &&...args) { f(static_cast<Types&&>(args)...); } - call to f contains unexpanded parameter pack - num_threads(f(static_cast<Types&&>(args)...))

[ABataev]

Update after review

[hfinkel]

lib/Sema/SemaOpenMP.cpp

2726

Ah, you're right. num_threads is not necessarily a compile-time constant, so the situation is different.

[hfinkel]

LGTM; thanks!

[ABataev]

Closed by commit rL219599 (authored by @ABataev).

include/clang/AST/StmtOpenMP.h

filtered_clause_iterator operator++(int) {
       ++(*this);
       return tmp;
     }
 
     bool operator!() { return Current == End; }
     operator bool() { return Current != End; }
   };
 
+  /// \brief Gets a single clause of the specified kind \a K associated with the
+  /// current directive iff there is only one clause of this kind (and assertion
+  /// is fired if there is more than one clause is associated with the
+  /// directive).
+  const OMPClause *getSingleClause(OpenMPClauseKind K) const;
+
   /// \brief Returns starting location of directive kind.
   SourceLocation getLocStart() const { return StartLoc; }
   /// \brief Returns ending location of directive.
   SourceLocation getLocEnd() const { return EndLoc; }
 
   /// \brief Set starting location of directive kind.
   ///
   /// \param Loc New starting location of directive.

lib/AST/Stmt.cpp

 
 OMPFlushClause *OMPFlushClause::CreateEmpty(const ASTContext &C, unsigned N) {
   void *Mem = C.Allocate(llvm::RoundUpToAlignment(sizeof(OMPFlushClause),
                                                   llvm::alignOf<Expr *>()) +
                          sizeof(Expr *) * N);
   return new (Mem) OMPFlushClause(N);
 }
 
+const OMPClause *
+OMPExecutableDirective::getSingleClause(OpenMPClauseKind K) const {
+  auto ClauseFilter =
+      [=](const OMPClause *C) -> bool { return C->getClauseKind() == K; };
+  OMPExecutableDirective::filtered_clause_iterator<decltype(ClauseFilter)> I(
+      clauses(), ClauseFilter);
+
+  if (I) {
+    auto Clause = *I;
+    assert(!++I && "There are at least 2 clauses of the specified kind");
+    return Clause;
+  }
+  return nullptr;
+}
+
 OMPParallelDirective *OMPParallelDirective::Create(
                                               const ASTContext &C,
                                               SourceLocation StartLoc,
                                               SourceLocation EndLoc,
                                               ArrayRef<OMPClause *> Clauses,
                                               Stmt *AssociatedStmt) {
   unsigned Size = llvm::RoundUpToAlignment(sizeof(OMPParallelDirective),
                                            llvm::alignOf<OMPClause *>());

lib/CodeGen/CGOpenMPRuntime.h

enum OpenMPRTLFunction {
     OMPRTL__kmpc_fork_call,
     // Call to __kmpc_int32 kmpc_global_thread_num(ident_t *loc);
     OMPRTL__kmpc_global_thread_num,
     // Call to void __kmpc_critical(ident_t *loc, kmp_int32 global_tid,
     // kmp_critical_name *crit);
     OMPRTL__kmpc_critical,
     // Call to void __kmpc_end_critical(ident_t *loc, kmp_int32 global_tid,
     // kmp_critical_name *crit);
-    OMPRTL__kmpc_end_critical
+    OMPRTL__kmpc_end_critical,
+    // Call to void __kmpc_push_num_threads(ident_t *loc, kmp_int32 global_tid,
+    // kmp_int32 num_threads);
+    OMPRTL__kmpc_push_num_threads
   };
 
 private:
   CodeGenModule &CGM;
   /// \brief Default const ident_t object used for initialization of all other
   /// ident_t objects.
   llvm::Constant *DefaultOpenMPPSource;
   /// \brief Map of flags and corrsponding default locations.
public:
   /// __kmpc_end_critical(ident_t *loc, kmp_int32 global_tid, kmp_critical_name
   /// * \a RegionLock)
   /// \param CGF Reference to current CodeGenFunction.
   /// \param RegionLock The lock object for critical region.
   /// \param Loc Location of the construct.
   virtual void EmitOMPCriticalRegionEnd(CodeGenFunction &CGF,
                                         llvm::Value *RegionLock,
                                         SourceLocation Loc);
+
+  /// \brief Emits call to void __kmpc_push_num_threads(ident_t *loc, kmp_int32
+  /// global_tid, kmp_int32 num_threads);
+  /// \param CGF Reference to current CodeGenFunction.
+  /// \param NumThreads An integer value of threads.
+  /// \param Loc Location of the construct.
+  virtual void EmitOMPNumThreadsClause(CodeGenFunction &CGF,
+                                       llvm::Value *NumThreads,
+                                       SourceLocation Loc);
 };
 } // namespace CodeGen
 } // namespace clang
 
 #endif

lib/CodeGen/CGOpenMPRuntime.cpp

case OMPRTL__kmpc_end_critical: {
     llvm::Type *TypeParams[] = {
         getIdentTyPointerTy(), CGM.Int32Ty,
         llvm::PointerType::getUnqual(KmpCriticalNameTy)};
     llvm::FunctionType *FnTy =
         llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
     RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_end_critical");
     break;
   }
+  case OMPRTL__kmpc_push_num_threads: {
+    // Build void __kmpc_push_num_threads(ident_t *loc, kmp_int32 global_tid,
+    // kmp_int32 num_threads)
+    llvm::Type *TypeParams[] = {getIdentTyPointerTy(), CGM.Int32Ty,
+                                CGM.Int32Ty};
+    llvm::FunctionType *FnTy =
+        llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
+    RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_push_num_threads");
+    break;
+  }
   }
   return RTLFn;
 }
 
 llvm::Value *CGOpenMPRuntime::GetCriticalRegionLock(StringRef CriticalName) {
   SmallString<256> Buffer;
   llvm::raw_svector_ostream Out(Buffer);
   Out << ".gomp_critical_user_" << CriticalName << ".var";
void CGOpenMPRuntime::EmitOMPCriticalRegionEnd(CodeGenFunction &CGF,
                                                SourceLocation Loc) {
   // Prepare other arguments and build a call to __kmpc_end_critical
   llvm::Value *Args[] = {EmitOpenMPUpdateLocation(CGF, Loc),
                          GetOpenMPGlobalThreadNum(CGF, Loc), RegionLock};
   auto RTLFn =
       CreateRuntimeFunction(CGOpenMPRuntime::OMPRTL__kmpc_end_critical);
   CGF.EmitRuntimeCall(RTLFn, Args);
 }
+
+void CGOpenMPRuntime::EmitOMPNumThreadsClause(CodeGenFunction &CGF,
+                                              llvm::Value *NumThreads,
+                                              SourceLocation Loc) {
+  // __kmpc_push_num_threads(&loc, global_tid, num_threads);
+  llvm::Value *Args[] = {
+      EmitOpenMPUpdateLocation(CGF, Loc), GetOpenMPGlobalThreadNum(CGF, Loc),
+      CGF.Builder.CreateIntCast(NumThreads, CGF.Int32Ty, /*isSigned*/ true)};
+  llvm::Constant *RTLFn = CGF.CGM.getOpenMPRuntime().CreateRuntimeFunction(
+      CGOpenMPRuntime::OMPRTL__kmpc_push_num_threads);
+  CGF.EmitRuntimeCall(RTLFn, Args);
+}
+

lib/CodeGen/CGStmtOpenMP.cpp

void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective &S) {
 
   llvm::Value *OutlinedFn;
   {
     CodeGenFunction CGF(CGM, true);
     CGCapturedStmtInfo CGInfo(*CS, CS->getCapturedRegionKind());
     CGF.CapturedStmtInfo = &CGInfo;
     OutlinedFn = CGF.GenerateCapturedStmtFunction(*CS);
   }
+  if (auto C = S.getSingleClause(/*K*/ OMPC_num_threads)) {
+    RunCleanupsScope NumThreadsScope(*this);
+    auto NumThreadsClause = cast<OMPNumThreadsClause>(C);
+    auto NumThreads = EmitScalarExpr(NumThreadsClause->getNumThreads(),
+                                     /*IgnoreResultAssign*/ true);
+    CGM.getOpenMPRuntime().EmitOMPNumThreadsClause(
+        *this, NumThreads, NumThreadsClause->getLocStart());
+  }
 
   // Build call __kmpc_fork_call(loc, 1, microtask, captured_struct/*context*/)
   llvm::Value *Args[] = {
       CGM.getOpenMPRuntime().EmitOpenMPUpdateLocation(*this, S.getLocStart()),
       Builder.getInt32(1), // Number of arguments after 'microtask' argument
       // (there is only one additional argument - 'context')
       Builder.CreateBitCast(OutlinedFn,
                             CGM.getOpenMPRuntime().getKmpc_MicroPointerTy()),

lib/Sema/SemaOpenMP.cpp

ExprResult Sema::PerformOpenMPImplicitIntegerConversion(SourceLocation Loc,
   return PerformContextualImplicitConversion(Loc, Op, ConvertDiagnoser);
 }
 
 OMPClause *Sema::ActOnOpenMPNumThreadsClause(Expr *NumThreads,
                                              SourceLocation StartLoc,
                                              SourceLocation LParenLoc,
                                              SourceLocation EndLoc) {
   Expr *ValExpr = NumThreads;
   if (!NumThreads->isValueDependent() && !NumThreads->isTypeDependent() &&
-      !NumThreads->isInstantiationDependent() &&
       !NumThreads->containsUnexpandedParameterPack()) {
     SourceLocation NumThreadsLoc = NumThreads->getLocStart();
     ExprResult Val =
         PerformOpenMPImplicitIntegerConversion(NumThreadsLoc, NumThreads);
     if (Val.isInvalid())
       return nullptr;
 
     ValExpr = Val.get();
 
     // OpenMP [2.5, Restrictions]
     //  The num_threads expression must evaluate to a positive integer value.
     llvm::APSInt Result;
     if (ValExpr->isIntegerConstantExpr(Result, Context) && Result.isSigned() &&
         !Result.isStrictlyPositive()) {
       Diag(NumThreadsLoc, diag::err_omp_negative_expression_in_clause)
           << "num_threads" << NumThreads->getSourceRange();
       return nullptr;
     }
   }
 
   return new (Context)
       OMPNumThreadsClause(ValExpr, StartLoc, LParenLoc, EndLoc);
 }
 
 ExprResult Sema::VerifyPositiveIntegerConstantInClause(Expr *E,
                                                        OpenMPClauseKind CKind) {
   if (!E)
     return ExprError();

test/OpenMP/parallel_num_threads_codegen.cpp

+// RUN: %clang_cc1 -verify -fopenmp=libiomp5 -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp=libiomp5 -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp=libiomp5 -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+// CHECK-DAG: [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
+// CHECK-DAG: [[S_TY:%.+]] = type { [[INTPTR_T_TY:i[0-9]+]], [[INTPTR_T_TY]], [[INTPTR_T_TY]] }
+// CHECK-DAG: [[STR:@.+]] = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00"
+// CHECK-DAG: [[DEF_LOC_2:@.+]] = private unnamed_addr constant [[IDENT_T_TY]] { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8]* [[STR]], i32 0, i32 0) }
+
+void foo();
+
+struct S {
+  intptr_t a, b, c;
+  S(intptr_t a) : a(a) {}
+  operator char() { return a; }
+  ~S() {}
+};
+
+template <typename T, int C>
+int tmain() {
+#pragma omp parallel num_threads(C)
+  foo();
+#pragma omp parallel num_threads(T(23))
+  foo();
+  return 0;
+}
+
+int main() {
+  S s(0);
+  char a = s;
+#pragma omp parallel num_threads(2)
+  foo();
+#pragma omp parallel num_threads(a)
+  foo();
+  return a + tmain<char, 5>() + tmain<S, 1>();
+}
+
+// CHECK:       define [[INT_TY:i[0-9]+]] @main(
+// CHECK-DAG:   [[S_ADDR:%.+]] = alloca [[S_TY]]
+// CHECK-DAG:   [[A_ADDR:%.+]] = alloca i8
+// CHECK-DAG:   [[GTID:%.+]] = call i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEF_LOC_2]])
+// CHECK-DAG:   call void [[S_TY_CONSTR:@.+]]([[S_TY]]* [[S_ADDR]], [[INTPTR_T_TY]] 0)
+// CHECK:       [[S_CHAR_OP:%.+]] = invoke{{.*}} i8 [[S_TY_CHAR_OP:@.+]]([[S_TY]]* [[S_ADDR]])
+// CHECK:       store i8 [[S_CHAR_OP]], i8* [[A_ADDR]]
+// CHECK:       call void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 2)
+// CHECK:       call void {{.*}}* @__kmpc_fork_call(
+// CHECK:       [[A_VAL:%.+]] = load i8* [[A_ADDR]]
+// CHECK:       [[RES:%.+]] = sext i8 [[A_VAL]] to i32
+// CHECK:       call void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 [[RES]])
+// CHECK:       call void {{.*}}* @__kmpc_fork_call(
+// CHECK:       invoke [[INT_TY]] [[TMAIN_CHAR_5:@.+]]()
+// CHECK:       invoke [[INT_TY]] [[TMAIN_S_1:@.+]]()
+// CHECK:       call void [[S_TY_DESTR:@.+]]([[S_TY]]* [[S_ADDR]])
+// CHECK:       ret [[INT_TY]]
+// CHECK:       }
+
+// CHECK:       define{{.*}} [[INT_TY]] [[TMAIN_CHAR_5]]()
+// CHECK:       [[GTID:%.+]] = call i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEF_LOC_2]])
+// CHECK:       call void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 5)
+// CHECK:       call void {{.*}}* @__kmpc_fork_call(
+// CHECK:       call void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 23)
+// CHECK:       call void {{.*}}* @__kmpc_fork_call(
+// CHECK:       ret [[INT_TY]] 0
+// CHECK-NEXT:  }
+
+// CHECK:       define{{.*}} [[INT_TY]] [[TMAIN_S_1]]()
+// CHECK:       [[GTID:%.+]] = call i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEF_LOC_2]])
+// CHECK:       call void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 1)
+// CHECK:       call void {{.*}}* @__kmpc_fork_call(
+// CHECK:       call void [[S_TY_CONSTR]]([[S_TY]]* [[S_TEMP:%.+]], [[INTPTR_T_TY]] 23)
+// CHECK:       [[S_CHAR_OP:%.+]] = invoke{{.*}} i8 [[S_TY_CHAR_OP]]([[S_TY]]* [[S_TEMP]])
+// CHECK:       [[RES:%.+]] = sext i8 [[S_CHAR_OP]] to i32
+// CHECK:       call void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 [[RES]])
+// CHECK:       call void [[S_TY_DESTR]]([[S_TY]]* [[S_TEMP]])
+// CHECK:       call void {{.*}}* @__kmpc_fork_call(
+// CHECK:       ret [[INT_TY]] 0
+// CHECK:       }
+
+#endif