[Openmp-commits] [PATCH] D135037: [OpenMP] Eliminate the ThreadStates array in favor of indirection

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/DeviceRTL/include/State.h:112
 
-extern ThreadStateTy *ThreadStates[mapping::MaxThreadsPerTeam];
+extern ThreadStateTy **ThreadStates;
 #pragma omp allocate(ThreadStates) allocator(omp_pteam_mem_alloc)
----------------
tianshilei1992 wrote:
> Does it work if we just have `ThreadStateTy *ThreadStates`?
We could, yes. Requires to rewrite the code more :(


================
Comment at: openmp/libomptarget/DeviceRTL/src/State.cpp:269
+                     atomic::seq_cst, atomic::seq_cst))
+      memory::freeGlobal(ThreadStatesPtr,
+                         "Thread state array allocated multiple times");
----------------
tianshilei1992 wrote:
> Is it a valid state when it reaches to this point? If not, I think it'd better to have an assertion here.
State is valid, yes. I guess we can assert load returns non zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135037