[Openmp-commits] [PATCH] D80649: [OpenMP] Improve D2D memcpy to use more efficient driver API

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added reviewers: AndreyChurbanov, grokos.
jdoerfert added a comment.

Some high-level comment. I haven't read the main logic part in depth yet but it looks reasonable to me. Others should chime in too.



================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:788
+
+  MemcpyDtoD:
+    Err = cuMemcpyDtoDAsync((CUdeviceptr)DstPtr, (CUdeviceptr)SrcPtr, Size,
----------------
Any reason not to make this a function and call it?


================
Comment at: openmp/libomptarget/src/api.cpp:178
+    if (rc == OFFLOAD_SUCCESS)
+      return OFFLOAD_SUCCESS;
     void *buffer = malloc(length);
----------------
Please move this into the conditional.


================
Comment at: openmp/libomptarget/src/device.cpp:400
+  return RTL->RTLName == OtherDevice.RTL->RTLName;
+}
+
----------------
We use the name? I somehow feel uneasy about this. Don't we have some form of ID?


================
Comment at: openmp/libomptarget/src/device.h:160
 
+  bool isExchangable(const DeviceTy& OtherDevice);
+
----------------
Documentation please.


================
Comment at: openmp/libomptarget/src/device.h:186
+  int32_t data_exchange(void *SrcPtr, DeviceTy DstDev, void *DstPtr,
+                        int64_t Size, __tgt_async_info *AsyncInfoPtr);
 
----------------
Documentation please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80649