[Parallel_libs-commits] [PATCH] D24063: [StreamExecutor] Add Stream::blockHostUntilDone

[Justin Lebar via Parallel_libs-commits]

jlebar added inline comments.

================
Comment at: streamexecutor/include/streamexecutor/Stream.h:87
@@ +86,3 @@
+  // Returns the result of getStatus() after the Stream work completes.
+  Error blockHostUntilDone() {
+    setError(PDevice->blockHostUntilDone(ThePlatformStream.get()));
----------------
jhen wrote:
> jlebar wrote:
> > Would "thenBlockHostUntilDone" be more consistent with the other names?  Or are you intentionally being inconsistent to signal that this is a different sort of thing (e.g. it doesn't return a Stream&)?
> Right, I'm being intentionally inconsistent for just the reason you gave.
> 
> Side note: things are kind of funny now because there is no `Stream::init` method. It used to be `S->init().thenA().thenB().block()`, now it's `S->thenA()->thenB().block()`. I almost want to change the names so it's `S->aThen().bThen().block()`, but of course that doesn't read well because the parameter list is after the "then".
I was wondering about that...

Of course even before you could do something like

  Stream *s = ...;
  s->init()->thenA().thenB().block();
  s->thenC().thenD().block();

Right?  That is, we only init the stream once.  But I guess that's a less-common use-case?

Should we encourage people to treat streams as ephemeral?

  Device.getStream()->thenA().thenB().thenC();

That doesn't scan so bad.

While we're thinking about it, it's also weird that you have to switch between `.` and `->`.  Although I'm not sure that returning a `Stream*` would be an improvement.  Maybe...


Repository:
  rL LLVM

https://reviews.llvm.org/D24063