Skip to content

[SYCL][NFC] Remove some work-arounds for C++11 #3168

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

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

v-klochkov
Copy link
Contributor

C++14 is required after #3053
Thus some of C++11 workarounds in the code became unnecessary.

Signed-off-by: Vyacheslav N Klochkov [email protected]

C++14 is required after intel#3053
Thus some of C++11 workarounds in the code became unnecessary.

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
@v-klochkov v-klochkov force-pushed the public_vklochkov_fp16_nfc branch from de41532 to d96ecbe Compare February 5, 2021 16:43
@bader
Copy link
Contributor

bader commented Feb 7, 2021

What about similar work-arounds in other files (e.g. accessor.hpp, buffer.hpp, etc.)?

@AGindinson
Copy link
Contributor

@v-klochkov, do you have plans on type traits' refactoring in scope of this shift, or is this something I could volunteer for instead of finalizing #2778?

@v-klochkov v-klochkov changed the title [SYCL][NFC] Remove work-arounds for C++11 [SYCL][NFC] Remove some work-arounds for C++11 Feb 8, 2021
@v-klochkov
Copy link
Contributor Author

@v-klochkov, do you have plans on type traits' refactoring in scope of this shift, or is this something I could volunteer for instead of finalizing #2778?

What about similar work-arounds in other files (e.g. accessor.hpp, buffer.hpp, etc.)?

@bader, This fix removes only some workarounds, not all of them. In particular it removes a bit more than was promised here: #2528 (review)

@v-klochkov
Copy link
Contributor Author

@v-klochkov, do you have plans on type traits' refactoring in scope of this shift, or is this something I could volunteer for instead of finalizing #2778?

@AGindinson , Sure, you can volunteer to do more code cleanup. With C++14 sycl::detail::enable_if_t can be replaced with std::enable_if_t, etc..
C++14 though still does not let you write something like std::is_same_v<T1,T2>

@bader
Copy link
Contributor

bader commented Feb 9, 2021

@v-klochkov, do you have plans on type traits' refactoring in scope of this shift, or is this something I could volunteer for instead of finalizing #2778?

What about similar work-arounds in other files (e.g. accessor.hpp, buffer.hpp, etc.)?

@bader, This fix removes only some workarounds, not all of them. In particular it removes a bit more than was promised here: #2528 (review)

That was my question. Why do we remove only some workarounds and not all of them?

@v-klochkov
Copy link
Contributor Author

That was my question. Why do we remove only some workarounds and not all of them?

There is no rush here. Having everything in one change would require a huge patch, which then would suffer from conflicts, etc.
For example, If someone would want to replace detail::enable_if_t with std::enable_if_t, it would be 44+ files.

In this patch I changed only places with "#if __cplusplus >= 201402L"
Other places that you mentioned above are different as they have (">", not ">=") : "#if __cplusplus > 201402L"

@bader bader merged commit 2d75f9b into intel:sycl Feb 9, 2021
@v-klochkov v-klochkov deleted the public_vklochkov_fp16_nfc branch February 9, 2021 20:43
zahiraam pushed a commit to zahiraam/llvm-1 that referenced this pull request Feb 16, 2021
C++14 is required after intel#3053
Thus some of C++11 workarounds in the code became unnecessary.

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
jsji pushed a commit that referenced this pull request May 30, 2025
- Wrong comment syntax for used
- Fixed return for Get commit sha job

Signed-off-by: Sidorov, Dmitry <[email protected]>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@c3b1fcd7726ae0c
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.

6 participants