[Parallel_libs-commits] [PATCH] D24114: [StreamExecutor] Dev handles in platform interface

[Jason Henline via Parallel_libs-commits]

jhen added inline comments.

================
Comment at: streamexecutor/include/streamexecutor/PlatformInterfaces.h:108
@@ -108,3 +107,3 @@
   virtual Error copyH2D(PlatformStreamHandle *S, const void *HostSrc,
-                        size_t SrcByteOffset, GlobalDeviceMemoryBase DeviceDst,
+                        size_t SrcByteOffset, const void *DeviceDstHandle,
                         size_t DstByteOffset, size_t ByteCount) {
----------------
jlebar wrote:
> Do we want the dst handles to be non-const void*s?  Or is the idea that the handle is always a const thing because you never write to it from host code?
Yeah, I wanted it to be const because I think that's more appropriate for an opaque handle. I'll keep it that way unless others want it changed.

================
Comment at: streamexecutor/include/streamexecutor/PlatformInterfaces.h:134
@@ -136,3 +133,3 @@
   /// Frees device memory previously allocated by allocateDeviceMemory.
-  virtual Error freeDeviceMemory(GlobalDeviceMemoryBase Memory) {
+  virtual Error freeDeviceMemory(const void *Handle) {
     return make_error("freeDeviceMemory not implemented for platform " +
----------------
jlebar wrote:
> Same for the handle we're freeing here.  (Although, as was pointed out to me recently, it's perfectly OK to call delete on a C++ const pointer.)
My idea is that the platform-specific implementation Will probably have to cast this handle to something else before using it anyway, so throwing on a const_cast too, if necessary, should seem natural.


https://reviews.llvm.org/D24114