[Openmp-commits] [PATCH] D117627: [OpenMP] Introduce an environment variable to disable atomic map clauses

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/src/device.cpp:23
 
+int HostDataToTargetTy::addEventIfNecessaryAndUnlock(
+    DeviceTy &Device, AsyncInfoTy &AsyncInfo) const {
----------------
This is arguably not a good practice to handle locks. We have `Entry->lock()` in one place and then nothing. For sure the function name `addEventIfNecessaryAndUnlock` contains "unlock", but it's still too vague because locks are usually used by pair or guarded by scope. Although I agree the code looks more neat, I don't think it's worth to do it in exchange of ambiguity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117627