[Openmp-commits] [PATCH] D104418: [OpenMP][Offloading] Fixed data race in libomptarget caused by async data movement

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 marked 6 inline comments as done.
tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/src/device.cpp:290
+        TargetPointer = nullptr;
+      }
+
----------------
jdoerfert wrote:
> Should we skip the event stuff if we failed?
It's different design philosophy. Either is fine to me, but arguably if a target supports event and related interfaces return error, I don't think we should continue (skip).


================
Comment at: openmp/libomptarget/src/omptarget.cpp:623
+        }
+      }
       // create shadow pointers for this entry
----------------
jdoerfert wrote:
> Worth a helper to avoid duplication.
Yes, this piece of code is kind of redundant. However, there are different locks intervened, if we abstract it to another helper function, it could make the code more difficult to understand because we could find mutex are locked in caller and unlocked in callee. I don't think it's a good practice. It fairly opens a door to misuse of locks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104418