[Beignet] [PATCH 4/4] Refine clCreateContext and clCreateContextFromType function.

junyan.he at inbox.com junyan.he at inbox.com
Mon Oct 10 07:20:10 UTC 2016


From: Junyan He <junyan.he at intel.com>

1. Make the code logic more clear, fix some error check problems.
2. Add devices list check, we may have more than 1 device later.

Signed-off-by: Junyan He <junyan.he at intel.com>
---
 src/cl_api.c         | 106 ++-----------------------------
 src/cl_api_context.c |  89 ++++++++++++++++++++++++++
 src/cl_context.c     | 176 ++++++++++++++++++++++++++++++---------------------
 src/cl_context.h     |  31 ++++-----
 src/cl_device_id.c   |  36 +++++++++++
 src/cl_device_id.h   |   2 +
 6 files changed, 246 insertions(+), 194 deletions(-)

diff --git a/src/cl_api.c b/src/cl_api.c
index 1f45e88..945d6c1 100644
--- a/src/cl_api.c
+++ b/src/cl_api.c
@@ -67,31 +67,6 @@ typedef intptr_t cl_device_partition_property;
 	  return RET; \
 	} while(0)
 
-static cl_int
-cl_check_device_type(cl_device_type device_type)
-{
-  const cl_device_type valid =  CL_DEVICE_TYPE_GPU
-                              | CL_DEVICE_TYPE_CPU
-                              | CL_DEVICE_TYPE_ACCELERATOR
-                              | CL_DEVICE_TYPE_DEFAULT
-                              | CL_DEVICE_TYPE_CUSTOM;
-
-  if( (device_type & valid) == 0) {
-    return CL_INVALID_DEVICE_TYPE;
-  }
-  if(UNLIKELY(!(device_type & CL_DEVICE_TYPE_DEFAULT) && !(device_type & CL_DEVICE_TYPE_GPU)))
-    return CL_DEVICE_NOT_FOUND;
-
-  return CL_SUCCESS;
-}
-
-static cl_int
-cl_device_id_is_ok(const cl_device_id device)
-{
-  if(UNLIKELY(device == NULL)) return CL_FALSE;
-  return device != cl_get_gt_device() ? CL_FALSE : CL_TRUE;
-}
-
 cl_int
 clGetPlatformIDs(cl_uint          num_entries,
                  cl_platform_id * platforms,
@@ -112,7 +87,8 @@ clGetDeviceIDs(cl_platform_id platform,
                cl_device_id * devices,
                cl_uint *      num_devices)
 {
-  cl_int err = CL_SUCCESS;
+  const cl_device_type valid_type = CL_DEVICE_TYPE_GPU | CL_DEVICE_TYPE_CPU | CL_DEVICE_TYPE_ACCELERATOR |
+                                    CL_DEVICE_TYPE_DEFAULT | CL_DEVICE_TYPE_CUSTOM;
 
   /* Check parameter consistency */
   if (UNLIKELY(devices == NULL && num_devices == NULL))
@@ -121,10 +97,8 @@ clGetDeviceIDs(cl_platform_id platform,
     return CL_INVALID_PLATFORM;
   if (UNLIKELY(devices && num_entries == 0))
     return CL_INVALID_VALUE;
-
-  err = cl_check_device_type(device_type);
-  if(err != CL_SUCCESS)
-    return err;
+  if ((device_type & valid_type) == 0)
+    return CL_INVALID_DEVICE_TYPE;
 
   return cl_get_device_ids(platform,
                            device_type,
@@ -169,78 +143,6 @@ clReleaseDevice(cl_device_id device)
   return CL_SUCCESS;
 }
 
-cl_context
-clCreateContext(const cl_context_properties *  properties,
-                cl_uint                        num_devices,
-                const cl_device_id *           devices,
-                void (* pfn_notify) (const char*, const void*, size_t, void*),
-                void *                         user_data,
-                cl_int *                       errcode_ret)
-{
-  cl_int err = CL_SUCCESS;
-  cl_context context = NULL;
-
-  /* Assert parameters correctness */
-  INVALID_VALUE_IF (devices == NULL);
-  INVALID_VALUE_IF (num_devices == 0);
-  INVALID_VALUE_IF (pfn_notify == NULL && user_data != NULL);
-
-  /* Now check if the user is asking for the right device */
-  INVALID_DEVICE_IF (cl_device_id_is_ok(*devices) == CL_FALSE);
-
-  context = cl_create_context(properties,
-                           num_devices,
-                           devices,
-                           pfn_notify,
-                           user_data,
-                           &err);
-  initialize_env_var();
-error:
-  if (errcode_ret)
-    *errcode_ret = err;
-  return context;
-}
-
-cl_context
-clCreateContextFromType(const cl_context_properties *  properties,
-                        cl_device_type                 device_type,
-                        void (CL_CALLBACK *pfn_notify) (const char *, const void *, size_t, void *),
-                        void *                         user_data,
-                        cl_int *                       errcode_ret)
-{
-  cl_context context = NULL;
-  cl_int err = CL_SUCCESS;
-  cl_device_id devices[1];
-  cl_uint num_devices = 1;
-
-  INVALID_VALUE_IF (pfn_notify == NULL && user_data != NULL);
-
-  err = cl_check_device_type(device_type);
-  if(err != CL_SUCCESS) {
-    goto error;
-  }
-
-  err = cl_get_device_ids(NULL,
-                          device_type,
-                          1,
-                          &devices[0],
-                          &num_devices);
-  if (err != CL_SUCCESS) {
-    goto error;
-  }
-
-  context = cl_create_context(properties,
-                              num_devices,
-                              devices,
-                              pfn_notify,
-                              user_data,
-                              &err);
-error:
-  if (errcode_ret)
-    *errcode_ret = err;
-  return context;
-}
-
 cl_command_queue
 clCreateCommandQueue(cl_context                   context,
                      cl_device_id                 device,
diff --git a/src/cl_api_context.c b/src/cl_api_context.c
index f52c56a..2160950 100644
--- a/src/cl_api_context.c
+++ b/src/cl_api_context.c
@@ -17,6 +17,95 @@
  */
 
 #include "cl_context.h"
+#include "cl_device_id.h"
+#include "cl_alloc.h"
+
+cl_context
+clCreateContext(const cl_context_properties *properties,
+                cl_uint num_devices,
+                const cl_device_id *devices,
+                void (*pfn_notify)(const char *, const void *, size_t, void *),
+                void *user_data,
+                cl_int *errcode_ret)
+{
+  cl_int err = CL_SUCCESS;
+  cl_context context = NULL;
+
+  do {
+    /* Assure parameters correctness */
+    if (devices == NULL) {
+      err = CL_INVALID_VALUE;
+      break;
+    }
+
+    if (num_devices == 0) {
+      err = CL_INVALID_VALUE;
+      break;
+    }
+
+    if (pfn_notify == NULL && user_data != NULL) {
+      err = CL_INVALID_VALUE;
+      break;
+    }
+
+    err = cl_devices_list_check(num_devices, devices);
+    if (err != CL_SUCCESS)
+      break;
+
+    context = cl_create_context(properties, num_devices, devices, pfn_notify, user_data, &err);
+  } while (0);
+
+  if (errcode_ret)
+    *errcode_ret = err;
+  return context;
+}
+
+cl_context
+clCreateContextFromType(const cl_context_properties *properties,
+                        cl_device_type device_type,
+                        void(CL_CALLBACK *pfn_notify)(const char *, const void *, size_t, void *),
+                        void *user_data,
+                        cl_int *errcode_ret)
+{
+  cl_context context = NULL;
+  cl_int err = CL_SUCCESS;
+  cl_device_id *devices = NULL;
+  cl_uint num_devices = 0;
+  const cl_device_type valid_type = CL_DEVICE_TYPE_GPU | CL_DEVICE_TYPE_CPU | CL_DEVICE_TYPE_ACCELERATOR |
+                                    CL_DEVICE_TYPE_DEFAULT | CL_DEVICE_TYPE_CUSTOM;
+
+  do {
+    /* Assure parameters correctness */
+    if (pfn_notify == NULL && user_data != NULL) {
+      err = CL_INVALID_VALUE;
+      break;
+    }
+
+    if ((device_type & valid_type) == 0) {
+      err = CL_INVALID_DEVICE_TYPE;
+      break;
+    }
+
+    /* Get the devices num first. */
+    err = cl_get_device_ids(NULL, device_type, 0, NULL, &num_devices);
+    if (err != CL_SUCCESS)
+      break;
+
+    assert(num_devices > 0);
+    devices = cl_malloc(num_devices * sizeof(cl_device_id));
+    err = cl_get_device_ids(NULL, device_type, num_devices, &devices[0], &num_devices);
+    if (err != CL_SUCCESS)
+      break;
+
+    context = cl_create_context(properties, num_devices, devices, pfn_notify, user_data, &err);
+  } while (0);
+
+  if (devices)
+    cl_free(devices);
+  if (errcode_ret)
+    *errcode_ret = err;
+  return context;
+}
 
 cl_int
 clRetainContext(cl_context context)
diff --git a/src/cl_context.c b/src/cl_context.c
index 229ab96..4617f11 100644
--- a/src/cl_context.c
+++ b/src/cl_context.c
@@ -30,6 +30,7 @@
 #include "cl_khr_icd.h"
 #include "cl_kernel.h"
 #include "cl_program.h"
+#include "performance.h"
 
 #include "CL/cl.h"
 #include "CL/cl_gl.h"
@@ -167,80 +168,146 @@ cl_context_remove_program(cl_context ctx, cl_program program) {
   program->ctx = NULL;
 }
 
-
-#define CHECK(var) \
-  if (var) \
-    return CL_INVALID_PROPERTY; \
-  else \
-    var = 1;
-
-static cl_int
+LOCAL cl_int
 cl_context_properties_process(const cl_context_properties *prop,
                               struct _cl_context_prop *cl_props, cl_uint * prop_len)
 {
   int set_cl_context_platform = 0,
+      set_cl_context_interop_user_sync = 0,
       set_cl_gl_context_khr = 0,
       set_cl_egl_display_khr = 0,
       set_cl_glx_display_khr = 0,
       set_cl_wgl_hdc_khr = 0,
       set_cl_cgl_sharegroup_khr = 0;
-  cl_int err = CL_SUCCESS;
 
   cl_props->gl_type = CL_GL_NOSHARE;
   cl_props->platform_id = 0;
+  memset(cl_props, 0, sizeof(_cl_context_prop));
+  *prop_len = 0;
 
   if (prop == NULL)
-    goto exit;
-
+    return CL_SUCCESS;
 
+  /* All the properties can just be set once. */
   while(*prop) {
+    if (prop + 1 == NULL) {
+      return CL_INVALID_PROPERTY;
+    }
+
     switch (*prop) {
     case CL_CONTEXT_PLATFORM:
-      CHECK (set_cl_context_platform);
+      if (set_cl_context_platform) {
+        return CL_INVALID_PROPERTY;
+      }
+      set_cl_context_platform = 1;
       cl_props->platform_id = *(prop + 1);
-      if (UNLIKELY((cl_platform_id) cl_props->platform_id != cl_get_platform_default())) {
-        err = CL_INVALID_PLATFORM;
-        goto error;
+
+      /* We now just support one platform. */
+      if (UNLIKELY((cl_platform_id)cl_props->platform_id != cl_get_platform_default())) {
+        return CL_INVALID_PLATFORM;
       }
       break;
+
+    case CL_CONTEXT_INTEROP_USER_SYNC:
+      if (set_cl_context_interop_user_sync) {
+        return CL_INVALID_PROPERTY;
+      }
+      set_cl_context_interop_user_sync = 1;
+      cl_props->interop_user_sync = *(prop + 1);
+      break;
+
     case CL_GL_CONTEXT_KHR:
-      CHECK (set_cl_gl_context_khr);
+      if (set_cl_gl_context_khr || set_cl_cgl_sharegroup_khr) {
+        return CL_INVALID_PROPERTY;
+      }
+      set_cl_gl_context_khr = 1;
       cl_props->gl_context = *(prop + 1);
       break;
+
     case CL_EGL_DISPLAY_KHR:
-      CHECK (set_cl_egl_display_khr);
+      if (set_cl_cgl_sharegroup_khr || set_cl_egl_display_khr ||
+          set_cl_glx_display_khr || set_cl_wgl_hdc_khr) {
+        return CL_INVALID_PROPERTY;
+      }
       cl_props->gl_type = CL_GL_EGL_DISPLAY;
       cl_props->egl_display = *(prop + 1);
       break;
+
     case CL_GLX_DISPLAY_KHR:
-      CHECK (set_cl_glx_display_khr);
+      if (set_cl_cgl_sharegroup_khr || set_cl_egl_display_khr ||
+          set_cl_glx_display_khr || set_cl_wgl_hdc_khr) {
+        return CL_INVALID_PROPERTY;
+      }
       cl_props->gl_type = CL_GL_GLX_DISPLAY;
       cl_props->glx_display = *(prop + 1);
       break;
+
     case CL_WGL_HDC_KHR:
-      CHECK (set_cl_wgl_hdc_khr);
+      if (set_cl_cgl_sharegroup_khr || set_cl_egl_display_khr ||
+          set_cl_glx_display_khr || set_cl_wgl_hdc_khr) {
+        return CL_INVALID_PROPERTY;
+      }
       cl_props->gl_type = CL_GL_WGL_HDC;
       cl_props->wgl_hdc = *(prop + 1);
       break;
+
     case CL_CGL_SHAREGROUP_KHR:
-      CHECK (set_cl_cgl_sharegroup_khr);
+      if (set_cl_cgl_sharegroup_khr || set_cl_egl_display_khr ||
+          set_cl_glx_display_khr || set_cl_wgl_hdc_khr || set_cl_gl_context_khr) {
+        return CL_INVALID_PROPERTY;
+      }
       cl_props->gl_type = CL_GL_CGL_SHAREGROUP;
       cl_props->cgl_sharegroup = *(prop + 1);
       break;
     default:
-      err = CL_INVALID_PROPERTY;
-      goto error;
+      return CL_INVALID_PROPERTY;
     }
+
     prop += 2;
     *prop_len += 2;
   }
+
+  /* Add the last NULL pointer. */
   (*prop_len)++;
-exit:
-error:
-  return err;
+  return CL_SUCCESS;
 }
 
+static cl_context
+cl_context_new(cl_context_prop props, const cl_context_properties* properties, cl_uint prop_len)
+{
+  cl_context ctx = cl_calloc(1, sizeof(_cl_context));
+  if (ctx == NULL)
+    return NULL;
+
+  if (properties) {
+    ctx->prop_user = cl_calloc(prop_len, sizeof(cl_context_properties));
+    if (ctx->prop_user == NULL) {
+      cl_free(ctx);
+      return NULL;
+    }
+    memcpy(ctx->prop_user, properties, sizeof(cl_context_properties) * prop_len);
+  }
+  ctx->prop_len = prop_len;
+
+  ctx->drv = cl_driver_new(props);
+  if (ctx->drv == NULL) {
+    cl_free(ctx);
+    if (ctx->prop_user)
+      cl_free(ctx->prop_user);
 
+    return NULL;
+  }
+
+  CL_OBJECT_INIT_BASE(ctx, CL_OBJECT_CONTEXT_MAGIC);
+  list_init(&ctx->queues);
+  list_init(&ctx->mem_objects);
+  list_init(&ctx->samplers);
+  list_init(&ctx->events);
+  list_init(&ctx->programs);
+  ctx->queue_cookie = 1;
+  ctx->ver = cl_driver_get_ver(ctx->drv);
+  return ctx;
+}
 
 LOCAL cl_context
 cl_create_context(const cl_context_properties *  properties,
@@ -250,70 +317,33 @@ cl_create_context(const cl_context_properties *  properties,
                   void *                         user_data,
                   cl_int *                       errcode_ret)
 {
-  /* cl_platform_id platform = NULL; */
-  struct _cl_context_prop props;
   cl_context ctx = NULL;
-  cl_int err = CL_SUCCESS;
+  struct _cl_context_prop props;
   cl_uint prop_len = 0;
-  /* XXX */
-  FATAL_IF (num_devices != 1, "Only one device is supported");
 
-  /* Check that we are getting the right platform */
-  if (UNLIKELY(((err = cl_context_properties_process(properties, &props, &prop_len)) != CL_SUCCESS)))
-    goto error;
+  *errcode_ret = cl_context_properties_process(properties, &props, &prop_len);
+  if (*errcode_ret != CL_SUCCESS)
+    return NULL;
 
   /* We are good */
-  if (UNLIKELY((ctx = cl_context_new(&props)) == NULL)) {
-    err = CL_OUT_OF_HOST_MEMORY;
-    goto error;
+  ctx = cl_context_new(&props, properties, prop_len);
+  if (ctx == NULL) {
+    *errcode_ret = CL_OUT_OF_HOST_MEMORY;
+    return NULL;
   }
 
-  if(properties != NULL && prop_len > 0) {
-    TRY_ALLOC (ctx->prop_user, CALLOC_ARRAY(cl_context_properties, prop_len));
-    memcpy(ctx->prop_user, properties, sizeof(cl_context_properties)*prop_len);
-  }
-  ctx->prop_len = prop_len;
   /* Attach the device to the context */
+  assert(num_devices == 1); // TODO: multi devices later.
   ctx->device = *devices;
 
   /* Save the user callback and user data*/
   ctx->pfn_notify = pfn_notify;
   ctx->user_data = user_data;
-  cl_driver_set_atomic_flag(ctx->drv, ctx->device->atomic_test_result);
-
-exit:
-  if (errcode_ret != NULL)
-    *errcode_ret = err;
-  return ctx;
-error:
-  cl_context_delete(ctx);
-  ctx = NULL;
-  goto exit;
-}
-
-LOCAL cl_context
-cl_context_new(struct _cl_context_prop *props)
-{
-  cl_context ctx = NULL;
 
-  TRY_ALLOC_NO_ERR (ctx, CALLOC(struct _cl_context));
-  CL_OBJECT_INIT_BASE(ctx, CL_OBJECT_CONTEXT_MAGIC);
-  list_init(&ctx->queues);
-  list_init(&ctx->mem_objects);
-  list_init(&ctx->samplers);
-  list_init(&ctx->events);
-  list_init(&ctx->programs);
-  ctx->queue_cookie = 1;
-  TRY_ALLOC_NO_ERR (ctx->drv, cl_driver_new(props));
-  ctx->props = *props;
-  ctx->ver = cl_driver_get_ver(ctx->drv);
+  cl_driver_set_atomic_flag(ctx->drv, ctx->device->atomic_test_result);
 
-exit:
+  initialize_env_var();
   return ctx;
-error:
-  cl_context_delete(ctx);
-  ctx = NULL;
-  goto exit;
 }
 
 LOCAL void
diff --git a/src/cl_context.h b/src/cl_context.h
index b2903a7..e582fe2 100644
--- a/src/cl_context.h
+++ b/src/cl_context.h
@@ -82,23 +82,26 @@ enum _cl_internal_ker_type {
   CL_INTERNAL_KERNEL_MAX
 };
 
-struct _cl_context_prop {
+/* Include all the possible properties for context. */
+typedef struct _cl_context_prop {
   cl_context_properties platform_id;
-  enum _cl_gl_context_type gl_type;
+  cl_bool interop_user_sync;
+  /* Extensions. */
   cl_context_properties gl_context;
+  enum _cl_gl_context_type gl_type;
   union {
     cl_context_properties egl_display;
     cl_context_properties glx_display;
     cl_context_properties wgl_hdc;
     cl_context_properties cgl_sharegroup;
   };
-};
+} _cl_context_prop;
 
 #define IS_EGL_CONTEXT(ctx)  (ctx->props.gl_type == CL_GL_EGL_DISPLAY)
 #define EGL_DISP(ctx)   (EGLDisplay)(ctx->props.egl_display)
 #define EGL_CTX(ctx)    (EGLContext)(ctx->props.gl_context)
 /* Encapsulate the whole device */
-struct _cl_context {
+typedef struct _cl_context {
   _cl_base_object base;
   cl_driver drv;                    /* Handles HW or simulator */
   cl_device_id device;              /* All information about the GPU device */
@@ -129,7 +132,7 @@ struct _cl_context {
                                      /* User's callback when error occur in context */
   void *user_data;                   /* A pointer to user supplied data */
 
-};
+} _cl_context;
 
 #define CL_OBJECT_CONTEXT_MAGIC 0x20BBCADE993134AALL
 #define CL_OBJECT_IS_CONTEXT(obj) ((obj &&                           \
@@ -146,23 +149,13 @@ extern void cl_context_add_event(cl_context ctx, cl_event sampler);
 extern void cl_context_remove_event(cl_context ctx, cl_event sampler);
 extern void cl_context_add_program(cl_context ctx, cl_program program);
 extern void cl_context_remove_program(cl_context ctx, cl_program program);
-
-/* Implement OpenCL function */
-extern cl_context cl_create_context(const cl_context_properties*,
-                                    cl_uint,
-                                    const cl_device_id*,
+extern cl_context cl_create_context(const cl_context_properties*, cl_uint, const cl_device_id*,
                                     void (CL_CALLBACK * pfn_notify) (const char*, const void*, size_t, void*),
-                                    void *,
-                                    cl_int*);
-
-/* Allocate and initialize a context */
-extern cl_context cl_context_new(struct _cl_context_prop *);
-
-/* Destroy and deallocate a context */
+                                    void *, cl_int*);
 extern void cl_context_delete(cl_context);
-
-/* Increment the context reference counter */
 extern void cl_context_add_ref(cl_context);
+extern cl_int cl_context_properties_process(const cl_context_properties *prop,
+                                            struct _cl_context_prop *cl_props, cl_uint * prop_len);
 
 /* Create the command queue from the given context and device */
 extern cl_command_queue cl_context_create_queue(cl_context,
diff --git a/src/cl_device_id.c b/src/cl_device_id.c
index 61d2d78..268fa61 100644
--- a/src/cl_device_id.c
+++ b/src/cl_device_id.c
@@ -869,6 +869,10 @@ cl_get_device_ids(cl_platform_id    platform,
 {
   cl_device_id device;
 
+  // TODO: just GPU now.
+  if(!(device_type & (CL_DEVICE_TYPE_DEFAULT | CL_DEVICE_TYPE_GPU)))
+    return CL_DEVICE_NOT_FOUND;
+
   /* Do we have a usable device? */
   device = cl_get_gt_device();
   if (device) {
@@ -1510,3 +1514,35 @@ cl_get_kernel_subgroup_info(cl_kernel kernel,
 error:
   return err;
 }
+
+LOCAL cl_int
+cl_devices_list_check(cl_uint num_devices, const cl_device_id *devices)
+{
+  cl_uint i;
+
+  if (devices == NULL)
+    return CL_INVALID_DEVICE;
+
+  assert(num_devices > 0);
+  for (i = 0; i < num_devices; i++) {
+    if (!CL_OBJECT_IS_DEVICE(devices[i])) {
+      return CL_INVALID_DEVICE;
+    }
+
+    if (devices[i]->available == CL_FALSE) {
+      return CL_DEVICE_NOT_AVAILABLE;
+    }
+
+    // We now just support one platform.
+    if (devices[i]->platform != cl_get_platform_default()) {
+      return CL_INVALID_DEVICE;
+    }
+
+    // TODO: We now just support Gen Device.
+    if (devices[i] != cl_get_gt_device()) {
+      return CL_INVALID_DEVICE;
+    }
+  }
+
+  return CL_SUCCESS;
+}
diff --git a/src/cl_device_id.h b/src/cl_device_id.h
index 8cd55bb..12d6a6e 100644
--- a/src/cl_device_id.h
+++ b/src/cl_device_id.h
@@ -130,6 +130,8 @@ struct _cl_device_id {
          ((cl_base_object)obj)->magic == CL_OBJECT_DEVICE_MAGIC &&  \
          CL_OBJECT_GET_REF(obj) >= 1))
 
+extern cl_int cl_devices_list_check(cl_uint num_devices, const cl_device_id *devices);
+
 /* Get a device from the given platform */
 extern cl_int cl_get_device_ids(cl_platform_id    platform,
                                 cl_device_type    device_type,
-- 
2.7.4





More information about the Beignet mailing list