[Openmp-commits] [PATCH] D112861: [OpenMP][DeviceRTL] Fixed an issue that causes hang in SU3

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added inline comments.


================
Comment at: openmp/libomptarget/DeviceRTL/src/Parallelism.cpp:129
+    // __kmpc_target_deinit may not hold.
+    synchronize::threadsAligned();
+
----------------
ye-luo wrote:
> tianshilei1992 wrote:
> > ye-luo wrote:
> > > This explanation seems plausible. Are the synchronization requirement of ASSERT and why being documented somewhere?
> > Have no idea.
> synchronize::threadsAligned() has been called at the end of above scope.
> What caused the out of  sync and needs this additional call?
> Why ASSERT requires synchronization.
> All needs explanation or documentation.
> "Synchronize all threads to make sure every thread exits the scope above;"
> Just sounds plausible
Making code slower by the addtional synchronization can also potentially skip triggering the hang. So it needs to be well understood and explained.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112861