Skip to content

SYCL_INTEL_bf16_conversion spec #4237

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

Jump to bottom
Merged
merged 8 commits into from
Aug 19, 2021
Merged

SYCL_INTEL_bf16_conversion spec #4237

merged 8 commits into from
Aug 19, 2021

Conversation

AlexeySotkin
Copy link
Contributor

Signed-off-by: Alexey Sotkin [email protected]

@AlexeySotkin AlexeySotkin requested a review from a team as a code owner August 2, 2021 16:11
@AlexeySotkin AlexeySotkin mentioned this pull request Aug 2, 2021
Merged
@AlexeySotkin AlexeySotkin changed the title SYCL_INTEL_bf16_conversion first draft SYCL_INTEL_bf16_conversion spec Aug 3, 2021
@MrSidims MrSidims self-requested a review August 3, 2021 11:23
MrSidims
MrSidims previously approved these changes Aug 3, 2021
View reviewed changes
MrSidims
MrSidims reviewed Aug 3, 2021
View reviewed changes
Copy link
Contributor

@MrSidims MrSidims left a comment

Choose a reason for hiding this comment

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

We aren't expecting to have any numerical limits for bfloat16 type, are we?

@MrSidims MrSidims mentioned this pull request Aug 3, 2021
Merged
MrSidims added a commit to MrSidims/llvm-test-suite that referenced this pull request Aug 3, 2021
@MrSidims
[SYCL] Add BFloat16 feature end-to-end test
636443f 
Spec: intel/llvm#4237
Implementation: intel/llvm#4213

Signed-off-by: Dmitry Sidorov <[email protected]>
@AlexeySotkin
Applying review comments
8d9066b 
Default constructor, copy constructor and destructor are explicitly declared as
defult.
Add clarification that the extension is an optional kernel feture
Rounding mode is specified
Minor improvement to the example
Fix a typo
@AlexeySotkin
Copy link
Contributor Author

We aren't expecting to have any numerical limits for bfloat16 type, are we?

This extension doesn't define the bfloat16 as a type, so I think defining the limits for bfloat16 is out of scope of this extension.

mlychkov
mlychkov reviewed Aug 5, 2021
View reviewed changes
@MrSidims MrSidims mentioned this pull request Aug 5, 2021
Closed
MrSidims added a commit to MrSidims/llvm that referenced this pull request Aug 12, 2021
@MrSidims
[SYCL] Add BFloat16 aspect
423ff17 
Currently this aspect is mapped on a device info which is
being proved by get_platform_name function, claiming, that any
Intel GPU device supports BFloat16 extension. Once we have
a lower-level spec we can re-map it on the appropriate OpenCL
extension.

Spec:
intel#4237

Signed-off-by: Dmitry Sidorov <[email protected]>
gmlueck
gmlueck reviewed Aug 13, 2021
View reviewed changes
@AlexeySotkin
Update SYCL_INTEL_bf16_conversion
a7354b4 
Add explicit conversion functions
Add operator overloadings
Apply code review suggestions
MrSidims
MrSidims previously approved these changes Aug 17, 2021
View reviewed changes
gmlueck
gmlueck reviewed Aug 17, 2021
View reviewed changes
@AlexeySotkin
Update SYCL_INTEL_bf16_conversion.asciidoc
cd6d55c 
Remove `uint16_t` constructor
@AlexeySotkin AlexeySotkin requested a review from MrSidims August 18, 2021 17:57
@bader bader added Documentation Missing documentation for the code, compiler or runtime features, etc. spec extension All issues/PRs related to extensions specifications labels Aug 19, 2021
@bader bader merged commit 9f8cc3a into intel:sycl Aug 19, 2021
@mkinsner
Copy link

I notice that the SYCL extension here defines comparison and arithmetic operations, but (1) is named "conversion" in the extension name and (2) the SPIR-V extension includes only conversions. Are math/comparison ops already handled in SPV somehow, and should we rename the SYCL extension to be something more general than "conversions"?

@AlexeySotkin
Copy link
Contributor Author

@mkinsner thanks for feedback.

Are math/comparison ops already handled in SPV somehow

No, we have only conversion in SPIR-V. Math/comparison is done on float type.

should we rename the SYCL extension to be something more general than "conversions"?

Makes sense. Any ideas? SYCL_INTEL_bfloat16?

@gmlueck
Copy link
Contributor

gmlueck commented Aug 23, 2021

Makes sense. Any ideas? SYCL_INTEL_bfloat16?

+1

vladimirlaz pushed a commit to intel/llvm-test-suite that referenced this pull request Sep 1, 2021
@MrSidims
[SYCL] Add BFloat16 feature end-to-end test (#390)
704201a 
Spec: intel/llvm#4237
Implementation: intel/llvm#4213

Signed-off-by: Dmitry Sidorov <[email protected]>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
@MrSidims
[SYCL] Add BFloat16 feature end-to-end test (intel/llvm-test-suite#390)
84dab2d 
Spec: intel#4237
Implementation: intel#4213

Signed-off-by: Dmitry Sidorov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmlueck gmlueck gmlueck approved these changes

@mlychkov mlychkov Awaiting requested review from mlychkov

@jbrodman jbrodman Awaiting requested review from jbrodman

@MrSidims MrSidims Awaiting requested review from MrSidims

Assignees
No one assigned
Labels
Documentation Missing documentation for the code, compiler or runtime features, etc. spec extension All issues/PRs related to extensions specifications
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@AlexeySotkin @mkinsner @gmlueck @jbrodman @MrSidims @mlychkov @bader