Improve selection of OpenCL platform and target device #2043

albelyakov · 2020-03-26T08:52:27Z

No description provided.

samples/sample_plugins/rotate_opencl/include/opencl_filter.h

dvrogozh · 2020-03-26T15:12:11Z

samples/sample_plugins/rotate_opencl/src/opencl_filter.cpp

+
+            for(std::vector<std::string>::iterator extNameIt = m_requiredOclExtensions.begin(); extNameIt != m_requiredOclExtensions.end(); ++extNameIt)
+            {
+                isDeviceAppropriate = isDeviceAppropriate && (std::strstr(&extNamesBuffer[0], extNameIt->c_str()) != NULL);


why do you check the first extension only: extNamesBuffer[0]? the one you need might not be the first one.

& extNamesBuffer[0] was a way to access all vector elements. Changed it to vector.data()

dvrogozh · 2020-03-26T15:13:24Z

samples/sample_plugins/rotate_opencl/src/opencl_filter.cpp

+
+            bool isDeviceAppropriate = true;
+
+            for(std::vector<std::string>::iterator extNameIt = m_requiredOclExtensions.begin(); extNameIt != m_requiredOclExtensions.end(); ++extNameIt)


for (const auto& ext: m_requireOclExtensions) form would be preferable whenever possible.

Yes, definitely.

dvrogozh · 2020-03-26T15:14:32Z

samples/sample_plugins/rotate_opencl/src/opencl_filter.cpp

+            for(std::vector<std::string>::iterator extNameIt = m_requiredOclExtensions.begin(); extNameIt != m_requiredOclExtensions.end(); ++extNameIt)
+            {
+                isDeviceAppropriate = isDeviceAppropriate && (std::strstr(&extNamesBuffer[0], extNameIt->c_str()) != NULL);
+            }


Well, you actually did not check for required extensions... You checked that the first extension reported is in your whitelist.

& extNamesBuffer[0] was a way to access all vector elements, so all of the extensions were checked. Changed it to vector.data()

dvrogozh · 2020-03-26T15:15:31Z

samples/sample_plugins/rotate_opencl/src/opencl_filter.cpp

        }
    }
+
+_select_platform_loop_end:


you can achieve pretty the same without goto label. Just return from the function when you need to.

dvrogozh · 2020-03-26T15:18:53Z

samples/sample_plugins/rotate_opencl/src/opencl_filter.cpp

+                continue;
+            }
+
+            size_t extNamesBufferSize = 0;


I would rename these variables to something more simple. extensionsNum and extensions would be better

*) For-each. *) Defines. *) vector.data usage instead of &vector[0].

onabiull

I'm going to squash these commits to a single commit upon merge.
@dvrogozh your call?

dvrogozh · 2020-03-31T16:55:22Z

samples/sample_plugins/rotate_opencl/src/opencl_filter.cpp


-        if(strstr(platform, "Intel")) // Use only Intel platfroms
+        error = clGetDeviceIDs(platform, CL_DEVICE_TYPE_GPU, devices.size(), &devices[0], 0);


&devices[0] -> devices.data()

dvrogozh · 2020-03-31T17:03:08Z

samples/sample_plugins/rotate_opencl/src/opencl_filter_dx11.cpp

 #include "opencl_filter_dx11.h"
 #include "sample_defs.h"

-#include "stdexcept"
-
 // INTEL DX9 sharing functions declared deprecated in OpenCL 1.2


why this is applicable for dx11.cpp file? both comment and pragma-warning?

dvrogozh · 2020-03-31T17:03:52Z

samples/sample_plugins/rotate_opencl/CMakeLists.txt

@@ -10,11 +10,9 @@ include_directories (

 set( LDFLAGS "${LDFLAGS} -Wl,--version-script=${PLUGINS_COMMON_PATH}/mfx_plugin.map" )

-# for clCreateFromGLTexture2D and clCreateFromGLTexture3D in CL/cl_gl.h
 set( CFLAGS "-DCL_USE_DEPRECATED_OPENCL_1_1_APIS" )


Do we really need CL_USE_DEPRECATED_OPENCL_1_1_APIS?

dvrogozh · 2020-03-31T17:04:27Z

samples/sample_plugins/rotate_opencl/include/sample_opencl_plugin.h

-#include <va/va.h>
-#define CL_USE_DEPRECATED_OPENCL_1_1_APIS 1
-#endif
-
 #undef CL_VERSION_1_2


where do we define CL_VERSION_1_2 and why do we need undef?

dvrogozh · 2020-03-31T17:05:27Z

samples/sample_plugins/rotate_opencl/src/opencl_filter_dx11.cpp

 #include <CL/cl_d3d11.h>

 using std::endl;

-#define INIT_CL_EXT_FUNC(x)    x = (x ## _fn)clGetExtensionFunctionAddress(#x);
 #define SAFE_OCL_FREE(P, FREE_FUNC)  { if (P) { FREE_FUNC(P); P = NULL; } }

 #define MAX_PLATFORMS       32


please, check whether we need these 2 defines? I thought we've moved to std::vector?

Remove outdated include and defines. Amendment for Intel-Media-SDK#2190. Includes changes from Intel-Media-SDK#2043 Co-authored-by: abelaykov <belaykov.alexander.a@gmail.com>

Remove outdated include and defines. Amendment for #2190. Includes changes from #2043 Co-authored-by: abelaykov <belaykov.alexander.a@gmail.com>

onabiull · 2020-07-31T13:02:23Z

Changes merged with minimal modifications as part of #2243.

Remove outdated include and defines. Amendment for Intel-Media-SDK#2190. Includes changes from Intel-Media-SDK#2043 Co-authored-by: abelaykov <belaykov.alexander.a@gmail.com>

Remove outdated include and defines. Amendment for #2190. Includes changes from #2043 Co-authored-by: abelaykov <belaykov.alexander.a@gmail.com>

albelyakov requested a review from a team as a code owner March 26, 2020 08:52

dvrogozh suggested changes Mar 26, 2020

View reviewed changes

Improve selection of OpenCL platform and target device

3a2b9a4

albelyakov force-pushed the wmouse_patches branch from e8da276 to 3a2b9a4 Compare March 27, 2020 17:30

albelyakov added 2 commits March 29, 2020 00:55

Review issues:

14f289f

*) For-each. *) Defines. *) vector.data usage instead of &vector[0].

Compilation issue.

b037ed2

albelyakov requested a review from dvrogozh March 31, 2020 08:38

onabiull approved these changes Mar 31, 2020

View reviewed changes

dvrogozh suggested changes Mar 31, 2020

View reviewed changes

onabiull pushed a commit that referenced this pull request Jul 31, 2020

Fixup b76c44a("Use CL/cl2.hpp for rotate_opencl_plugin")

5478175

Remove outdated include and defines. Amendment for #2190. Includes changes from #2043 Co-authored-by: abelaykov <belaykov.alexander.a@gmail.com>

onabiull closed this Jul 31, 2020

onabiull pushed a commit that referenced this pull request Aug 3, 2020

Fixup b76c44a("Use CL/cl2.hpp for rotate_opencl_plugin")

a303d12

Remove outdated include and defines. Amendment for #2190. Includes changes from #2043 Co-authored-by: abelaykov <belaykov.alexander.a@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve selection of OpenCL platform and target device #2043

Improve selection of OpenCL platform and target device #2043

albelyakov commented Mar 26, 2020

dvrogozh Mar 26, 2020

albelyakov Mar 28, 2020

dvrogozh Mar 26, 2020

albelyakov Mar 28, 2020

dvrogozh Mar 26, 2020

albelyakov Mar 28, 2020

dvrogozh Mar 26, 2020

albelyakov Mar 28, 2020

dvrogozh Mar 26, 2020

albelyakov Mar 28, 2020

onabiull left a comment

dvrogozh Mar 31, 2020

dvrogozh Mar 31, 2020

dvrogozh Mar 31, 2020

dvrogozh Mar 31, 2020

dvrogozh Mar 31, 2020

onabiull commented Jul 31, 2020


		bool isDeviceAppropriate = true;

		for(std::vector<std::string>::iterator extNameIt = m_requiredOclExtensions.begin(); extNameIt != m_requiredOclExtensions.end(); ++extNameIt)


		if(strstr(platform, "Intel")) // Use only Intel platfroms
		error = clGetDeviceIDs(platform, CL_DEVICE_TYPE_GPU, devices.size(), &devices[0], 0);

Improve selection of OpenCL platform and target device #2043

Improve selection of OpenCL platform and target device #2043

Conversation

albelyakov commented Mar 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onabiull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onabiull commented Jul 31, 2020