[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;
+      }
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.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list