[Openmp-commits] [PATCH] D142514: [OpenMP][libomptarget] Notify the plugins regarding new mapping/unmappings

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:797
+  auto PinnedPtrOrErr = PinnedAllocs.lockHostBuffer(
+      HstPtr, Size, /* check whether already locked */ true);
+  if (!PinnedPtrOrErr)
----------------
ye-luo wrote:
> jdoerfert wrote:
> > ye-luo wrote:
> > > I don't understand this part of code.
> > > The implementation actually lock the host buffer instead of just notifying.
> > While it is mapped, we want to keep it (by default) pinned. The escape hatch (env var) is missing though.
> > This allows us to avoid the pin/unpin on the data transfer back and intermediate updates.
> Unconditionally pinning mapped host memory should be an option to opt-in rather than default.
> If I just map 10 scalars, don't pin them, staging can be better.
> if users want fine-grain control, use the lock API. For really lazy users, give them the env var to opt-in.
Mapping scalars as firstprivate or as tofrom? The former would not go through this mechanism. The latter makes reasonable sense to pin the page, no?

I agree we need an env var, default is the question.


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

https://reviews.llvm.org/D142514