[Openmp-commits] [PATCH] D152054: [OpenMP] Codegen support for thread_limit on target directive

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9818
+      D.hasClausesOfKind<OMPInReductionClause>() ||
+      (CGM.getLangOpts().OpenMP >= 51 && D.getDirectiveKind() == OMPD_target &&
+       D.hasClausesOfKind<OMPThreadLimitClause>());
----------------
What if D is combined target directive, i.e. D.getDirectiveKind() is something like OMPD_target_teams, etc.?


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:5143-5148
+        S.getSingleClause<OMPThreadLimitClause>()) {
+      // Emit __kmpc_set_thread_limit() to set the thread_limit for the task
+      // enclosing this target region. This will indirectly set the thread_limit
+      // for every applicable construct within target region.
+      CGF.CGM.getOpenMPRuntime().emitThreadLimitClause(
+          CGF, S.getSingleClause<OMPThreadLimitClause>()->getThreadLimit(),
----------------
Avoid double call of S.getSingleClause<OMPThreadLimitClause>(), store in local variable call result.


================
Comment at: clang/test/OpenMP/target_codegen.cpp:849
 // OMP51: [[CE:%.*]] = load {{.*}} [[CEA]]
-// OMP51: call i32 @__tgt_target_kernel({{.*}}, i64 -1, i32 -1, i32 [[CE]],
+// OMP51: call ptr @__kmpc_omp_task_alloc({{.*@.omp_task_entry.*}})
+// OMP51: call i32 [[OMP_TASK_ENTRY]]
----------------
It requires extra resource consumption, can you try to avoid creating outer task, if possible?


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:809
+    return thread_limit;
+  else
+    return thread->th.th_current_task->td_icvs.thread_limit;
----------------
No need for else here


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152054/new/

https://reviews.llvm.org/D152054