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

Clarify vec::as behavior #724

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Pennycook
Copy link
Contributor

The original description did not cover various corner-cases that could lead to undefined behavior (e.g., reinterpreting using vec::as<bool>).

Since C++20 defines std::bit_cast, and SYCL 2020 pre-adopts sycl::bit_cast, defining vec::as equivalent to bit_cast<AsT> has two advantages:

  • It simplifies the specification of vec::as significantly; and
  • It enables vec::as to inherit undefined behavior, etc from bit_cast.

Closes #455.

The original description did not cover various corner-cases that could lead to
undefined behavior (e.g., reinterpreting using vec::as<bool>).

Since C++20 defines std::bit_cast, and SYCL 2020 pre-adopts sycl::bit_cast,
defining vec::as equivalent to bit_cast<AsT> has two advantages:

- It simplifies the specification of vec::as significantly; and
- It enables vec::as to inherit undefined behavior, etc from bit_cast.
this SYCL [code]#vec#, and the size of the elements in the new SYCL
[code]#vec# ([code]#NumElements * sizeof(DataT)#) must be the same as the
size of the elements in this SYCL [code]#vec#.
a@ Equivalent to [code]#bit_cast<AsT>(*this)#.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need some additional words to say that bit-cast from a 3-element vec to a 4-element vec produces undefined results? See #447.

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 think this might be covered by the definition of bit_cast, but we could still add some clarification.

The C++20 definition says that "Each bit of the value representation of the result is equal to the corresponding bit in the object representation of from". The definition in the latest draft is much more verbose, and includes statements like "A bit in the value representation of the result is indeterminate if it does not correspond to a bit in the value representation of from or corresponds to a bit for which the smallest enclosing object is not within its lifetime or has an indeterminate value."

By either definition, I think the intended behavior is clear for vec::as. Since a 3-element vec is specified to have the same alignment and size as a 4-element vec, the bits in the fourth element of the result must come from padding bits that are not part of the value representation of the 3-element vec. By the C++20 definition, the bits in the fourth element of the result will be padding bits of unknown value. By the C++23 definition, the bits in the fourth element of the result are explicitly "indeterminate" because they didn't come from the value representation.

Maybe we could just say something like:

Note: Since the object representation of a vec<T, 3> contains padding bits (see Section 4.14.2.6), using as or bit_cast to create a vec with a different number of elements can lead to undefined behavior.

...or we could even put the note in Section 4.14.2.6, since it applies to use of bit_cast as well.

Note that I said can lead to undefined behavior, because for certain operations some of the bits are guaranteed to be well-defined. For example, when going from vec<T, 3> to vec<T, 4>, the first three elements should be guaranteed to have well-defined values (and this is the behavior of DPC++, at least).

I realize now that this is actually weaker than the wording that was there before, but I think it's a more accurate reflection of how implementations behave. And it would be odd for as to have a restriction that a developer could work around trivially by using bit_cast instead.

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.

Should be sycl::vec::as<bool> clarified/prohibited?
3 participants