[Openmp-commits] [PATCH] D104418: [OpenMP][Offloading] Fixed data race in libomptarget caused by async data movement
Shilei Tian via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Mon Dec 27 20:32:59 PST 2021
tianshilei1992 marked 6 inline comments as done.
tianshilei1992 added inline comments.
Comment at: openmp/libomptarget/src/device.cpp:290
+ TargetPointer = nullptr;
> 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
> 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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the Openmp-commits