[Mesa-dev] [PATCH 2/2] st/clover: Always flush the queue when waiting on an hard_event

[Francisco Jerez]

Niels Ole Salscheider <niels_ole at salscheider-online.de> writes:

> The OpenCL spec says:
> "Any blocking commands queued in a command-queue and clReleaseCommandQueue
> perform an implicit flush of the command-queue. These blocking commands are
> [...] or clWaitForEvents."
>

Are you sure we want to do this just to please the spec?  The spec is
indeed very clear that clWaitForEvents should trigger a flush, but the
meaning of flushing is much less clear about the subset of queued
commands that a flush is supposed to flush and whether the
implementation is allowed to cache and lump commands together across
multiple flushes.

In my understanding we are allowed to do that as long as the user has no
reasonable way to tell the difference.  AFAICT the only cases where our
behavior might break user assumptions imply that the user is misusing
clGetEventInfo to poll an event *and* that the user is relying on the
non-intuitive assumption that waiting for an event ev0 will flush a
later event ev1 implicitly.

Both seem very unlikely to me, and implementing the spec strictly means
that we're going to punish all well-behaving applications by flushing
more often than we need -- Flushing can be quite a heavyweight operation
and I think it's something we should attempt to minimize.

Do you have any example of a real world application that relies on this?
Or at least some reasonable use case?

Thanks.

PS: I'm sorry that it took so long for me to review this series,
    sometimes I overlook mails that aren't specifically addressed to me,
    this should be far less likely if you add me to the CC list.

> Flushing the queue unconditionally also helps to actually clear the
> queued_events list of the queue object.
>
> Signed-off-by: Niels Ole Salscheider <niels_ole at salscheider-online.de>
> ---
>  src/gallium/state_trackers/clover/core/event.cpp | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/core/event.cpp b/src/gallium/state_trackers/clover/core/event.cpp
> index cbb97bf..8b5acd0 100644
> --- a/src/gallium/state_trackers/clover/core/event.cpp
> +++ b/src/gallium/state_trackers/clover/core/event.cpp
> @@ -153,8 +153,7 @@ void
>  hard_event::wait() const {
>     pipe_screen *screen = queue()->dev.pipe;
>  
> -   if (status() == CL_QUEUED)
> -      queue()->flush();
> +   queue()->flush();
>  
>     if (!__fence ||
>         !screen->fence_finish(screen, __fence, PIPE_TIMEOUT_INFINITE))
> -- 
> 1.8.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130926/c26fc067/attachment.pgp>