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

cl_intel_bfloat16_conversions #762

Merged
merged 3 commits into from
Sep 14, 2022

Conversation

bashbaug
Copy link
Contributor

This PR adds the cl_intel_bfloat16_conversions specification that adds built-in functions to convert between 32-bit floating point values and bfloat16 values.

The related SPIR-V extension draft for bfloat16 conversions can be found here:

https://github.com/intel/llvm/blob/sycl/sycl/doc/design/spirv-extensions/SPV_INTEL_bfloat16_conversion.asciidoc

I will keep this as a draft PR and extension version 0.9.0 initially to allow for last minute changes before publication, but I believe the writeup is complete and I would appreciate any feedback. Thanks!

@bashbaug bashbaug marked this pull request as draft February 18, 2022 21:35
haonanya pushed a commit to haonanya/SPIRV-LLVM-Translator that referenced this pull request Jul 6, 2022
svenvh pushed a commit to KhronosGroup/SPIRV-LLVM-Translator that referenced this pull request Jul 6, 2022
haonanya pushed a commit to haonanya/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2022
haonanya pushed a commit to haonanya/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2022
haonanya pushed a commit to haonanya/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2022
svenvh pushed a commit to KhronosGroup/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2022
haonanya pushed a commit to haonanya/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2022
haonanya pushed a commit to haonanya/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2022
svenvh pushed a commit to KhronosGroup/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2022
haonanya pushed a commit to haonanya/SPIRV-LLVM-Translator that referenced this pull request Jul 13, 2022
svenvh pushed a commit to KhronosGroup/SPIRV-LLVM-Translator that referenced this pull request Jul 28, 2022
@bashbaug bashbaug marked this pull request as ready for review August 26, 2022 20:40
@bashbaug bashbaug changed the title WIP: cl_intel_bfloat16_conversions cl_intel_bfloat16_conversions Aug 26, 2022
Copy link
Contributor

@alycm alycm left a comment

Choose a reason for hiding this comment

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

LGTM, just a few typos.

Co-authored-by: Alastair Murray <[email protected]>
@bashbaug
Copy link
Contributor Author

Thanks, that's embarassing. I'm not sure if my spell checker was too aggressive or not aggressive enough! I've accepted all three of the spelling fixes.

@alycm
Copy link
Contributor

alycm commented Sep 14, 2022

Some comments that are not part of my review because they are technical comments on a shipping vendor extension, but leaving them here for posterity should this ever be considered more widely.

Please note that this extension currently does not introduce a bfloat16 type to OpenCL C and instead the built-in functions convert to or from a ushort 16-bit unsigned integer type with a bit pattern that represents a bfloat16 value.

Not adding a new type is interesting as there is precedence for a storage-only floating-point type in OpenCL (half when cl_khr_fp16 is not supported). The only advantages that I can think of doing so is reduced implementation work, and opening the possibility for using integer arithmetic operations on the type, i.e. bit-level manipulation of the bfloat16.

Because bfloat16 ends with a number this does lead to awkward function names like intel_convert_bfloat1616_as_ushort16, but the awkward-ness is preferable to the ambiguity without the vector size suffix.

If we decide to add a true bfloat16 type we should consider other names that do not end in a number (bfloat16_t?).

Yes, the paired numbers aren't great for reading, it requires both knowing the bfloat16 is a type, and that type**n** is the vector convention to be able to read something like bloat162.

@alycm alycm merged commit 9c65735 into KhronosGroup:main Sep 14, 2022
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.

2 participants