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

bfloat16: faster conversion and assignment from integer types #801

Conversation

N-Dekker
Copy link
Contributor

Added a converting constructor and an assignment operator template from
any integer type to bfloat16_t. Allows significantly faster conversion
and assignment from integer than when using bfloat16_t(float) or
operator=(float), by avoiding an fpclassify(f) call, and skipping
the cases for denormal floats and NaNs.

Copy link

@emfomenk emfomenk left a comment

Choose a reason for hiding this comment

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

LGTM

And thanks for the test!

src/common/bfloat16.hpp Outdated Show resolved Hide resolved
Added a converting constructor and an assignment operator template from
any integer type to `bfloat16_t`. Allows significantly faster conversion
and assignment from an integer than when using `bfloat16_t(float)` or
`operator=(float)`, by avoiding an `fpclassify(f)` call, and skipping
the cases for denormal floats and NaNs.
@N-Dekker N-Dekker force-pushed the bfloat16-faster-conversion-from-integer-types branch from 6c7d534 to 51638e7 Compare August 11, 2020 14:10
@N-Dekker N-Dekker marked this pull request as ready for review August 11, 2020 14:37
@N-Dekker N-Dekker requested a review from emfomenk August 11, 2020 18:51
@emfomenk emfomenk self-assigned this Aug 12, 2020
Copy link

@emfomenk emfomenk left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, Niels!
I will run the internal CI to make sure nothing is broken and will merge it soon.

@emfomenk
Copy link

Merged! Thanks a lot for the contribution, Niels!

@N-Dekker
Copy link
Contributor Author

N-Dekker commented Sep 25, 2020

@emfomenk Hi Evarist! Do you think there is still a chance that the two little commits that we did to src/common/bfloat16.hpp in July, 073e346 (this pull request) and d68271b (pull request #780) will be included with oneDNN v2.0?

Apparently they did not make it into v2.0-beta09/src/common/bfloat16.hpp

@emfomenk
Copy link

Hi @N-Dekker,

Thanks for checking the status!

The dev-v2 branch is being merged with the mainline (master) before every release.
For v2.0-beta09 the merge happened around 07/10, while your commit was upstreamed on 07/20. So, you are right, v2.0-beta09 won't have these commits. However, the current dev-v2 already contains the fix and the upcoming v2.0-beta10 will do so.

Is it fine for you to wait till beta10 (that will be released in a ~month)?

@N-Dekker
Copy link
Contributor Author

@emfomenk Thanks, Evarist! Sure, it's perfectly fine to me to have them in v2.0-beta10!

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