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

Shilei Tian via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Jan 18 17:25:24 PST 2022


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



More information about the Openmp-commits mailing list