sandeepkosuri 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>()); ---------------- ABataev wrote: > What if D is combined target directive, i.e. D.getDirectiveKind() is > something like OMPD_target_teams, etc.? I will fix that, thanks for noticing. ================ 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(), ---------------- ABataev wrote: > Avoid double call of S.getSingleClause<OMPThreadLimitClause>(), store in > local variable call result. sure. ================ 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]] ---------------- ABataev wrote: > It requires extra resource consumption, can you try to avoid creating outer > task, if possible? I tried different ideas for making `thread_limit` work on `target`. I tried to reuse the existing implementation by replacing the directive to `target teams(1) thread_limit(x)` at parsing , sema and IR stages. I couldn't successfully implement any of them. So, I tried adding `num_threads` for all the parallel directives within `target`, and there were corner cases like parallel directives in a function which is called in target region, which were becoming tedious to handle. This method seem to encompass the idea of thread limit on `target` pretty well and also works... So I proceeded with this idea. ================ Comment at: openmp/runtime/src/kmp_ftn_entry.h:809 + return thread_limit; + else + return thread->th.th_current_task->td_icvs.thread_limit; ---------------- ABataev wrote: > No need for else here oops, I will fix that Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152054/new/ https://reviews.llvm.org/D152054 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits