[Parallel_libs-commits] [PATCH] D23138: [StreamExecutor] Add kernel types

[Justin Lebar via Parallel_libs-commits]

jlebar accepted this revision.
jlebar added a comment.
This revision is now accepted and ready to land.

> I'm sorry this patch has been so hard to read. :)


Oh, sorry if I gave you the wrong impression.  I do a lot of reviews, and your patches are consistently in the top quintile, both for readability and just in general.  This one is no exception!

Thank you for the fixes, this looks good to me (modulo the outstanding std::move issue).


================
Comment at: streamexecutor/include/streamexecutor/Kernel.h:24
@@ +23,3 @@
+///     void Launch(
+///       const TypedKernel<ParameterTs...> &Kernel, ParamterTs... Arguments);
+/// \endcode
----------------
Ah, I see.  Okay!

================
Comment at: streamexecutor/lib/unittests/KernelTest.cpp:86
@@ +85,3 @@
+
+  auto MaybeKernel = TypeParam::create(&MockExecutor, Spec);
+  EXPECT_TRUE(static_cast<bool>(MaybeKernel));
----------------
Okay, I get it now!  Thanks.  This is fine.  Like you say it's simple, but that's not a problem.


https://reviews.llvm.org/D23138