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

[James Price via Parallel_libs-commits]

jprice added a comment.

Not sure what the protocol is for finding issues in patches that have already been committed - should I be posting to parallel_libs-dev instead?

This issue only showed up when I tried to actually write some code that uses all this stuff.


================
Comment at: parallel-libs/trunk/streamexecutor/include/streamexecutor/Executor.h:44
@@ +43,3 @@
+  Expected<GlobalDeviceMemory<T>> allocateDeviceMemory(size_t ElementCount) {
+    return PExecutor->allocateDeviceMemory(ElementCount * sizeof(T));
+  }
----------------
There's a mismatch between the return types of these two `allocateDeviceMemory` methods. The `PlatformExecutor` version returns a `GlobalDeviceMemoryBase`, whereas this one tries to return a `GlobalDeviceMemory<T>`.

This causes compilation failure if this `Executor::allocateDeviceMemory<T>()` method is actually used. Because this method is templated, it needs to actually be called somewhere for the issue to show up, and it doesn't appear to be covered in the unit tests.



Repository:
  rL LLVM

https://reviews.llvm.org/D23577