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

[Guilherme Valarini via Phabricator via Openmp-commits]

gValarini added a comment.

In D104418#2825161 <https://reviews.llvm.org/D104418#2825161>, @tianshilei1992 wrote:

> Or actually we can pass the update decision to `getOrAllocTgtPtr` such that we don't have to deal with lock(s) across functions.

This could be a nicer solution alongside a proper renaming of the `getOrAllocTgtPtr` to reflect its multiple responsibilities (if you go forward with it, of course). Just two comments though:

1. It would be good to check if other "users" of the de `DeviceTy` wouldn't need such synchronization mechanism. If so, they could use the same changes introduced by https://reviews.llvm.org/D104382 (`getTgtPtrBegin` used by `targetDataEnd` is the only example that I can think of);
2. Maybe letting the lifetime of `TargetPointerResultTy` instances determine for how long an entry is locked is the best future-proof solution. I am thinking about future changes that could be done to the `targetDataBegin` function (and maybe other ones) that could use the additional synchronization. But this is just a thought though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104418