-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix finding ocl-icd in non-standard directories #1252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. This got me thinking, can/does ocl-icd expose the macOS OpenCL framework for the proxy device?
EDIT: nvm, saw the mention in the docs in the other PR
@@ -988,6 +988,7 @@ else() | |||
pkg_check_modules(OCL_ICD ocl-icd>=1.3) | |||
endif() | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stray newline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one minor nitpick
endif() | ||
|
||
# Find OCL_ICD_LIBRARIES with the hint from pkg-config | ||
find_library(OCL_ICD_LIBRARIES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC how find_library
works, this would find any library named OpenCL in any paths it searches, and that includes "system" paths. But in this case we want to find only ocl-icd, not any other OpenCL ICD. So i think this one needs a NO_DEFAULT_PATH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that what's done already at https://github.com/pocl/pocl/blob/master/CMakeLists.txt#L991-L1003 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but that code was also incorrectly leaving OCL_ICD_LIBRARIES set when it found other than ocl-icd loader. But this is just a minor nitpick.
@jansol, I use https://github.com/jrprice/ocl_icd_wrapper to expose Apple's OpenCL framework to the ICD loader. |
No description provided.