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

Jason Henline via Parallel_libs-commits parallel_libs-commits at lists.llvm.org
Wed Aug 24 09:55:05 PDT 2016


jhen added a comment.

In https://reviews.llvm.org/D23577#523872, @jlebar wrote:

> I think this is officially unwieldy.  Which I accept the blame for.  :)


Yeah, it might be a bit of a maintenance problem, but I do think it will be much nicer for the user to use, so I think it's been an overall improvement.


================
Comment at: streamexecutor/include/streamexecutor/Stream.h:353
@@ +352,3 @@
+  /// GlobalDeviceMemorySlice<T>) but the source and destination are
+  /// GlobalDeviceMemory<T> rather than a GlobalDeviceMemorySlice<T>.
+  template <typename T>
----------------
jlebar wrote:
> This review is getting so long I'm tempted to say it will be easier to check this in as-is and edit things like this later.  But I think it would be nice to consider whether there's a way to reduce the comment copypasta -- there's a lot of it, and I think it obscures the API.  I think in an earlier patch you did some doxygen "function group" magic?
> 
> Again, maybe better to save this for later.
I definitely think I can cut down the comments a lot and make the API more understandable in the process. As you suggested, I'll check this in now and then do that in the next CL.


https://reviews.llvm.org/D23577





More information about the Parallel_libs-commits mailing list