[Parallel_libs-commits] [PATCH] D23211: [StreamExecutor] Add DeviceMemory and kernel arg packing

[James Price via Parallel_libs-commits]

jprice added a subscriber: jprice.

================
Comment at: streamexecutor/include/streamexecutor/DeviceMemory.h:48
@@ +47,3 @@
+/// This is effectively a pair consisting of an opaque handle and a buffer size
+/// in bytes. The opaque handle is a platform-independent handle to the actual
+/// memory that is allocated on the device.
----------------
platform-dependent?

================
Comment at: streamexecutor/include/streamexecutor/DeviceMemory.h:135
@@ +134,3 @@
+  /// The static method makeFromElementCount is provided for users of this class
+  /// because it's name makes the meaning of the size paramter clear.
+  DeviceMemory(void *Handle, size_t ElementCount)
----------------
s/it's/its/
s/paramter/parameter/

================
Comment at: streamexecutor/include/streamexecutor/DeviceMemory.h:137
@@ +136,3 @@
+  DeviceMemory(void *Handle, size_t ElementCount)
+      : DeviceMemoryBase(Handle, ElementCount * sizeof(ElemT)) {}
+};
----------------
The DeviceMemoryBase constructor has its Handle parameter qualified as `const` - is there a reason why this one isn't? Same for the `makeFromElementCount()` function that calls this.

================
Comment at: streamexecutor/include/streamexecutor/DeviceMemory.h:183
@@ +182,3 @@
+  /// The static method makeFromElementCount is provided for users of this class
+  /// because it's name makes the meaning of the size paramter clear.
+  explicit SharedDeviceMemory(size_t ElementCount)
----------------
s/it's/its/
s/paramter/parameter/


https://reviews.llvm.org/D23211