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

Update and fix SYCL headers #20

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

ianayl
Copy link

@ianayl ianayl commented Aug 19, 2024

I noticed that SYCL headers were outdated and were failing to build when following build instructions in the README. This PR:

  1. Adds the necessary includes in cmake files for builds with -DBUILD_SYCL=on to compile successfully
  2. Removes obsolete references to sycl::info::device::opencl_c_version, which is no longer in the SYCL spec
  3. Changes to use new <sycl/sycl.hpp> headers instead of obsolete <cl/sycl.hpp>

I would like to raise attention to point 2 however: I am unsure if printing the opencl_c_version is important, but judging by the fact that the SYCL spec does not provide alternatives for this in newer specs, I am assuming this is no longer applicable. If the e.g. opencl version is still important debugging information, please let me know and I will add a print for e.g. sycl::info::device::version instead. Thanks for understanding!

Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check that a few of these changes are required? I seem to be able to build fine without them (tested with the 2024.2 SYCL compiler on Linux). Thanks!

source/benchmarks/CMakeMacrosAddBenchmark.cmake Outdated Show resolved Hide resolved
source/framework/utility/CMakeLists.txt Outdated Show resolved Hide resolved
source/tools/show_devices_sycl/CMakeLists.txt Outdated Show resolved Hide resolved
@ianayl
Copy link
Author

ianayl commented Sep 9, 2024

Could you check that a few of these changes are required? I seem to be able to build fine without them (tested with the 2024.2 SYCL compiler on Linux). Thanks!

I don't actually remember what my environment was that only worked with these changes, but I am unable to replicate that environment now. I've gone ahead and removed the changes, my bad on that part!

Copy link
Contributor

@MichalMrozek MichalMrozek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ip+1

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