-
Couldn't load subscription status.
- Fork 793
[NFC][SYCL] Drop bfloat16::Bfloat16StorageT
#20180
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
Conversation
It's not part of the specification and should have never been a public type alias inside `bfloat16`. There aren't too many uses of it (`bfloat16` itself and `convertToOpenCLType`/`vec::convert`) so I don't see much value in creating a named type alias.
| using Bfloat16StorageT | ||
| __SYCL_DEPRECATED("bfloat16::Bfloat16StorageT is non-standard and has " | ||
| "been deprecated.") = uint16_t; | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We run the risk of removing this as part of the upcoming ABI-break window. Maybe we should have a comment to prevent that from happening? Otherwise there's probably not much point in deprecating it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I say take the risk of breaking someone, because it is unlikely that anyone uses it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm doing it mostly for the trunk users, but I do plan to remove it asap (next major release).
It's not part of the specification and should have never been a public type alias inside
bfloat16. There aren't too many uses of it (bfloat16itself andconvertToOpenCLType/vec::convert) so I don't see much value in creating a named type alias.