Skip to content

Update SYCL_INTEL_bf16_conversion.asciidoc #5248

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

Closed
wants to merge 5 commits into from

Conversation

AlexeySotkin
Copy link
Contributor

Align the document with changes made in #4989

Align the document with changes made in intel#4989
@AlexeySotkin AlexeySotkin requested a review from a team as a code owner December 30, 2021 13:08
@JackAKirk
Copy link
Contributor

Am I right in understanding that there is no reason this proposal can't be implemented for other backends (cuda)?
The intel backend specific element of the implementation seems to be only via the spirv functions __spirv_ConvertFToBF16INTEL and __spirv_ConvertBF16ToFINTEL. Is this correct?

Would it be acceptable for us to add NVPTX backend implementations of these spirv functions?

For example __spirv_ConvertFToBF16INTEL (perhaps suitably renamed?) would call the cuda builtin that performs the conversion using round to nearest even (as per the proposal).

Thanks

@JackAKirk
Copy link
Contributor

Am I right in understanding that there is no reason this proposal can't be implemented for other backends (cuda)? The intel backend specific element of the implementation seems to be only via the spirv functions __spirv_ConvertFToBF16INTEL and __spirv_ConvertBF16ToFINTEL. Is this correct?

Would it be acceptable for us to add NVPTX backend implementations of these spirv functions?

For example __spirv_ConvertFToBF16INTEL (perhaps suitably renamed?) would call the cuda builtin that performs the conversion using round to nearest even (as per the proposal).

Thanks

I think I see that the implementations of the spirv functions that are labelled INTEL are hidden. I think I understand that they will remain intel specific: in this case, and assuming that this extension could be generalized, what would be the preferred way of generalizing this extension to support other vendor's backends? A higher level spirv function that would call INTEL labelled spirv/other backend specific spirv depending on the backend compiled for?

Thanks

@AlexeySotkin AlexeySotkin requested review from MrSidims and a team January 10, 2022 16:04
@bader bader requested review from a team and removed request for a team January 10, 2022 18:22
@JackAKirk
Copy link
Contributor

I made/tested an example draft oneapi namespace extension supporting CUDA here: #5393

@rdeodhar
Copy link
Contributor

OK, I've updated the PR to remove from_bits() and raw() and kept the other changes.

@gmlueck
Copy link
Contributor

gmlueck commented May 2, 2022

OK, I've updated the PR to remove from_bits() and raw() and kept the other changes.

It looks like from_bits and raw are still in the synopsis.

This PR also needs to be merged with the main branch and updated. The extension spec is now at "sycl/doc/extensions/experimental/sycl_ext_oneapi_bfloat16.asciidoc".

@rdeodhar
Copy link
Contributor

rdeodhar commented May 2, 2022

Moved the required changes to "sycl/doc/extensions/experimental/sycl_ext_oneapi_bfloat16.asciidoc".

@rdeodhar rdeodhar requested a review from gmlueck May 3, 2022 05:38
We decided in the PR comments to remove this implicit conversion to
`uint16_t`, and it was removed from the table describing the member
functions.  I think it was just a merge mistake that this conversion
was re-added to the synopsis, so remove it.
@MrSidims
Copy link
Contributor

Closing in favor of #6524 .

@MrSidims MrSidims closed this Sep 21, 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.

7 participants