From 0c777cc326d710e5f57ff2013de21fb56eb840df Mon Sep 17 00:00:00 2001 From: Sherry Yuan Date: Fri, 4 Mar 2022 08:50:01 -0800 Subject: [PATCH 1/3] Remove checks on whether query is not CL_DEVICE_EXTENSIONS ------------------------------------------------------------------------ The query for CL_DEVICE_EXTENSIONS will have nonzero return size, the checks can now be removed to check there are positive number of extensions. --- test/acl_device_test.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/acl_device_test.cpp b/test/acl_device_test.cpp index fbd97dfc..614f4090 100644 --- a/test/acl_device_test.cpp +++ b/test/acl_device_test.cpp @@ -324,10 +324,7 @@ MT_TEST(DeviceInfo, basic) { CHECK_EQUAL(CL_SUCCESS, clGetDeviceInfo(device, queries[i], str_size, &str[0], &size_ret)); - if (queries[i] != CL_DEVICE_EXTENSIONS) { - // Non-empty result for most supported queries. - CHECK(size_ret > 0); - } + CHECK(size_ret > 0); switch (queries[i]) { case CL_DEVICE_VENDOR: From 4ac7ca880db207ad6d41c275cc9dc39cdfeb8c29 Mon Sep 17 00:00:00 2001 From: Sherry Yuan Date: Fri, 4 Mar 2022 09:22:13 -0800 Subject: [PATCH 2/3] Define function to return string literal for checking all extensions --------------------------------------------------------------------------- The string literall returned is statically allocated so that it is safe to return one from a function. The reason why std::string was not used. once modified, the c_cstr() pointer may no longer be valid. Futher tsan reported a heap use after free error if the acl_platform_extensions return std::string instead. --- include/acl_platform.h | 1 + src/acl_platform.cpp | 54 +++++++++++++++++++++------------------- test/acl_device_test.cpp | 5 ++++ 3 files changed, 35 insertions(+), 25 deletions(-) diff --git a/include/acl_platform.h b/include/acl_platform.h index 29e8c533..b852e51b 100644 --- a/include/acl_platform.h +++ b/include/acl_platform.h @@ -21,6 +21,7 @@ extern int platform_owner_tid; void acl_init_platform(void); void acl_finalize_init_platform(unsigned int num_devices, const cl_device_id *devices); +const char *acl_platform_extensions(void); #if defined(__cplusplus) } /* extern "C" */ diff --git a/src/acl_platform.cpp b/src/acl_platform.cpp index 8f90279b..9bae35d3 100644 --- a/src/acl_platform.cpp +++ b/src/acl_platform.cpp @@ -257,6 +257,34 @@ int acl_platform_is_valid(cl_platform_id platform) { return platform == &acl_platform; } +const char *acl_platform_extensions() { + return "cl_khr_byte_addressable_store" // yes, we have correct access to + // individual bytes! + " cles_khr_int64" // yes, we support 64-bit ints in the embedded + // profile + + " cl_khr_icd" +#if ACL_SUPPORT_IMAGES == 1 + " cl_khr_3d_image_writes" +#endif +#if ACL_SUPPORT_DOUBLE == 1 + " cl_khr_fp64" +#endif +#ifdef ACL_120 + " cl_khr_global_int32_base_atomics" + " cl_khr_global_int32_extended_atomics" + " cl_khr_local_int32_base_atomics" + " cl_khr_local_int32_extended_atomics" +#endif +#ifndef __arm__ + // Add the following once all USM APIs are implemented, Intel publishes + // the spec, and published USM conformance tests pass + //" cl_intel_unified_shared_memory" +#endif + " cl_intel_create_buffer_with_properties" + " cl_intel_mem_channel_property"; +} + // Initialize the internal bookkeeping based on the system definition // provided to us. void acl_init_platform(void) { @@ -298,31 +326,7 @@ void acl_init_platform(void) { // The extensions string specifies the extensions supported by our framework. // To add an extension, append a flag name and separate it from others using a // space. - acl_platform.extensions = - "cl_khr_byte_addressable_store" // yes, we have correct access to - // individual bytes! - " cles_khr_int64" // yes, we support 64-bit ints in the embedded profile - - " cl_khr_icd" -#if ACL_SUPPORT_IMAGES == 1 - " cl_khr_3d_image_writes" -#endif -#if ACL_SUPPORT_DOUBLE == 1 - " cl_khr_fp64" -#endif -#ifdef ACL_120 - " cl_khr_global_int32_base_atomics" - " cl_khr_global_int32_extended_atomics" - " cl_khr_local_int32_base_atomics" - " cl_khr_local_int32_extended_atomics" -#endif -#ifndef __arm__ - // Add the following once all USM APIs are implemented, Intel publishes - // the spec, and published USM conformance tests pass - //" cl_intel_unified_shared_memory" -#endif - " cl_intel_create_buffer_with_properties" - " cl_intel_mem_channel_property"; + acl_platform.extensions = acl_platform_extensions(); acl_platform.profile = "EMBEDDED_PROFILE"; acl_platform.hal = acl_get_hal(); diff --git a/test/acl_device_test.cpp b/test/acl_device_test.cpp index 614f4090..90d7bb59 100644 --- a/test/acl_device_test.cpp +++ b/test/acl_device_test.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -326,6 +327,10 @@ MT_TEST(DeviceInfo, basic) { CHECK(size_ret > 0); + if (queries[i] == CL_DEVICE_EXTENSIONS) { + CHECK(strcmp(str, acl_platform_extensions()) == 0); + } + switch (queries[i]) { case CL_DEVICE_VENDOR: case CL_DEVICE_VERSION: From 382297da67aac6fb0679446a97988ec3bd0b1362 Mon Sep 17 00:00:00 2001 From: Sherry Yuan Date: Thu, 3 Mar 2022 12:13:50 -0800 Subject: [PATCH 3/3] Add cl_intel_mem_alloc_buffer_location to supported extension -------------------------------------------------------------------------- For sycl runtime to check whether a lower runtime library support a specific extension, they use clGetDeviceInfo with CL_DEVICE_EXTENSIONS as query parameter. If the extension of interest appears in the string then the runtime will use the extension accordingly. Otherwise the extension will have no effect. --- src/acl_platform.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/acl_platform.cpp b/src/acl_platform.cpp index 9bae35d3..69548270 100644 --- a/src/acl_platform.cpp +++ b/src/acl_platform.cpp @@ -282,7 +282,8 @@ const char *acl_platform_extensions() { //" cl_intel_unified_shared_memory" #endif " cl_intel_create_buffer_with_properties" - " cl_intel_mem_channel_property"; + " cl_intel_mem_channel_property" + " cl_intel_mem_alloc_buffer_location"; } // Initialize the internal bookkeeping based on the system definition