[Parallel_libs-commits] [PATCH] D23577: [StreamExecutor] Executor add synchronous methods

[Jason Henline via Parallel_libs-commits]

jhen added inline comments.

================
Comment at: streamexecutor/include/streamexecutor/Executor.h:48
@@ +47,3 @@
+  /// Frees memory previously allocated with allocateDeviceMemory.
+  template <typename T> Error freeDeviceMemory(GlobalDeviceMemory<T> *Memory) {
+    return PExecutor->freeDeviceMemory(Memory);
----------------
The thing I would really miss from the current interface is the way the user has to specify a `DeviceMemory` as a copy destination by taking its address before passing it as an argument to the copy function. I like how this makes it hard for the user to accidentally pass the source as the destination and vice versa. Is there a good way to keep this nice feature and match the model of `DeviceMemory` as a pointer more closely?

================
Comment at: streamexecutor/include/streamexecutor/Executor.h:110
@@ +109,3 @@
+  Error synchronousMemcpyD2H(const GlobalDeviceMemory<T> &DeviceSrc,
+                             llvm::MutableArrayRef<T> HostDst) {
+    return synchronousMemcpyD2H(DeviceSrc, HostDst,
----------------
Okay, that sounds like a good reason to me.


https://reviews.llvm.org/D23577