[Beignet] [PATCH 1/2] runtime: Use cl_get_platform_default to replace global value.

junyan.he at inbox.com junyan.he at inbox.com
Fri Jul 3 03:24:50 PDT 2015


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

The init order of the intel_platform and the intel_extension is
somehow not clear. When some API such as clGetDeviceIDs can pass
NULL as cl_platform_id, we just use the global value intel_platform
as the default but do not care about the init state of the extension.
The init of the extension may be done when the cl device is created.
This is OK if the paltform and the device have the same extensions.
But now because of the fp16, they are not always the same.
Use cl_get_platform_default to replace the global value to ensure that
when default platform is available, the extension is also inited.

Signed-off-by: Junyan He <junyan.he at linux.intel.com>
---
 src/cl_api.c         |  6 +++---
 src/cl_context.c     |  2 +-
 src/cl_device_id.c   | 55 +++++++++++++++++++++-------------------------------
 src/cl_extensions.c  | 38 +++++++++++++++++-------------------
 src/cl_platform_id.c | 28 +++++++++++++++++---------
 src/cl_platform_id.h |  4 ++--
 6 files changed, 65 insertions(+), 68 deletions(-)

diff --git a/src/cl_api.c b/src/cl_api.c
index 3e72deb..1ba775f 100644
--- a/src/cl_api.c
+++ b/src/cl_api.c
@@ -195,7 +195,7 @@ clGetPlatformInfo(cl_platform_id    platform,
                   size_t *          param_value_size_ret)
 {
   /* Only one platform. This is easy */
-  if (UNLIKELY(platform != NULL && platform != intel_platform))
+  if (UNLIKELY(platform != NULL && platform != cl_get_platform_default()))
     return CL_INVALID_PLATFORM;
 
   return cl_get_platform_info(platform,
@@ -217,7 +217,7 @@ clGetDeviceIDs(cl_platform_id platform,
   /* Check parameter consistency */
   if (UNLIKELY(devices == NULL && num_devices == NULL))
     return CL_INVALID_VALUE;
-  if (UNLIKELY(platform && platform != intel_platform))
+  if (UNLIKELY(platform && platform != cl_get_platform_default()))
     return CL_INVALID_PLATFORM;
   if (UNLIKELY(devices && num_entries == 0))
     return CL_INVALID_VALUE;
@@ -3193,7 +3193,7 @@ void*
 clGetExtensionFunctionAddressForPlatform(cl_platform_id platform,
                               const char *func_name)
 {
-  if (UNLIKELY(platform != NULL && platform != intel_platform))
+  if (UNLIKELY(platform != NULL && platform != cl_get_platform_default()))
     return NULL;
   return internal_clGetExtensionFunctionAddress(func_name);
 }
diff --git a/src/cl_context.c b/src/cl_context.c
index 773f545..45064ad 100644
--- a/src/cl_context.c
+++ b/src/cl_context.c
@@ -68,7 +68,7 @@ cl_context_properties_process(const cl_context_properties *prop,
     case CL_CONTEXT_PLATFORM:
       CHECK (set_cl_context_platform);
       cl_props->platform_id = *(prop + 1);
-      if (UNLIKELY((cl_platform_id) cl_props->platform_id != intel_platform)) {
+      if (UNLIKELY((cl_platform_id) cl_props->platform_id != cl_get_platform_default())) {
         err = CL_INVALID_PLATFORM;
         goto error;
       }
diff --git a/src/cl_device_id.c b/src/cl_device_id.c
index 9f03027..7956646 100644
--- a/src/cl_device_id.c
+++ b/src/cl_device_id.c
@@ -351,7 +351,7 @@ cl_get_gt_device(void)
       DECL_INFO_STRING(has_break, intel_hsw_gt3_device, name, "Intel(R) HD Graphics Haswell CRW GT3 reserved");
 has_break:
       device->vendor_id = device_id;
-      device->platform = intel_platform;
+      device->platform = cl_get_platform_default();
       ret = device;
       break;
 
@@ -363,7 +363,7 @@ has_break:
       DECL_INFO_STRING(ivb_gt1_break, intel_ivb_gt1_device, name, "Intel(R) HD Graphics IvyBridge S GT1");
 ivb_gt1_break:
       intel_ivb_gt1_device.vendor_id = device_id;
-      intel_ivb_gt1_device.platform = intel_platform;
+      intel_ivb_gt1_device.platform = cl_get_platform_default();
       ret = &intel_ivb_gt1_device;
       break;
 
@@ -375,7 +375,7 @@ ivb_gt1_break:
       DECL_INFO_STRING(ivb_gt2_break, intel_ivb_gt2_device, name, "Intel(R) HD Graphics IvyBridge S GT2");
 ivb_gt2_break:
       intel_ivb_gt2_device.vendor_id = device_id;
-      intel_ivb_gt2_device.platform = intel_platform;
+      intel_ivb_gt2_device.platform = cl_get_platform_default();
       ret = &intel_ivb_gt2_device;
       break;
 
@@ -383,7 +383,7 @@ ivb_gt2_break:
       DECL_INFO_STRING(baytrail_t_device_break, intel_baytrail_t_device, name, "Intel(R) HD Graphics Bay Trail-T");
 baytrail_t_device_break:
       intel_baytrail_t_device.vendor_id = device_id;
-      intel_baytrail_t_device.platform = intel_platform;
+      intel_baytrail_t_device.platform = cl_get_platform_default();
       ret = &intel_baytrail_t_device;
       break;
 
@@ -399,9 +399,9 @@ baytrail_t_device_break:
       DECL_INFO_STRING(brw_gt1_break, intel_brw_gt1_device, name, "Intel(R) HD Graphics BroadWell ULX GT1");
 brw_gt1_break:
       /* For Gen8 and later, half float is suppported and we will enable cl_khr_fp16. */
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_brw_gt1_device.vendor_id = device_id;
-      intel_brw_gt1_device.platform = intel_platform;
+      intel_brw_gt1_device.platform = cl_get_platform_default();
       ret = &intel_brw_gt1_device;
       break;
 
@@ -416,9 +416,9 @@ brw_gt1_break:
     case PCI_CHIP_BROADWLL_U_GT2:
       DECL_INFO_STRING(brw_gt2_break, intel_brw_gt2_device, name, "Intel(R) HD Graphics BroadWell ULX GT2");
 brw_gt2_break:
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_brw_gt2_device.vendor_id = device_id;
-      intel_brw_gt2_device.platform = intel_platform;
+      intel_brw_gt2_device.platform = cl_get_platform_default();
       ret = &intel_brw_gt2_device;
       break;
 
@@ -433,9 +433,9 @@ brw_gt2_break:
     case PCI_CHIP_BROADWLL_U_GT3:
       DECL_INFO_STRING(brw_gt3_break, intel_brw_gt3_device, name, "Intel(R) HD Graphics BroadWell ULX GT2");
 brw_gt3_break:
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_brw_gt3_device.vendor_id = device_id;
-      intel_brw_gt3_device.platform = intel_platform;
+      intel_brw_gt3_device.platform = cl_get_platform_default();
       ret = &intel_brw_gt3_device;
       break;
 
@@ -445,9 +445,9 @@ brw_gt3_break:
     case PCI_CHIP_CHV_3:
       DECL_INFO_STRING(chv_break, intel_chv_device, name, "Intel(R) HD Graphics Cherryview");
 chv_break:
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_chv_device.vendor_id = device_id;
-      intel_chv_device.platform = intel_platform;
+      intel_chv_device.platform = cl_get_platform_default();
       ret = &intel_chv_device;
       break;
 
@@ -463,9 +463,9 @@ chv_break:
     case PCI_CHIP_SKYLAKE_SRV_GT1:
       DECL_INFO_STRING(skl_gt1_break, intel_skl_gt1_device, name, "Intel(R) HD Graphics Skylake Server GT1");
 skl_gt1_break:
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_skl_gt1_device.vendor_id = device_id;
-      intel_skl_gt1_device.platform = intel_platform;
+      intel_skl_gt1_device.platform = cl_get_platform_default();
       ret = &intel_skl_gt1_device;
       break;
 
@@ -482,9 +482,9 @@ skl_gt1_break:
     case PCI_CHIP_SKYLAKE_SRV_GT2:
       DECL_INFO_STRING(skl_gt2_break, intel_skl_gt2_device, name, "Intel(R) HD Graphics Skylake Server GT2");
 skl_gt2_break:
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_skl_gt2_device.vendor_id = device_id;
-      intel_skl_gt2_device.platform = intel_platform;
+      intel_skl_gt2_device.platform = cl_get_platform_default();
       ret = &intel_skl_gt2_device;
       break;
 
@@ -495,9 +495,9 @@ skl_gt2_break:
     case PCI_CHIP_SKYLAKE_SRV_GT3:
       DECL_INFO_STRING(skl_gt3_break, intel_skl_gt3_device, name, "Intel(R) HD Graphics Skylake Server GT3");
 skl_gt3_break:
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_skl_gt3_device.vendor_id = device_id;
-      intel_skl_gt3_device.platform = intel_platform;
+      intel_skl_gt3_device.platform = cl_get_platform_default();
       ret = &intel_skl_gt3_device;
       break;
 
@@ -506,9 +506,9 @@ skl_gt3_break:
     case PCI_CHIP_SKYLAKE_SRV_GT4:
       DECL_INFO_STRING(skl_gt4_break, intel_skl_gt4_device, name, "Intel(R) HD Graphics Skylake Server GT4");
 skl_gt4_break:
-      cl_intel_platform_enable_fp16_extension(intel_platform);
+      cl_intel_platform_enable_fp16_extension(cl_get_platform_default());
       intel_skl_gt4_device.vendor_id = device_id;
-      intel_skl_gt4_device.platform = intel_platform;
+      intel_skl_gt4_device.platform = cl_get_platform_default();
       ret = &intel_skl_gt4_device;
       break;
 
@@ -636,17 +636,6 @@ cl_get_device_ids(cl_platform_id    platform,
 {
   cl_device_id device;
 
-  /* Spec allow platform to be NULL, and If platform
-     is NULL, the behavior is implementation-defined.
-     We can not init the device before platform init. */
-  if (!platform) {
-    if (num_devices)
-      *num_devices = 0;
-    if (devices)
-      *devices = 0;
-    return CL_DEVICE_NOT_FOUND;
-  }
-
   /* Do we have a usable device? */
   device = cl_get_gt_device();
   if (device) {
@@ -678,8 +667,8 @@ cl_get_device_ids(cl_platform_id    platform,
       *num_devices = 1;
     if (devices) {
       *devices = device;
-      (*devices)->extensions = intel_platform->extensions;
-      (*devices)->extensions_sz = intel_platform->extensions_sz;
+      (*devices)->extensions = cl_get_platform_default()->extensions;
+      (*devices)->extensions_sz = cl_get_platform_default()->extensions_sz;
     }
     return CL_SUCCESS;
   }
diff --git a/src/cl_extensions.c b/src/cl_extensions.c
index 0a29d2f..f1948b3 100644
--- a/src/cl_extensions.c
+++ b/src/cl_extensions.c
@@ -13,7 +13,9 @@
 #include <string.h>
 #include <assert.h>
 
-static struct cl_extensions intel_extensions =
+/* This extension should be common for all the intel GPU platform.
+   Every device may have its own additional externsions. */
+static struct cl_extensions intel_platform_extensions =
 {
   {
 #define DECL_EXT(name) \
@@ -91,14 +93,12 @@ process_extension_str(cl_extensions_t *extensions)
   }
 }
 
-static int ext_initialized = 0;
 
 LOCAL void
 cl_intel_platform_enable_fp16_extension(cl_platform_id intel_platform)
 {
-  cl_extensions_t *extensions = &intel_extensions;
+  cl_extensions_t *extensions = &intel_platform_extensions;
   int id;
-  assert(ext_initialized);
 
   for(id = OPT1_EXT_START_ID; id <= OPT1_EXT_END_ID; id++)
   {
@@ -107,29 +107,27 @@ cl_intel_platform_enable_fp16_extension(cl_platform_id intel_platform)
   }
 
   process_extension_str(extensions);
-  intel_platform->internal_extensions = &intel_extensions;
-  intel_platform->extensions = intel_extensions.ext_str;
+  intel_platform->internal_extensions = &intel_platform_extensions;
+  intel_platform->extensions = intel_platform_extensions.ext_str;
   intel_platform->extensions_sz = strlen(intel_platform->extensions) + 1;
 }
 
 LOCAL void
 cl_intel_platform_extension_init(cl_platform_id intel_platform)
 {
-  if (ext_initialized) {
-    intel_platform->internal_extensions = &intel_extensions;
-    intel_platform->extensions = intel_extensions.ext_str;
-    return;
+  static int ext_initialized = 0;
+
+  if (!ext_initialized) {
+    check_basic_extension(&intel_platform_extensions);
+    check_opt1_extension(&intel_platform_extensions);
+    check_gl_extension(&intel_platform_extensions);
+    check_intel_extension(&intel_platform_extensions);
+    process_extension_str(&intel_platform_extensions);
+    ext_initialized = 1;
   }
-  check_basic_extension(&intel_extensions);
-  check_opt1_extension(&intel_extensions);
-  check_gl_extension(&intel_extensions);
-  check_intel_extension(&intel_extensions);
-  process_extension_str(&intel_extensions);
-
-  intel_platform->internal_extensions = &intel_extensions;
-  intel_platform->extensions = intel_extensions.ext_str;
-  intel_platform->extensions_sz = strlen(intel_platform->extensions) + 1;
 
-  ext_initialized = 1;
+  intel_platform->internal_extensions = &intel_platform_extensions;
+  intel_platform->extensions = intel_platform_extensions.ext_str;
+  intel_platform->extensions_sz = strlen(intel_platform->extensions) + 1;
   return;
 }
diff --git a/src/cl_platform_id.c b/src/cl_platform_id.c
index bc2d799..d7a1f68 100644
--- a/src/cl_platform_id.c
+++ b/src/cl_platform_id.c
@@ -41,8 +41,19 @@ static struct _cl_platform_id intel_platform_data = {
 
 #undef DECL_INFO_STRING
 
-/* Intel platform (only GPU now) */
-cl_platform_id const intel_platform = &intel_platform_data;
+/* Intel platform (only GPU now).
+   It is used as default when the API's platform ptr is NULL */
+static cl_platform_id intel_platform = NULL;
+LOCAL cl_platform_id
+cl_get_platform_default(void)
+{
+  if (intel_platform)
+    return intel_platform;
+
+  intel_platform = &intel_platform_data;
+  cl_intel_platform_extension_init(intel_platform);
+  return intel_platform;
+}
 
 LOCAL cl_int
 cl_get_platform_ids(cl_uint          num_entries,
@@ -52,29 +63,28 @@ cl_get_platform_ids(cl_uint          num_entries,
   if (num_platforms != NULL)
     *num_platforms = 1;
 
-  cl_intel_platform_extension_init(intel_platform);
   /* Easy right now, only one platform is supported */
   if(platforms)
-    *platforms = intel_platform;
+    *platforms = cl_get_platform_default();
 
   return CL_SUCCESS;
 }
 
 #define DECL_FIELD(CASE,FIELD)                                  \
   case JOIN(CL_,CASE):                                          \
-    if (param_value_size < intel_platform->JOIN(FIELD,_sz))     \
+    if (param_value_size < cl_get_platform_default()->JOIN(FIELD,_sz))     \
       return CL_INVALID_VALUE;                                  \
     if (param_value_size_ret != NULL)                           \
-      *param_value_size_ret = intel_platform->JOIN(FIELD,_sz);  \
+      *param_value_size_ret = cl_get_platform_default()->JOIN(FIELD,_sz);  \
     memcpy(param_value,                                         \
-           intel_platform->FIELD,                               \
-           intel_platform->JOIN(FIELD,_sz));                    \
+           cl_get_platform_default()->FIELD,                               \
+           cl_get_platform_default()->JOIN(FIELD,_sz));                    \
       return CL_SUCCESS;
 
 #define GET_FIELD_SZ(CASE,FIELD)                                \
   case JOIN(CL_,CASE):                                          \
     if (param_value_size_ret != NULL)                           \
-      *param_value_size_ret = intel_platform->JOIN(FIELD,_sz);  \
+      *param_value_size_ret = cl_get_platform_default()->JOIN(FIELD,_sz);  \
     return CL_SUCCESS;
 
 LOCAL cl_int
diff --git a/src/cl_platform_id.h b/src/cl_platform_id.h
index 7b78db1..865317a 100644
--- a/src/cl_platform_id.h
+++ b/src/cl_platform_id.h
@@ -44,8 +44,8 @@ struct _cl_platform_id {
   struct cl_extensions *internal_extensions;
 };
 
-/* Platform implemented by this run-time */
-extern cl_platform_id const intel_platform;
+/* Return the default platform */
+extern cl_platform_id cl_get_platform_default(void);
 
 /* Return the valid platform */
 extern cl_int cl_get_platform_ids(cl_uint          num_entries,
-- 
1.9.1



More information about the Beignet mailing list