Skip to content

[NFC][SYCL] Remove checks for C++17 #7687

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
merged 1 commit into from
Dec 8, 2022
Merged

[NFC][SYCL] Remove checks for C++17 #7687

merged 1 commit into from
Dec 8, 2022

Conversation

aelovikov-intel
Copy link
Contributor

This has been the minimally supported version for some time and we have a static_assert to guard for it. No need to conditionally disable parts of the codebase.

@aelovikov-intel
[NFC][SYCL] Remove checks for C++17
59b621a 
This has been the minimally supported version for some time and we have
a static_assert to guard for it. No need to conditionally disable parts
of the codebase.
@aelovikov-intel aelovikov-intel requested a review from a team as a code owner December 7, 2022 18:06
aelovikov-intel
aelovikov-intel commented Dec 7, 2022
View reviewed changes
sycl/include/sycl/detail/defines_elementary.hpp
@@ -37,9 +37,7 @@
#endif

#ifndef __SYCL_DEPRECATED
// The deprecated attribute is not supported in some situations(e.g. namespace)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not touching lines 60 and 76 because those should be addressed by eliminating the defines and updating the code base to use the C++17 features without wrapping them behind the macros.

@aelovikov-intel aelovikov-intel mentioned this pull request Dec 7, 2022
Merged
@aelovikov-intel
Copy link
Contributor Author 

Unresolved Tests (5):
  SYCL :: Assert/assert_in_kernels.cpp
  SYCL :: Assert/assert_in_multiple_tus.cpp
  SYCL :: Assert/assert_in_multiple_tus_one_ndebug.cpp
  SYCL :: Assert/assert_in_one_kernel.cpp
  SYCL :: Assert/assert_in_simultaneously_multiple_tus_one_ndebug.cpp

Known - #7634.

cperkinsintel
cperkinsintel approved these changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

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

Sweet! LGTM.

Let the if constexpr flow like water!

@aelovikov-intel
Copy link
Contributor Author

@intel/llvm-gatekeepers , this PR is ready.

@pvchupin pvchupin merged commit c103a6a into intel:sycl Dec 8, 2022
@AlexeySachkov AlexeySachkov mentioned this pull request Dec 9, 2022
Merged
@aelovikov-intel aelovikov-intel deleted the c++17-checks-remove branch May 1, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cperkinsintel cperkinsintel cperkinsintel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aelovikov-intel @cperkinsintel @pvchupin