ROCm / ROCm-OpenCL-Runtime

ROCm OpenOpenCL Runtime
170 stars 60 forks source link

Remove bundled Khronos loader/headers in favor of system version #121

Open ranisalt opened 4 years ago

ranisalt commented 4 years ago

Instead of using bundled OpenCL ICD loader and headers, leverage the system version instead. This makes it better to build packages and keep them up to date, as ROCm loader won't lag behind.

Users on Fedora can install mesa-libgl-devel, on Debian it should be mesa-libgl-dev, and I get the very same functionality after replacing those headers with system.

vsytch commented 4 years ago

As much as we want to switch to using upstream OCL ICD and headers, this for now is not possible. Due to historic reasons we never upstreamed many AMD specific extensions. These right now only reside in the headers in this repository. Stale headers require a stale ICD.

Hopefully for OCL 3.0 we can clean this mess up.

ranisalt commented 4 years ago

@vsytch how unfortunate :( would it be possible to backport the necessary changes to current headers and loader?

I know that up until a couple of weeks ago the headers used were the upstream Khronos version, at least when building the package for Arch Linux we used to clone that repo and symlink it. So I guess the modifications can be forwarded there?

ranisalt commented 4 years ago

To clarify what I'm asking: I just compared side by side the Khronos OpenCL headers repo with the opencl2.2 directory from this repo and, apart from the outdated copyright notice, whitespace changes and some additions for OpenCL 3.0 (which should not affect, as they are guarded by defines), there is no relevant difference. So it's unclear to me why I couldn't just use Khronos upstream to build ROCm OpenCL Runtime, as it successfully builds and runs (checked output of clinfo and clpeak).

There are some pieces of code exclusive to this repo, but those could easily be pushed to the Khronos repo for a unified header interface.

CamilleScholtz commented 3 years ago

Running this on my machine, and I can confirm that it works just fine.

Mystro256 commented 2 years ago

I made some changes to help unbundle the ICD loader via the option "BUILD_ICD", e.g. cmake -DBUILD_ICD=OFF. https://github.com/RadeonOpenCompute/ROCm-OpenCL-Runtime/pull/146/commits/e6ba9f6e613728621abe6cf3575d1bce92f620ae

From this pull request: https://github.com/RadeonOpenCompute/ROCm-OpenCL-Runtime/pull/146

I'm not sure that openCL headers can be easily unbundled at this point.