Skip to content

Conversation

@gmlueck
Copy link
Contributor

@gmlueck gmlueck commented Dec 3, 2024

This is change 7 of 9 that fix problems with the specification of the vec class. An implementation that follows the existing specification would not accept common code patterns and would not pass the CTS. None of the existing implementations actually follow the existing specification.

This change clarifies that std::byte is a legal element type for vec. This is one possible interpretation of the previous wording where we said that the element data type must be a "basic scalar type". We think that phrase included sycl::byte. However, sycl::byte was deprecated in SYCL 2020 saying that std::byte is a replacement.

If we allow std::byte, we need to adjust the constraints for the various vec operators. We decided to allow these operators on vec of std::byte, which mostly follows the C++ rules for operators on plain std::byte:

  • Comparison: ==, !=, <, <=, >, >=
  • Bitwise: &, |, ^, ~, &=, |=, ^=

I decided it was clearer to rephrase the constraints to say which types are allowed rather than listing the types that are not allowed. For example, this results in phrasing like:

Available only when DataT is an integral type.

Rather than:

Available only when: DataT != float && DataT != double &&
DataT != half && DataT != std::byte.

These changes correspond to slide 26 of the presentation that was discussed in the WG meetings.

This is change 7 of 9 that fix problems with the specification of the
`vec` class.  An implementation that follows the existing specification
would not accept common code patterns and would not pass the CTS.  None
of the existing implementations actually follow the existing
specification.

This change clarifies that `std::byte` is a legal element type for
`vec`.  This is one possible interpretation of the previous wording
where we said that the element data type must be a "basic scalar type".
We think that phrase included `sycl::byte`.  However, `sycl::byte` was
deprecated in SYCL 2020 saying that `std::byte` is a replacement.

If we allow `std::byte`, we need to adjust the constraints for the
various `vec` operators.  We decided to allow these operators on `vec`
of `std::byte`, which mostly follows the C++ rules for operators on
plain `std::byte`:

* Comparison: ==, !=, <, <=, >, >=
* Bitwise: &, |, ^, ~, &=, |=, ^=

I decided it was clearer to rephrase the constraints to say which types
are allowed rather than listing the types that are not allowed.  For
example, this results in phrasing like:

> Available only when DataT is an integral type.

Rather than:

> Available only when: DataT != float && DataT != double &&
> DataT != half && DataT != std::byte.

These changes correspond to slide 26 of the presentation that was
discussed in the WG meetings.
Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Thanks for the generalization.

aelovikov-intel added a commit to aelovikov-intel/SYCL-CTS that referenced this pull request Feb 26, 2025
aelovikov-intel added a commit to aelovikov-intel/SYCL-CTS that referenced this pull request Feb 26, 2025
@gmlueck
Copy link
Contributor Author

gmlueck commented Feb 26, 2025

CTS tests added in KhronosGroup/SYCL-CTS#1058

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Mar 31, 2025
KhronosGroup/SYCL-Docs#670

Technically, we also implement part of
KhronosGroup/SYCL-Docs#674 (`std::byte` as
element type) here, but there is no reasonable way to make them
completely independent.

This is built on top of
intel#17712 and
intel#17713.

With these three pieces in place we can keep building CTS successfully,
so enable this new implementation (via `__SYCL_USE_PREVIEW_VEC_IMPL`)
automatically under `-fpreview-breaking-changes` mode.
aelovikov-intel added a commit to intel/llvm that referenced this pull request Apr 2, 2025
Implements KhronosGroup/SYCL-Docs#670.

Technically, we also implement part of
KhronosGroup/SYCL-Docs#674
(`std::byte` as element type) here, but there is no reasonable way 
to make them completely independent.

This is built on top of #17712 and
#17713.
@tomdeakin
Copy link
Contributor

KhronosGroup/SYCL-CTS#1058 CTS now merged

@tomdeakin
Copy link
Contributor

WG approved to merge

@gmlueck gmlueck merged commit ee2b2f8 into KhronosGroup:main May 22, 2025
3 checks passed
gmlueck added a commit to gmlueck/SYCL-Docs that referenced this pull request May 22, 2025
Cherry pick KhronosGroup#674 from main
(cherry picked from commit ee2b2f8)
@gmlueck gmlueck deleted the gmlueck/vec-byte branch May 22, 2025 18:50
carbotaniuman pushed a commit to carbotaniuman/SYCL-CTS that referenced this pull request Aug 4, 2025
carbotaniuman pushed a commit to carbotaniuman/SYCL-CTS that referenced this pull request Aug 4, 2025
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.

4 participants