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.

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

Floating-point arithmetic on 386/x87 (32-bit) #785

Merged
merged 3 commits into from
Jun 20, 2022
Merged

Floating-point arithmetic on 386/x87 (32-bit) #785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55d858f
cmake: Use SSE2 on 32-bit x86
chfast Apr 16, 2021
6922011
test: Reenable previously disabled testfloat tests (for 32-bit)
chfast Apr 16, 2021
98bcb85
test: Disable failing ui32_to_f64/min floattest
chfast Apr 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .codespell-whitelist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
mut
crate
arithmetics
8 changes: 8 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ if(CMAKE_CXX_COMPILER_ID MATCHES Clang)
endif()
endif()

# This currently is for checking 32-bit mode on x86_64.
# TODO: potentially include x86, i386, i686 here
if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 AND CMAKE_SIZEOF_VOID_P EQUAL 4)
Copy link
Member

Choose a reason for hiding this comment

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

x86_64?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe it's different on true 386, but I have not way of checking.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sure it should be i386 on any non-64-bit x86 (at some point they tried to retrofit ia32 though).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This can fixed if someone would use such system (I doubt this will ever happen). At this point I'm not sure what name CMake uses because this is not specified except they mention uname.

Copy link
Member

Choose a reason for hiding this comment

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

Some search suggest that it may return the following: x86, i386, i686.

See https://boringssl.googlesource.com/boringssl/+/refs/heads/2272/CMakeLists.txt and http://www.autoscool-clermont.com/blum/wp-content/themes/matrix/inc/kirki/assets/js/vendor/codemirror/mode/cmake/index.html (which has set(X86_ALIASES x86 i386 i686 x86_64 amd64)).

Copy link
Member

@axic axic Jun 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

How about sorting it out with:

Suggested change
if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 AND CMAKE_SIZEOF_VOID_P EQUAL 4)
# This is currently is for checking 32-bit mode on x86_64.
# TODO: potentially include x86, i386, i686 here
if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 AND CMAKE_SIZEOF_VOID_P EQUAL 4)

Copy link
Member

Choose a reason for hiding this comment

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

Also just realised that basically this check here enables "32-bit mode support" as opposed to supporting x86. Maybe just documenting that too is good.

Copy link
Member

@axic axic Jun 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@chfast please add this and then we can merge

Also see #785 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please make the required changes yourself.

# On 32-bit opt-in for floating-point implementation with SSE2.
# See the "x87 FPU" section in the README for explanation.
add_compile_options(-msse2 -mfpmath=sse)
endif()

# An option to enable assertions in non-Debug build types.
# Disabling assertions in Debug build type has no effect (assertions are still enabled).
option(ENABLE_ASSERTIONS "Enable NDEBUG based assertions" OFF)
Expand Down
22 changes: 22 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,28 @@ In the GNU C Library the rounding mode can be controlled via the [`fesetround` a

If strict compliance is sought with WebAssembly, then the user of Fizzy must ensure to keep the default rounding mode.

### x87 FPU

On the 32-bit Intel i386 architecture an [x87](https://en.wikipedia.org/wiki/X87)-compatible FPU is used by default to perform floating-point operations.
The FPU is claimed to be IEEE-754 compliant, but there is one gotcha. The operations are executed with so-called *internal precision* and the results are rounded to the target precision at the end [[1]](#1).
By default, the precision is set to [80-bit extended precision](https://en.wikipedia.org/wiki/Extended_precision) (except for VC++ runtime [[2]](#2)).
Unfortunately, this causes problems for 64-bit double precision operations (`f64.add`, `f64.sub`, `f64.mul`, `f64.div`) as the results may be different from when computed with double precision directly.

The FPU precision can be dynamically modified by using compiler intrinsics [[1]](#1), but this has similar issues to controlling the rounding mode and there exists no C/C++ standard way of doing so.

We decided against fighting the x87 FPU quirks, because floating-point operations were not the top priorities.
Instead of creating manual workarounds, a reasonable solution is to opt-in for using SSE2 instructions to implement WebAssembly floating-point instructions,
not only for 64-bit (where it is the default), but 32-bit builds as well. This means for strict WebAssembly compliance the SSE2 instruction set is required.

This is controlled by the [`-msse2 -mfpmath=sse`][x86-options] compiler options, and one can always override to experiment with the x87 FPU.
Worth mentioning that the [`-mpc64`][x86-options] GCC compiler option is supposed to set the FPU to 64-bit double precision mode, but for an unknown reason this is not working.

See also:
1. <a id=1></a>[Deterministic cross-platform floating point arithmetics](http://christian-seiler.de/projekte/fpmath/)
2. <a id=2></a>[Intermediate Floating-Point Precision](https://randomascii.wordpress.com/2012/03/21/intermediate-floating-point-precision/)
3. [Verified Compilation of Floating-Point Computations](https://hal.inria.fr/hal-00862689v3)

[x86-options]: https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html

## Development

Expand Down
11 changes: 3 additions & 8 deletions test/testfloat/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,9 @@ if(TESTFLOAT_GEN)
list(APPEND IGNORE_LIST ui64_to_f64/min)
endif()

if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 AND CMAKE_SIZEOF_VOID_P EQUAL 4)
# TODO: On i386 (32-bit) there are bugs in the arithmetic instructions
# with default rounding.
list(APPEND IGNORE_LIST
f64_add/near_even
f64_sub/near_even
f64_mul/near_even
f64_div/near_even)
if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 AND CMAKE_SIZEOF_VOID_P EQUAL 4 AND CMAKE_CXX_COMPILER_ID MATCHES GNU)
# TODO: GCC 32-bit i386 build produces -0 for input 0, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100119.
Copy link
Member

Choose a reason for hiding this comment

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

Supposedly fixed in gcc 12.

Copy link
Member

Choose a reason for hiding this comment

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

Still doesn't work:

138/183 Test #103: fizzy/testfloat/ui32_to_f64/min ......................***Failed    0.01 sec
FAILURE: 8000000000000000 <- 00000000
         0000000000000000 (expected)
FAILURE: 8000000000000000 <- 00000000
         0000000000000000 (expected)
FAILURE: 8000000000000000 <- 00000000
         0000000000000000 (expected)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The bug is indeed fixed in GCC 12.1. Are you sure we are using GCC 12? Maybe there is some other bug lurking here.

Copy link
Member

Choose a reason for hiding this comment

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

Ran it on the CI, which I believed had 12, but now on a second look it is 11.

list(APPEND IGNORE_LIST ui32_to_f64/min)
endif()

set(ROUNDING_MODES near_even minMag min max)
Expand Down