Skip to content

common: remove support for SyCL older than 2020 #2216

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
Merged
spalicki merged 1 commit into from
Dec 16, 2024
Merged

common: remove support for SyCL older than 2020 #2216

spalicki merged 1 commit into from
Dec 16, 2024

Conversation

@spalicki
Copy link
Contributor

@spalicki spalicki commented Nov 14, 2024

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 Outdated
#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

@mgouicem mgouicem Nov 15, 2024

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
Contributor

vpirogov commented Nov 15, 2024

make test

densamoilov
densamoilov reviewed Nov 16, 2024
View reviewed changes
@spalicki
Copy link
Contributor Author

spalicki commented Nov 18, 2024

@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

densamoilov commented Nov 19, 2024

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

spalicki commented Nov 19, 2024

make test

@spalicki spalicki force-pushed the spalicki/remove_sycl_121 branch from b29fa15 to aef8703 Compare November 22, 2024 03:27
@spalicki
Copy link
Contributor Author

spalicki commented Nov 22, 2024

make test

@spalicki spalicki force-pushed the spalicki/remove_sycl_121 branch from aef8703 to e3cfd3f Compare November 25, 2024 18:56
@spalicki
Copy link
Contributor Author

spalicki commented Nov 25, 2024

make test

@spalicki spalicki requested review from sgeor255 and t4c1 November 26, 2024 22:35
@spalicki spalicki self-assigned this Nov 26, 2024
@spalicki spalicki requested a review from densamoilov November 27, 2024 18:04
@spalicki spalicki force-pushed the spalicki/remove_sycl_121 branch 2 times, most recently from 2765ee0 to f944b63 Compare December 7, 2024 00:17
densamoilov
densamoilov reviewed Dec 7, 2024
View reviewed changes
@spalicki spalicki force-pushed the spalicki/remove_sycl_121 branch from f944b63 to 999a883 Compare December 9, 2024 22:02
@spalicki
Copy link
Contributor Author

spalicki commented Dec 9, 2024

make test

@vpirogov vpirogov added this to the v3.7 milestone Dec 11, 2024
mgouicem
mgouicem approved these changes Dec 13, 2024
View reviewed changes
@spalicki spalicki merged commit 09f34d3 into main Dec 16, 2024
33 of 34 checks passed
@spalicki spalicki deleted the spalicki/remove_sycl_121 branch December 16, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@densamoilov densamoilov densamoilov left review comments

@hidefromkgb hidefromkgb hidefromkgb approved these changes

@TaoLv TaoLv TaoLv approved these changes

@mgouicem mgouicem mgouicem approved these changes

@rjoursler rjoursler rjoursler approved these changes

@t4c1 t4c1 Awaiting requested review from t4c1

@sgeor255 sgeor255 Awaiting requested review from sgeor255

Assignees

@spalicki spalicki

Labels

platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel

Projects

None yet

Milestone

v3.7

Development

Successfully merging this pull request may close these issues.

7 participants

@spalicki @vpirogov @densamoilov @hidefromkgb @TaoLv @mgouicem @rjoursler