[Beignet] [PATCH v2] GetEventProfilingInfo could query all event status.

[Yang, Rong R]

Both patches has been pushed, and correct xionghu's commit log.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Tuesday, August 4, 2015 13:28
> To: Luo, Xionghu
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH v2] GetEventProfilingInfo could query all event
> status.
> 
> 
> The spec only says if event is not in CL_COMPLETE status, it will get
> CL_PROFILING_INFO_NOT_AVAILABLE. It doesn't mean a clFinish() should
> update all events status. According to the spec, clFinish() only need to make
> sure the previously enqueued task has been processed and completed.
> 
> Form my point of view, if the application wants to call
> clGetEventProfilingInfo() to get an event's profiling information, the
> application should call
> clWaitForEvents() firstly. But given the fact that the specification is not very
> clear for whether clFinish() should update all events' status and some
> applications are really believe all events has been also updated after the
> clFinish() call. We may make a work around in Beignet to make those
> applications happy. Your patch is one solution and another may be better
> solution may be to add cl_event_update_status(event, 0) when calling to
> clGetEventProfilingInfo(). The patch is as below,
> 
> From a5a1b3f372d17f26cc20fba078490b61614f07e5 Mon Sep 17 00:00:00 2001
> From: Zhigang Gong <zhigang.gong at intel.com>
> Date: Tue, 4 Aug 2015 13:21:27 +0800
> Subject: [PATCH] runtime: always try to update event status in
> clGetEventProfilingInfo().
> 
> Some applications forgot to call clWaitForEvents() before calling to
> clGetEventProfilingInfo(). Let's update the event's status here.
> 
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
>  src/cl_api.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/cl_api.c b/src/cl_api.c index 69eb0bc..5c9b250 100644
> --- a/src/cl_api.c
> +++ b/src/cl_api.c
> @@ -1468,6 +1468,7 @@ clGetEventProfilingInfo(cl_event             event,
>    cl_ulong ret_val;
> 
>    CHECK_EVENT(event);
> +  cl_event_update_status(event, 0);
> 
>    if (event->type == CL_COMMAND_USER ||
>        !(event->queue->props & CL_QUEUE_PROFILING_ENABLE) ||
> --
> 1.9.1
> 
> 
> This way we can avoid unecessary event updating in clFinish() call.
> The overhead is only introduced when the application calling
> clGetEventProfilingInfo() and we do not need busy wait for the event
> updating here.
> 
> Thanks,
> Zhigang Gong.
> 
> On Tue, Aug 04, 2015 at 01:11:10PM +0800, xionghu.luo at intel.com wrote:
> > From: Luo Xionghu <xionghu.luo at intel.com>
> >
> > this could fix the shoc project reported
> > CL_PROFILING_INFO_NOT_AVAILABLE issue.
> > https://github.com/vetter/shoc/issues/47
> > v2: per spec, the GetEventProfilingInfo need return
> > CL_PROFILING_INFO_NOT_AVAILABLE if the event is not CL_COMPLETE,
> so
> > the event should be updated in clFinish.
> >
> > Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
> > ---
> >  src/cl_command_queue.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/cl_command_queue.c b/src/cl_command_queue.c index
> > 4e4ebfb..4b92311 100644
> > --- a/src/cl_command_queue.c
> > +++ b/src/cl_command_queue.c
> > @@ -276,6 +276,9 @@ LOCAL cl_int
> >  cl_command_queue_finish(cl_command_queue queue)  {
> >    cl_gpgpu_sync(cl_get_thread_batch_buf(queue));
> > +  cl_event last_event = get_last_event(queue);  if (last_event)
> > +    cl_event_update_status(last_event, 1);
> >    return CL_SUCCESS;
> >  }
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet