Skip to content

Fixed issue with installed multiple versions of the OpenCL which lead…#2378

Merged
vpirogov merged 1 commit intouxlfoundation:mainfrom
redradist:fix/multi-opencl-versions
Feb 25, 2025
Merged

Fixed issue with installed multiple versions of the OpenCL which lead…#2378
vpirogov merged 1 commit intouxlfoundation:mainfrom
redradist:fix/multi-opencl-versions

Conversation

@redradist
Copy link
Contributor

@redradist redradist commented Jan 11, 2025

Description

This PR fix issue with multiple version OpenCL installed on machne as well as brings new versions of OpenCL 3_0

Related PR #2001, issue was more deeply investigated and PR was reworked

For details explanation of an issue with ${OPENCL_INCLUDE_DIR}/CL/cl.h https://stackoverflow.com/a/79347124/5157245

@redradist redradist requested a review from a team as a code owner January 11, 2025 08:14
@redradist redradist force-pushed the fix/multi-opencl-versions branch from a73728d to 3bc6014 Compare January 11, 2025 08:17
@redradist redradist force-pushed the fix/multi-opencl-versions branch from 3bc6014 to a0a730d Compare January 11, 2025 08:20
@redradist redradist requested a review from densamoilov January 20, 2025 16:16
@redradist
Copy link
Contributor Author

Guys is there any other comments to this PR ??

@vpirogov
Copy link
Contributor

Guys is there any other comments to this PR ??

I don't understand what this change does based on stackoverflow discussion. Specifically this part:

CHECK_SYMBOL_EXISTS try to compile test program with the following include statment: #include <${OPENCL_INCLUDE_DIR}/CL/cl.h>, - where it was in my case: #include </usr/include/CL/cl.h>, after that compiler struggle between 2 type of header files for OpenCL in /usr/include/CL/ and /usr/local/cuda/include/CL/

Also note, that the code you are trying to change comes from CMake upstream verbatim.

If you want to pursue this further consider one of the following options:

  • Open a merge request in CMake upstream to get input from the CMake developers.
  • Check if updating FindOpenCL.cmake from upstream solves your problem. If it does we'll happily accept the PR with update.
  • Provide a oneDNN build command and list of packages that demonstrate the issue so that we can reproduce the issue on our end and investigate.

@redradist
Copy link
Contributor Author

redradist commented Feb 23, 2025

Guys is there any other comments to this PR ??

I don't understand what this change does based on stackoverflow discussion. Specifically this part:

CHECK_SYMBOL_EXISTS try to compile test program with the following include statment: #include <${OPENCL_INCLUDE_DIR}/CL/cl.h>, - where it was in my case: #include </usr/include/CL/cl.h>, after that compiler struggle between 2 type of header files for OpenCL in /usr/include/CL/ and /usr/local/cuda/include/CL/

Also note, that the code you are trying to change comes from CMake upstream verbatim.

If you want to pursue this further consider one of the following options:

* Open a [merge request in CMake upstream](https://gitlab.kitware.com/cmake/cmake/-/merge_requests) to get input from the CMake developers.

* Check if updating FindOpenCL.cmake from upstream solves your problem. If it does we'll happily accept the PR with update.

* Provide a oneDNN build command and list of packages that demonstrate the issue so that we can reproduce the issue on our end and investigate.

@densamoilov @vpirogov Done, PR is merged:

https://gitlab.kitware.com/cmake/cmake/-/merge_requests/10366

For more details see comments in PR of CMake

Here it is merged in master branch https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindOpenCL.cmake?ref_type=heads

@redradist
Copy link
Contributor Author

redradist commented Feb 24, 2025

@vpirogov @YanIntel PR is approved, but Labeler / label (pull_request_target) was cancelled previously due to it PR is blocked for now.

Could someone with permissions to restart this job ?

@vpirogov
Copy link
Contributor

Could someone with permissions to restart this job ?

Disregard this one.

@vpirogov
Copy link
Contributor

make test

@vpirogov vpirogov force-pushed the fix/multi-opencl-versions branch from a0a730d to 0dbb77c Compare February 24, 2025 18:21
@vpirogov
Copy link
Contributor

make test

@redradist redradist force-pushed the fix/multi-opencl-versions branch from 0dbb77c to 31f3bd3 Compare February 25, 2025 07:14
@redradist redradist force-pushed the fix/multi-opencl-versions branch from 31f3bd3 to 6e73b16 Compare February 25, 2025 15:56
@redradist
Copy link
Contributor Author

@vpirogov @densamoilov All tests are passed, could be merged

@vpirogov
Copy link
Contributor

make test

@vpirogov vpirogov added this to the v3.8 milestone Feb 25, 2025
@vpirogov vpirogov merged commit 2d3f544 into uxlfoundation:main Feb 25, 2025
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants