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.

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

Clarify vec::as behavior #724

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

Clarify vec::as behavior #724

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions adoc/chapters/programming_interface.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18019,14 +18019,9 @@ vec<ConvertT, NumElements> convert() const
a@
[source]
----
template <typename asT> asT as() const
template <typename AsT> AsT as() const
----
a@ Bitwise reinterprets this SYCL [code]#vec# as a SYCL [code]#vec# of a
different element type and number of elements specified by [code]#asT#.
The new SYCL [code]#vec# type must have the same storage size in bytes as
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.


a@
[source]
Expand Down