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

Should be sycl::vec::as<bool> clarified/prohibited? #455

Open
KornevNikita opened this issue Aug 16, 2023 · 5 comments · May be fixed by #724
Open

Should be sycl::vec::as<bool> clarified/prohibited? #455

KornevNikita opened this issue Aug 16, 2023 · 5 comments · May be fixed by #724
Labels
clarification Something is unclear

Comments

@KornevNikita
Copy link
Contributor

KornevNikita commented Aug 16, 2023

After recent SYCL-CTS change, we've noticed that here we're calling sycl::vec::as<bool> for different sycl::vec<T>, where T is char, int, float and so on, like:

auto inputVec = sycl::vec<char, 8>(0, 1, 2, 3, 4, 5, 6, 7);
auto asVec = inputVec.as<sycl::vec<bool, 8>>();

Looks like it's not legal. SYCL2020 defines template <typename asT> asT as() const method as:

Bitwise reinterprets this SYCL vec as a SYCL vec of a different element type and number of elements specified by asT. The new SYCL vec type must have the same storage size in bytes as this SYCL vec.

According to https://en.cppreference.com/w/cpp/numeric/bit_cast:
If there is no value of type To corresponding to the value representation produced, the behavior is undefined.

So, it looks like as<bool> should be prohibited as it's UB.

@KornevNikita
Copy link
Contributor Author

@fraggamuffin
Copy link

deprecate in future and use static casts to give alternative?
warn to user that it is UB
CTS impact: remove the test

@keryell
Copy link
Member

keryell commented Aug 17, 2023

Just adding for information a link to the relevant part in the current C++ specification draft https://eel.is/c++draft/bit.cast

@fraggamuffin
Copy link

use C++ std::bit.cast

KornevNikita added a commit to KornevNikita/SYCL-CTS that referenced this issue Aug 18, 2023
KornevNikita added a commit to KornevNikita/SYCL-CTS that referenced this issue Aug 18, 2023
@tomdeakin
Copy link
Contributor

Awaiting PR on spec to clarify this. Already removed the test in the CTS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Something is unclear
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants