[Parallel_libs-commits] [PATCH] D23333: [StreamExecutor] Add basic Stream operations

[Justin Lebar via Parallel_libs-commits]

jlebar added inline comments.

================
Comment at: streamexecutor/include/streamexecutor/Stream.h:64
@@ +63,3 @@
+public:
+  Stream(StreamParent *Parent);
+
----------------
explicit?

================
Comment at: streamexecutor/include/streamexecutor/Stream.h:110
@@ +109,3 @@
+                        llvm::MutableArrayRef<T> HostDst, size_t ElementCount) {
+    if (ElementCount > DeviceSrc.getElementCount()) {
+      setError("copying too many elements, " + llvm::Twine(ElementCount) +
----------------
LLVM style is to leave off curly braces where possible.

================
Comment at: streamexecutor/include/streamexecutor/Stream.h:149
@@ +148,3 @@
+      thenRawMemcpyH2D(HostSrc.data(), DeviceDst, ElementCount * sizeof(T));
+    }
+    return *this;
----------------
Leave off curly braces?  (I know, it's awful, but consistency is good too...)  Same below.

================
Comment at: streamexecutor/lib/Stream.cpp:51
@@ +50,3 @@
+  return *this;
+}
+
----------------
I wonder if we need any of these `*raw*` functions anymore, now that they're just forwarding to the parent.


https://reviews.llvm.org/D23333