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.

Sign up for GitHub

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

Jump to bottom

common: remove support for SyCL older than 2020 #2216

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

common: remove support for SyCL older than 2020 #2216

wants to merge 1 commit into from

Conversation

spalicki
Copy link
Contributor

Removing SYCL 1.2.1 specific code.

@spalicki spalicki requested review from a team as code owners November 14, 2024 20:52
@github-actions github-actions bot added the platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel label Nov 14, 2024
mgouicem
mgouicem reviewed Nov 15, 2024
View reviewed changes
src/graph/utils/sycl_check.hpp
#define DNNL_USE_SYCL121_API 0
#endif
#if defined(__INTEL_LLVM_COMPILER) && __INTEL_LLVM_COMPILER >= 20230000
#elif defined(__LIBSYCL_MAJOR_VERSION) && __LIBSYCL_MAJOR_VERSION >= 6
Copy link
Contributor

Choose a reason for hiding this comment

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

(random) there might be other spots to edit:

  • check SYCL_COMPILER_VERSION macro
  • there might be some spots in cuda/hip backend as well where workarounds were implemented (tagging @t4c1 @sgeor255).

@vpirogov
Copy link
Member

make test

@spalicki
Copy link
Contributor Author

@densamoilov @mgouicem I also wanted to remove CL/sycl.hpp header include since it is there just for compatibility with older SyCL versions. In many places we have:

#if __has_include(<sycl/sycl.hpp>)
#include <sycl/sycl.hpp>
#elif __has_include(<CL/sycl.hpp>)
#include <CL/sycl.hpp>
#else
#error "Unsupported compiler"
#endif

I wanted to simplify it to:

#if __has_include(<sycl/sycl.hpp>)
#include <sycl/sycl.hpp>
#else
#error "Unsupported compiler"
#endif

@densamoilov
Copy link
Contributor

I also wanted to remove CL/sycl.hpp header include since it is there just for compatibility with older SyCL versions.

@spalicki, I think it should be fine.

@spalicki
Copy link
Contributor Author

make test

@spalicki
Copy link
Contributor Author

make test

@spalicki
Copy link
Contributor Author

make test

@spalicki spalicki self-assigned this Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgouicem mgouicem mgouicem left review comments

@rjoursler rjoursler rjoursler approved these changes

@TaoLv TaoLv TaoLv approved these changes

@t4c1 t4c1 Awaiting requested review from t4c1

@sgeor255 sgeor255 Awaiting requested review from sgeor255

@densamoilov densamoilov Awaiting requested review from densamoilov

At least 2 approving reviews are required to merge this pull request.

Assignees

@spalicki spalicki

Labels
platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@spalicki @vpirogov @densamoilov @TaoLv @mgouicem @rjoursler