Skip to content
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 macos with workaround #209

Closed
wants to merge 2 commits into from

Conversation

OGriebel
Copy link

This is about issues #205 #206.
#196 introduced breaking changes for MacOS.

!!! Consider this commit as a dirty workaround to get macos back running now.

With version 0.9.0 of OpenCL.jl, the following output is returned for macos (x86).

julia> device, ctx, queue = cl.create_compute_context()
ERROR: CLError(code=-1001, CL_PLATFORM_NOT_FOUND_KHR)
Stacktrace:
 [1] macro expansion
   @ ~/.julia/packages/OpenCL/BTcrM/src/macros.jl:6 [inlined]
 [2] platforms()
   @ OpenCL.cl ~/.julia/packages/OpenCL/BTcrM/src/platform.jl:43
 [3] create_some_context()
   @ OpenCL.cl ~/.julia/packages/OpenCL/BTcrM/src/context.jl:277
 [4] create_compute_context()
   @ OpenCL.cl ~/.julia/packages/OpenCL/BTcrM/src/util.jl:2
 [5] top-level scope
   @ REPL[2]:1

This commit reverts the changes from #196 for macos, only. Other OS versions should not be affected. However, this requires to be validated by others. Following output is returned for my macos (x86).

julia> device, ctx, queue = cl.create_compute_context()
(OpenCL.Device(Intel(R) UHD Graphics 630 on Apple @0x0000000001024500), OpenCL.Context(@0x00007fdb11dcd260 on Intel(R) UHD Graphics 630), OpenCL.CmdQueue(@0x00007fdb143e26d0))

Fixing this bug properly, requires changes to the OpenCL_jll library. Roughly, the following steps are required:

...

# These are the platforms we will build for by default, unless further
# platforms are passed in on the command line
platforms = supported_platforms(; exclude=(Sys.iswindows || Sys.isapple))
platforms_macos = [ Platform("x86_64", "macos"), Platform("aarch64", "macos") ] # Support M-Series Processors as well?

# The products that we will ensure are always built
products = [
    LibraryProduct("libOpenCL", :libopencl)
]
products_macos = [
    FrameworkProduct("libOpenCL", :libopencl)
]

# Dependencies that must be installed before this package can be built
dependencies = [
    BuildDependency(PackageSpec(; name="OpenCL_Headers_jll", version=v"2022.09.23"))
]
dependencies_macos = [
    # At this point it is not clear to me, how to change OpenCL_Headers_jll.
]

# Build the tarballs, and possibly a `build.jl` as well.
if any(should_build_platform.(triplet.(platforms)))
    build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies; julia_compat="1.6", preferred_gcc_version = v"6.1.0")
end
if any(should_build_platform.(triplet.(platforms_macos)))
    build_tarballs(ARGS, name, version, sources, script, platforms_macos, products_macos, dependencies_macos; julia_compat="1.6", preferred_gcc_version = v"6.1.0")
end

This reverts the changes for MacOS from commit "Add OpenCL_jll dependence" JuliaGPU#196

However, consider this as a dirty workaround to get macos back running. Fixing this within the OpenCL_jll library is will be required.
Include Libdl dependency
@juliohm
Copy link
Member

juliohm commented Jan 23, 2024 via email

@OGriebel
Copy link
Author

Short answer, unfortunately not.

Long answer:
My understanding of FrameworkProduct() and the respective dependencies is somewhat limited. In case this works similar to LibraryProduct(), the changes should be minor.

I never tried to fix a *_jll library with BinaryBuilder on Yggdrasil. If you could provide me with a reference on how to work with these, I might have a try.

@juliohm
Copy link
Member

juliohm commented Jan 23, 2024 via email

@OGriebel
Copy link
Author

Thanks, I will have a look into it. It might take a bit though, as the library is running for the moment. Other parts of my project will be done first but added to the list.

Consider this hotfix closed for now. As soon as I am successful, I will create pull requests to all affected repositories and revert the changes of this commit.

@maleadt
Copy link
Member

maleadt commented Aug 9, 2024

This PR isn't applicable after the recent refactor (and wasn't a viable solution anyway), so I propose closing it and continuing the discussion in #205.

@maleadt maleadt closed this Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants