Skip to content

Conversation

steffenlarsen
Copy link
Contributor

This commit adds the missing std::hash and std::numeric_limits specializations for the sycl::ext::oneapi::bfloat16 class.

This commit adds the missing std::hash and std::numeric_limits
specializations for the sycl::ext::oneapi::bfloat16 class.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen
Copy link
Contributor Author

@gmlueck - These are also specialized for sycl::half despite not being explicitly mentioned by the specification. Is it fine to add these without changing the extensions for bfloat16 or do we need associated extension spec mentions?

Signed-off-by: Larsen, Steffen <[email protected]>
@gmlueck
Copy link
Contributor

gmlueck commented Aug 20, 2025

@gmlueck - These are also specialized for sycl::half despite not being explicitly mentioned by the specification. Is it fine to add these without changing the extensions for bfloat16 or do we need associated extension spec mentions?

I think we should update the bfloat16 extension spec to say that the extension does provide these specializations. Using sycl::half as a precedent is not a good idea because it is so poorly specified currently. (There is a SYCL spec issue about this.)

For example, we could add a new section to the spec titled something like "Standard C++ library specializations"

Comment on lines 155 to 157
// Friend traits.
friend std::numeric_limits<bfloat16>;
friend std::hash<bfloat16>;
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 have any guarantees on the layout/size? If so, can we use bit_cast instead of friendship?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do, but bit_cast in our implementation is only sometimes constexpr, so the friendship for numeric_limits lets it be unconditionally constexpr. For std::hash it should be fine to drop the friendship though!

Comment on lines +630 to +631
// Specialization of some functions in namespace `std`.
namespace std {
Copy link
Contributor

Choose a reason for hiding this comment

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

Subjective, but I prefer

// in global ns
template <> struct std::{type}<types...> { ... };

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 don't have a strong preference, but the current style is what we use for hash in other headers, so I would like to keep it as is. Then we can change it at a global level if we want.

Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen requested a review from a team as a code owner August 21, 2025 06:11
@steffenlarsen steffenlarsen changed the title [SYCL] Add std::hash and std::numeric_limits specialization for bfloat16 [SYCL][Docs] Add std::hash and std::numeric_limits specialization for bfloat16 Aug 21, 2025
@steffenlarsen steffenlarsen removed the request for review from a team August 22, 2025 04:11
@steffenlarsen steffenlarsen merged commit 09780f1 into intel:sycl Aug 22, 2025
28 checks passed
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.

5 participants