erts: Fix float16 construction in the software fallback#11334
Open
a1x-an wants to merge 1 commit into
Open
Conversation
Contributor
CT Test Results 3 files 136 suites 50m 49s ⏱️ Results for commit 6530318. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
Contributor
|
Thanks for the PR! Can you rebase this onto the tag |
The software conversion used when _Float16 is not available (notably
all official Windows/MSVC builds) produced wrong bit patterns for
nearly all values that are subnormal in binary16, and misrounded some
normal values:
1> <<F:16/float>> = <<0,51>>. %% 51 * 2^-24, exact
2> <<F:16/float>>.
<<0,39>> %% construct(match(B)) =/= B
The subnormal branch of f32_to_f16() OR:ed the implicit leading bit
into position 23 after the mantissa had already been reduced to its
top 10 bits, and used a whole ULP as the rounding increment. The
normal branch ignored bits 9..0 in its sticky test, so values just
above a rounding midpoint could round down as if they were ties.
Additionally, converting through float (double -> float -> binary16)
rounds twice, so results could differ from the native _Float16 path
for the same input.
Replace the fallback with a direct integer-exact double -> binary16
conversion with a single round-to-nearest-even step, giving the same
results as the native path. The match direction (f16_to_f32) was
correct and is unchanged.
Verified exhaustively against a compiler-provided _Float16 oracle:
all 2^32 float32 inputs, construct/match identity over all finite
binary16 values, and 10^9 random doubles produce identical results.
Also verified in a patched emulator built from maint on x86_64 Linux
with the fallback both disabled (native path, unchanged) and forced
via FLOAT16_IS_CONVERTIBLE (fixed path, now identical to native).
9aa1b6f to
6530318
Compare
Author
|
Done, rebased onto |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes GH-11332.
The software float16 conversion used when
_Float16is not available(notably all official Windows/MSVC builds) produced wrong bit patterns for
nearly all values that are subnormal in binary16, and misrounded some normal
values, so the emulator could not re-construct a value it had just matched:
Three defects in
f32_to_f16()(erts/emulator/beam/erl_bits.c):after the mantissa had already been reduced to its top 10 bits, then
shifted by an exponent-derived amount - producing garbage for all
binary16 subnormals except a few accidental fixed points. The rounding
increment was also a full ULP (
1 << shift) where half an ULP isrequired.
above a rounding midpoint whose sticky bits lie only in the low bits
rounded down as if they were ties (when the kept LSB was even).
f32_to_f16((float) x)) rounds twice(double→float→binary16), so results could differ from the native
single-rounding
(_Float16) xpath even with 1) and 2) fixed:<<1.000488282181322574615478515625:16/float>>(= 1 + 2⁻¹¹ + 2⁻³⁰)gave 0x3C00 through the fallback but 0x3C01 on native builds.
The fallback is replaced by a direct integer-exact double→binary16
conversion with a single round-to-nearest-even step, giving results
identical to the native path. The match direction (
f16_to_f32) wascorrect and is unchanged.
Verification
_Float16oracle(gcc 13): exhaustive sweep of all finite float32 inputs - 0 mismatches for
the new conversion (the current one disagrees on 417,333,236 inputs);
construct∘match identity over all finite binary16 values - 0 failures
(currently 2026); 10⁹ random finite doubles - 0 mismatches.
cl /O2- the toolchain of the official Windows builds, which lacks_Float16and therefore uses this fallback) and with gcc-O2produces bit-identical outputs for both the old and the new conversion
over all finite float32 inputs (FNV-1a digests over the full sweep
match exactly), so the fix is compiler-independent.
mainton x86_64 Linux (musl/gcc):_Float16path: all witness values and the full binary16identity sweep pass (unchanged behaviour);
FLOAT16_IS_CONVERTIBLE: allwitness values and the full identity sweep pass, including the
double-rounding witness - the fallback now matches the native path
exactly.
reproduces all failures; the new
bs_construct_SUITE:fp16/1cases failon it and pass with the fix.
New test cases cover exact subnormals (previously corrupted), a
sticky-bits rounding witness, an exact tie (round-to-even), the
double-rounding witness, and a construct∘match identity sweep over every
finite binary16 value.