[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/PlatformInterfaces.h:45
@@ +44,3 @@
+  explicit PlatformStreamHandle(
+      PlatformStreamExecutor *ParentExecutor = nullptr)
+      : ParentExecutor(ParentExecutor) {}
----------------
Why do we allow a null executor?

================
Comment at: streamexecutor/include/streamexecutor/PlatformInterfaces.h:57
@@ +56,3 @@
+private:
+  PlatformStreamExecutor *ParentExecutor;
+};
----------------
Not sold on the "parent" part of the name.  At best it seems redundant.  Would it be confusing if we just called it "executor"?

================
Comment at: streamexecutor/lib/StreamExecutor.cpp:35
@@ +34,3 @@
+  }
+  (*MaybePlatformStream)->setParentExecutor(PlatformExecutor);
+  return llvm::make_unique<Stream>(std::move(*MaybePlatformStream));
----------------
Hm, I think this should be the PE's responsibility, since otherwise anyone who calls PE::createStream will have to remember to do this.  We can assert it here if you want.


https://reviews.llvm.org/D23333