[Openmp-commits] [PATCH] D141227: [OpenMP][libomptarget] Implement memory lock/unlock API in NextGen plugins

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

In D141227#4034338 <https://reviews.llvm.org/D141227#4034338>, @tschuett wrote:

> https://github.com/openucx/ucx/blob/master/src/ucs/datastruct/pgtable.h super optimized page table for a similar use case.

The license doesn't look like something we could copy. If there is a paper or similar we could reimplement it I assume.



================
Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp:1767
 
+  Expected<void *> dataLockImpl(void *HstPtr, int64_t Size) override {
+    void *PinnedPtr = nullptr;
----------------
Brief documentation seems to be used elsewhere, add it here and below.


================
Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h:321
+  /// for async memory copies.
+  bool isHostPinnedBuffer(void *HstPtr, void **DevPtr = nullptr) const {
+    std::shared_lock<std::shared_mutex> Lock(Mutex);
----------------
jdoerfert wrote:
> tschuett wrote:
> > I would return `std::optional<void *>` to avoid passing mutable pointers.
> This makes more sense. Though, again, why boxing. Returning the devptr content avoids mutable arguments just fine 
And remove the const cast at the use site. Cast it here if necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141227