[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