Re: [Beignet] [PATCH v3] Utests: Allow testing cl_intel_accelerator via ICD

2016-10-12 Thread Weng, Chuanbo
Hi Rebecca,
This version LGTM except some points need to be minor refined. Just see 
my comments below.

-Original Message-
From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of 
Rebecca N. Palmer
Sent: Wednesday, October 12, 2016 5:50 AM
To: Weng, Chuanbo <chuanbo.w...@intel.com>; beignet@lists.freedesktop.org
Subject: Re: [Beignet] [PATCH v3] Utests: Allow testing cl_intel_accelerator 
via ICD

v3: Use extension check, not beignet check.  Treat claiming to have the 
extension but not having the kernel as a failure.

---
(v2 was the un-numbered 10/10/16 08:07 version...which I subsequently noticed 
was broken in that it assumed a non-NULL 
clGetExtensionFunctionAddressForPlatform result meant the extension was 
supported, which it doesn't in general, 
https://www.khronos.org/registry/cl/sdk/2.1/docs/man/xhtml/clGetExtensionFunctionAddressForPlatform.html
 )

--- a/utests/builtin_kernel_block_motion_estimate_intel.cpp
+++ b/utests/builtin_kernel_block_motion_estimate_intel.cpp
@@ -8,6 +8,19 @@ OCLRELEASEACCELERATORINTEL * oclReleaseA
 
 void builtin_kernel_block_motion_estimate_intel(void)
 {
+  std::string extStr;
+  size_t param_value_size;
+  OCL_CALL(clGetDeviceInfo, device, CL_DEVICE_EXTENSIONS, 0, 0, 
+ _value_size);  std::vector param_value(param_value_size);  
+ OCL_CALL(clGetDeviceInfo, device, CL_DEVICE_EXTENSIONS, param_value_size,
+   param_value.empty() ? NULL : _value.front(), 
+ _value_size);  if (!param_value.empty())
+extStr = std::string(_value.front(), param_value_size-1);  // 
+ cl_intel_motion_estimation depends on cl_intel_accelerator, so we only 
+ need to check one  if (strstr(extStr.c_str(), "cl_intel_motion_estimation") 
== NULL) {
+printf("No cl_intel_motion_estimation, Skip!");
+return;
+  }
[Chuanbo] It would be better if you wrapper this part of code into 
cl_check_motion_estimation() and then move
it to utest_helper.cpp. This will keep existing code organization style.
There is a bug in Beignet: cl_intel_motion_estimation is supported by IVB only, 
but all devices show string cl_intel_motion_estimation
in their CL_DEVICE_EXTENSIONS. I'll work out a patch to fix this problem.

   char* built_in_kernel_names;
   size_t built_in_kernels_size;
   cl_int err = CL_SUCCESS;
@@ -21,7 +34,8 @@ void builtin_kernel_block_motion_estimat
   if (strstr(built_in_kernel_names, "block_motion_estimate_intel") == NULL)
   {
 free(built_in_kernel_names);
-return;
+printf("Can't find block_motion_estimate_intel built-in kernel");
[Chuanbo] Although I know there are somewhere else using printf instead 
of fprintf(stderr, ...), let's keep in mind that
we should better use fprintf(stderr, ...) for output of error info.
+OCL_ASSERT(0);
   }
 
   cl_program built_in_prog = clCreateProgramWithBuiltInKernels(ctx, 1, 
, built_in_kernel_names, );
--- a/utests/CMakeLists.txt
+++ b/utests/CMakeLists.txt
@@ -287,7 +287,8 @@ set (utests_sources
   multi_queue_events.cpp
   compiler_mix.cpp
   compiler_math_3op.cpp
-  compiler_bsort.cpp)
+  compiler_bsort.cpp
+  builtin_kernel_block_motion_estimate_intel.cpp)
 
 if (LLVM_VERSION_NODOT VERSION_GREATER 34)
   SET(utests_sources
@@ -328,7 +329,6 @@ else(GEN_PCI_ID)
 endif(GEN_PCI_ID)
 
 if (NOT_BUILD_STAND_ALONE_UTEST)
-  SET(utests_sources ${utests_sources} 
builtin_kernel_block_motion_estimate_intel.cpp)
   ADD_CUSTOM_TARGET(kernel_bin.bin DEPENDS ${kernel_bin}.bin)  endif 
(NOT_BUILD_STAND_ALONE_UTEST)
 

___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet
___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet


Re: [Beignet] [PATCH v3] Utests: Allow testing cl_intel_accelerator via ICD

2016-10-11 Thread Rebecca N. Palmer
v3: Use extension check, not beignet check.  Treat claiming
to have the extension but not having the kernel as a failure.

---
(v2 was the un-numbered 10/10/16 08:07 version...which I subsequently
noticed was broken in that it assumed a non-NULL
clGetExtensionFunctionAddressForPlatform result meant the extension
was supported, which it doesn't in general,
https://www.khronos.org/registry/cl/sdk/2.1/docs/man/xhtml/clGetExtensionFunctionAddressForPlatform.html
 )

--- a/utests/builtin_kernel_block_motion_estimate_intel.cpp
+++ b/utests/builtin_kernel_block_motion_estimate_intel.cpp
@@ -8,6 +8,19 @@ OCLRELEASEACCELERATORINTEL * oclReleaseA
 
 void builtin_kernel_block_motion_estimate_intel(void)
 {
+  std::string extStr;
+  size_t param_value_size;
+  OCL_CALL(clGetDeviceInfo, device, CL_DEVICE_EXTENSIONS, 0, 0, 
_value_size);
+  std::vector param_value(param_value_size);
+  OCL_CALL(clGetDeviceInfo, device, CL_DEVICE_EXTENSIONS, param_value_size,
+   param_value.empty() ? NULL : _value.front(), 
_value_size);
+  if (!param_value.empty())
+extStr = std::string(_value.front(), param_value_size-1);
+  // cl_intel_motion_estimation depends on cl_intel_accelerator, so we only 
need to check one
+  if (strstr(extStr.c_str(), "cl_intel_motion_estimation") == NULL) {
+printf("No cl_intel_motion_estimation, Skip!");
+return;
+  }
   char* built_in_kernel_names;
   size_t built_in_kernels_size;
   cl_int err = CL_SUCCESS;
@@ -21,7 +34,8 @@ void builtin_kernel_block_motion_estimat
   if (strstr(built_in_kernel_names, "block_motion_estimate_intel") == NULL)
   {
 free(built_in_kernel_names);
-return;
+printf("Can't find block_motion_estimate_intel built-in kernel");
+OCL_ASSERT(0);
   }
 
   cl_program built_in_prog = clCreateProgramWithBuiltInKernels(ctx, 1, 
, built_in_kernel_names, );
--- a/utests/CMakeLists.txt
+++ b/utests/CMakeLists.txt
@@ -287,7 +287,8 @@ set (utests_sources
   multi_queue_events.cpp
   compiler_mix.cpp
   compiler_math_3op.cpp
-  compiler_bsort.cpp)
+  compiler_bsort.cpp
+  builtin_kernel_block_motion_estimate_intel.cpp)
 
 if (LLVM_VERSION_NODOT VERSION_GREATER 34)
   SET(utests_sources
@@ -328,7 +329,6 @@ else(GEN_PCI_ID)
 endif(GEN_PCI_ID)
 
 if (NOT_BUILD_STAND_ALONE_UTEST)
-  SET(utests_sources ${utests_sources} 
builtin_kernel_block_motion_estimate_intel.cpp)
   ADD_CUSTOM_TARGET(kernel_bin.bin DEPENDS ${kernel_bin}.bin)
 endif (NOT_BUILD_STAND_ALONE_UTEST)
 

___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet