valhalla
valhalla copied to clipboard
8334432: Refine Float16.fma
Adding comments and test cases for Float16.fma.
Progress
- [x] Change must not contain extraneous whitespace
Issue
- JDK-8334432: Refine Float16.fma (Enhancement - P4)
Reviewers
- Jatin Bhateja (@jatin-bhateja - Committer) ⚠️ Review applies to 2a321de9
Contributors
- Raffaello Giulietti
<[email protected]>
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1143/head:pull/1143
$ git checkout pull/1143
Update a local copy of the PR:
$ git checkout pull/1143
$ git pull https://git.openjdk.org/valhalla.git pull/1143/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1143
View PR using the GUI difftool:
$ git pr show -t 1143
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1143.diff
Webrev
:wave: Welcome back darcy! A progress list of the required criteria for merging this PR into lworld+fp16 will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
@jddarcy This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8334432: Refine Float16.fma
Co-authored-by: Raffaello Giulietti <[email protected]>
Reviewed-by: jbhateja, rgiulietti
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been no new commits pushed to the lworld+fp16 branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.
As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@jatin-bhateja) but any other Committer may sponsor as well.
➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).
I've asked my colleague @rgiulietti to help with the error analysis of the current implementation.
The commented-out code in the fma for an inexact product-sum does get executed for the expected test cases. If special-case code for an inexact product-sum turns out need to be needed, this extra code will be deleted before a push.
Webrevs
- 07: Full - Incremental (2d357510)
- 06: Full - Incremental (172eea95)
- 05: Full - Incremental (f3ceea16)
- 04: Full - Incremental (32bb73a6)
- 03: Full - Incremental (46a84462)
- 02: Full - Incremental (ea5f4bf0)
- 01: Full - Incremental (ce044ce2)
- 00: Full (2a321de9)
The analysis done here seems correct, but there is an implicit suspension moment in the last case.
That leaves possibly non-exact product-sum with a combination of product in the subnormal range of Float16 and the c term to be added in being not-small. However, if this product-sum is non-exact, the smaller term from the product, with at most 22 exponent bit positions set, and the the 11 bits from c being summed in, must be separated by at least 53 - (22 + 11) = 20 bit positions otherwise the product-sum would fit in a double. I believe this implies at least one of the double-rounding scenarios cannot occur, in particular a half-way result in the smaller precision, Float16 in this case, rounding differently because sticky bit information from the higher precision was rounded away.
Here's a further analysis of this case.
Double rounding is usually harmless. It is harmful only in two situations:
- The first rounding from the exact value to the extended precision (here
double) happens to be directed toward 0 to a value exactly midway between two adjacent working precision (herefloat16) values, followed by a second rounding from there which again happens to be directed toward 0 to one of these values (the one with lesser magnitude). A single rounding from the exact value to the working precision, in contrast, rounds to the value with larger magnitude. - Symmetrically, the first rounding to the extended precision happens to be directed away from 0 to a value exactly midway between two adjacent working precision values, followed by a second rounding from there which again happens to be directed away from 0 to one of these values (the one with larger magnitude). However, a single rounding from the exact value to the working precision rounds to the value with lesser magnitude.
In any other case double rounding is innocuous, returning the same value as a single rounding to the working precision.
We only need to ensure that the first rounding to double does not produce the midpoint of two adjacent float16 values.
-
If a·b and c have the same sign, the sum a·b + c has a significand with a large gap of 20 or more 0s between the bits of the signifcand of c to the left (at most 11 bits) and those of the product a·b to the right (at most 22 bits). The rounding bit for the final working precision of
float16is the leftmost 0 in the gap.- If rounding to
doubleis directed toward 0, all the 0s in the gap are preserved, thus thefloat16rounding bit is unaffected and remains 0. This means that thedoublevalue is not the midpoint of two adjacentfloat16values, so double rounding is harmless. - If rounding to
doubleis directed away form 0, the rightmost 0 in the gap might be replaced by a 1, but the others are unaffected, including thefloat16rounding bit. Again, this shows that thedoublevalue is not the midpoint of two adjacentfloat16values, and double rounding is innocuous.
- If rounding to
-
If a·b and c have opposite signs, the sum a·b + c the long gap of 0s above is replaced by a long gap of 1s. The
float16rounding bit is the leftmost 1 in the gap, or the second leftmost 1 iff c is a power of 2. In both cases, the rounding bit is followed by at least another 1.- If rounding to
doubleis directed toward 0, thefloat16rounding bit and its follower are preserved and both 1, so thedoublevalue is not the midpoint of two adjacentfloat16values, and double rounding is harmless. - If rounding to
doubleis directed away from 0, thefloat16rounding bit and its follower are either preserved (both 1), or both switch to 0. Either way, thedoublevalue is again not the midpoint of two adjacentfloat16values, and double rounding is harmless.
- If rounding to
/contributor @rgiulietti
@jddarcy Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:
/contributor add @openjdk-bot/contributor add duke/contributor add J. Duke <[email protected]>
User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.
/contributor add @rgiulietti
@jddarcy
Contributor Raffaello Giulietti <[email protected]> successfully added.
The analysis done here seems correct, but there is an implicit suspension moment in the last case.
That leaves possibly non-exact product-sum with a combination of product in the subnormal range of Float16 and the c term to be added in being not-small. However, if this product-sum is non-exact, the smaller term from the product, with at most 22 exponent bit positions set, and the the 11 bits from c being summed in, must be separated by at least 53 - (22 + 11) = 20 bit positions otherwise the product-sum would fit in a double. I believe this implies at least one of the double-rounding scenarios cannot occur, in particular a half-way result in the smaller precision, Float16 in this case, rounding differently because sticky bit information from the higher precision was rounded away.
Here's a further analysis of this case.
Double rounding is usually harmless. It is harmful only in two situations:
* The first rounding from the exact value to the extended precision (here `double`) happens to be directed _toward_ 0 to a value exactly midway between two adjacent working precision (here `float16`) values, followed by a second rounding from there which again happens to be directed _toward_ 0 to one of these values (the one with lesser magnitude). A single rounding from the exact value to the working precision, in contrast, rounds to the value with larger magnitude. * Symmetrically, the first rounding to the extended precision happens to be directed _away_ from 0 to a value exactly midway between two adjacent working precision values, followed by a second rounding from there which again happens to be directed _away_ from 0 to one of these values (the one with larger magnitude). However, a single rounding from the exact value to the working precision rounds to the value with lesser magnitude.In any other case double rounding is innocuous, returning the same value as a single rounding to the working precision. We only need to ensure that the first rounding to
doubledoes not produce the midpoint of two adjacentfloat16values.* If a·b and c have the same sign, the sum a·b + c has a significand with a large gap of 20 or more 0s between the bits of the signifcand of c to the left (at most 11 bits) and those of the product a·b to the right (at most 22 bits). The rounding bit for the final working precision of `float16` is the leftmost 0 in the gap. * If rounding to `double` is directed toward 0, all the 0s in the gap are preserved, thus the `float16` rounding bit is unaffected and remains 0. This means that the `double` value is _not_ the midpoint of two adjacent `float16` values, so double rounding is harmless. * If rounding to `double` is directed away form 0, the rightmost 0 in the gap might be replaced by a 1, but the others are unaffected, including the `float16` rounding bit. Again, this shows that the `double` value is _not_ the midpoint of two adjacent `float16` values, and double rounding is innocuous. * If a·b and c have opposite signs, the sum a·b + c the long gap of 0s above is replaced by a long gap of 1s. The `float16` rounding bit is the leftmost 1 in the gap, or the second leftmost 1 iff c is a power of 2. In both cases, the rounding bit is followed by at least another 1. * If rounding to `double` is directed toward 0, the `float16` rounding bit and its follower are preserved and both 1, so the `double` value is _not_ the midpoint of two adjacent `float16` values, and double rounding is harmless. * If rounding to `double` is directed away from 0, the `float16` rounding bit and its follower are either preserved (both 1), or both switch to 0. Either way, the `double` value is again _not_ the midpoint of two adjacent `float16` values, and double rounding is harmless.
Thanks; that agrees with the sketch of those cases I was working on. I'll update the explanatory comment and cleanup the implementation accordingly.
Getting back to this PR, I pushed an update to the explanation about why Float16 fma is double arithmetic is sufficient and an initial update of the tests.
More test augmentation as previously discussed on the way.
Hi @jddarcy , Some queries / comments.
Hi @jatin-bhateja ,
Please re-review the current state of the PR and if it looks good, I'm integrate it for sponsorship. Thanks.
Thanks for the review comments @rgiulietti ; the most recent push should address the points you raised.
@jatin-bhateja , if you re-review the current state of the PR, I'll integrate the changes so they can be sponsored. Thanks.
/integrate
@jddarcy Your change (at version 2d357510c6f14f2284ccb126f711877788e16402) is now ready to be sponsored by a Committer.
/sponsor
Going to push as commit be93540f9bed7b18e0e788f9635782554415d920.
@jatin-bhateja @jddarcy Pushed as commit be93540f9bed7b18e0e788f9635782554415d920.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.