[Beignet] [Patch V2] Fix an event status bug.

[Zhigang Gong]

LGTM, pushed, thanks.

On Thu, Jun 19, 2014 at 10:37:42PM +0800, Yang Rong wrote:
> If event status is an Error code, the status of events wait on this event also should set to Error code.
> 
> V2: should not execute the enqueue command wait on the event whose status is error.
> Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> ---
>  src/cl_driver.h         | 12 ++++++++----
>  src/cl_driver_defs.c    |  1 +
>  src/cl_event.c          | 18 ++++++++++++++----
>  src/intel/intel_gpgpu.c |  9 +++++++++
>  4 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/src/cl_driver.h b/src/cl_driver.h
> index d935235..2999eb7 100644
> --- a/src/cl_driver.h
> +++ b/src/cl_driver.h
> @@ -193,19 +193,23 @@ extern cl_gpgpu_flush_cb *cl_gpgpu_flush;
>  typedef cl_gpgpu_event (cl_gpgpu_event_new_cb)(cl_gpgpu);
>  extern cl_gpgpu_event_new_cb *cl_gpgpu_event_new;
>  
> -/* new a event for a batch buffer */
> +/* update the batch buffer of this event */
>  typedef int (cl_gpgpu_event_update_status_cb)(cl_gpgpu_event, int);
>  extern cl_gpgpu_event_update_status_cb *cl_gpgpu_event_update_status;
>  
> -/* new a event for a batch buffer */
> +/* pending flush the batch buffer of this event */
>  typedef void (cl_gpgpu_event_pending_cb)(cl_gpgpu, cl_gpgpu_event);
>  extern cl_gpgpu_event_pending_cb *cl_gpgpu_event_pending;
>  
> -/* new a event for a batch buffer */
> +/* flush the batch buffer of this event */
>  typedef void (cl_gpgpu_event_resume_cb)(cl_gpgpu_event);
>  extern cl_gpgpu_event_resume_cb *cl_gpgpu_event_resume;
>  
> -/* new a event for a batch buffer */
> +/* cancel exec batch buffer of this event */
> +typedef void (cl_gpgpu_event_cancel_cb)(cl_gpgpu_event);
> +extern cl_gpgpu_event_cancel_cb *cl_gpgpu_event_cancel;
> +
> +/* delete a gpgpu event */
>  typedef void (cl_gpgpu_event_delete_cb)(cl_gpgpu_event);
>  extern cl_gpgpu_event_delete_cb *cl_gpgpu_event_delete;
>  
> diff --git a/src/cl_driver_defs.c b/src/cl_driver_defs.c
> index 3a9b9ed..c9385c4 100644
> --- a/src/cl_driver_defs.c
> +++ b/src/cl_driver_defs.c
> @@ -81,6 +81,7 @@ LOCAL cl_gpgpu_event_new_cb *cl_gpgpu_event_new = NULL;
>  LOCAL cl_gpgpu_event_update_status_cb *cl_gpgpu_event_update_status = NULL;
>  LOCAL cl_gpgpu_event_pending_cb *cl_gpgpu_event_pending = NULL;
>  LOCAL cl_gpgpu_event_resume_cb *cl_gpgpu_event_resume = NULL;
> +LOCAL cl_gpgpu_event_cancel_cb *cl_gpgpu_event_cancel = NULL;
>  LOCAL cl_gpgpu_event_delete_cb *cl_gpgpu_event_delete = NULL;
>  LOCAL cl_gpgpu_event_get_exec_timestamp_cb *cl_gpgpu_event_get_exec_timestamp = NULL;
>  LOCAL cl_gpgpu_event_get_gpu_cur_timestamp_cb *cl_gpgpu_event_get_gpu_cur_timestamp = NULL;
> diff --git a/src/cl_event.c b/src/cl_event.c
> index 76d6760..db69721 100644
> --- a/src/cl_event.c
> +++ b/src/cl_event.c
> @@ -390,9 +390,15 @@ void cl_event_set_status(cl_event event, cl_int status)
>  
>    if(status <= CL_COMPLETE) {
>      if(event->enqueue_cb) {
> -      cl_enqueue_handle(event, &event->enqueue_cb->data);
> -      if(event->gpgpu_event)
> -        cl_gpgpu_event_update_status(event->gpgpu_event, 1);  //now set complet, need refine
> +      if(status == CL_COMPLETE) {
> +        cl_enqueue_handle(event, &event->enqueue_cb->data);
> +        if(event->gpgpu_event)
> +          cl_gpgpu_event_update_status(event->gpgpu_event, 1);  //now set complet, need refine
> +      } else {
> +        if(event->gpgpu_event)
> +          cl_gpgpu_event_cancel(event->gpgpu_event);  //Error cancel the enqueue
> +      }
> +
>        event->status = status;  //Change the event status after enqueue and befor unlock
>  
>        pthread_mutex_unlock(&event->ctx->event_lock);
> @@ -453,7 +459,11 @@ void cl_event_set_status(cl_event event, cl_int status)
>  
>      /* Call the pending operation */
>      evt = cb->event;
> -    cl_event_set_status(cb->event, CL_COMPLETE);
> +    /* TODO: if this event wait on several events, one event's
> +       status is error, the others is complete, what's the status
> +       of this event? Can't find the description in OpenCL spec.
> +       Simply update to latest finish wait event.*/
> +    cl_event_set_status(cb->event, status);
>      if(evt->emplict == CL_FALSE) {
>        cl_event_delete(evt);
>      }
> diff --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c
> index 6af6e40..7a95b41 100644
> --- a/src/intel/intel_gpgpu.c
> +++ b/src/intel/intel_gpgpu.c
> @@ -1177,6 +1177,14 @@ intel_gpgpu_event_resume(intel_event_t *event)
>  }
>  
>  static void
> +intel_gpgpu_event_cancel(intel_event_t *event)
> +{
> +  assert(event->batch);           //This command have pending.
> +  intel_batchbuffer_delete(event->batch);
> +  event->batch = NULL;
> +}
> +
> +static void
>  intel_gpgpu_event_delete(intel_event_t *event)
>  {
>    assert(event->batch == NULL);   //This command must have been flushed.
> @@ -1362,6 +1370,7 @@ intel_set_gpgpu_callbacks(int device_id)
>    cl_gpgpu_event_update_status = (cl_gpgpu_event_update_status_cb *)intel_gpgpu_event_update_status;
>    cl_gpgpu_event_pending = (cl_gpgpu_event_pending_cb *)intel_gpgpu_event_pending;
>    cl_gpgpu_event_resume = (cl_gpgpu_event_resume_cb *)intel_gpgpu_event_resume;
> +  cl_gpgpu_event_cancel = (cl_gpgpu_event_cancel_cb *)intel_gpgpu_event_cancel;
>    cl_gpgpu_event_delete = (cl_gpgpu_event_delete_cb *)intel_gpgpu_event_delete;
>    cl_gpgpu_event_get_exec_timestamp = (cl_gpgpu_event_get_exec_timestamp_cb *)intel_gpgpu_event_get_exec_timestamp;
>    cl_gpgpu_event_get_gpu_cur_timestamp = (cl_gpgpu_event_get_gpu_cur_timestamp_cb *)intel_gpgpu_event_get_gpu_cur_timestamp;
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet