[Openmp-commits] [PATCH] D98349: [OpenMP][PoC][WIP] Device Rewrite, ICVs, ThreadState

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/deviceRTLs/common/include/ThreadState.h:53
+extern ThreadStateTy *ThreadStates[MAX_THREADS_PER_TEAM];
+#pragma omp allocate(ThreadStates) allocator(omp_pteam_mem_alloc)
+
----------------
I think we want a per warp array as we can reasonably synchronize then. The first draft did not have a preallocated array at all but it is hard to create it properly at runtime if you have no idea what threads are running.


================
Comment at: openmp/libomptarget/deviceRTLs/common/src/ICVs.cpp:100
+  ThreadStates[TId]->ICVState.nthreads_var = V;
+}
+
----------------
This is a scheme that needs to be generalized for other "setters".


================
Comment at: openmp/libomptarget/deviceRTLs/common/src/parallel.cu:260
 EXTERN int32_t __kmpc_global_thread_num(kmp_Ident *loc) {
-  int tid = GetLogicalThreadIdInBlock(checkSPMDMode(loc));
-  return GetOmpThreadId(tid, checkSPMDMode(loc));
+  return omp_get_thread_num();
 }
----------------
conceptually this should be the other way around, omp_get_thread_num calls this function. However, we should make the names actually match, so the impl. of omp_XXX is in __kmpc_XXX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98349