RFR: 8295159: DSO created with -ffast-math breaks Java floating-point arithmetic [v10]

[David Holmes]

On Mon, 16 Oct 2023 16:22:27 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> A bug in GCC causes shared libraries linked with -ffast-math to disable denormal arithmetic. This breaks Java's floating-point semantics.
>> 
>> The bug is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55522
>> 
>> One solution is to save and restore the floating-point control word around System.loadLibrary(). This isn't perfect, because some shared library might load another shared library at runtime, but it's a lot better than what we do now. 
>> 
>> However, this fix is not complete. `dlopen()` is called from many places in the JDK. I guess the best thing to do is find and wrap them all. I'd like to hear people's opinions.
>
> Andrew Haley has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Comments only.
>  - Review feedback

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 5169:

> 5167:       // Perform a little arithmetic to make sure that denormal
> 5168:       // numbers are handled correctly, i.e. that the "Denormals Are
> 5169:       // Zeros" flag has not been set.

I don't understand what this part is doing. I thought it was simply checking so you could log/warn if the unexpected mode was detected. But it seems to cause MXCSR to not be restored when there is an issue, where I would expect you would always want to restore to overwrite the invalid mode the JNI call made. ??

src/hotspot/os/linux/os_linux.cpp line 1817:

> 1815:   fenv_t default_fenv;
> 1816:   int rtn = fegetenv(&default_fenv);
> 1817:   assert(rtn == 0, "fegetnv must succeed");

typo: fetgetnv -> fegetenv

src/hotspot/share/runtime/stubRoutines.cpp line 330:

> 328:   // _small_denormal is the smallest denormal number that has two bits
> 329:   // set. _large_denormal is a number such that, when _small_denormal
> 330:   // is added it it, must be rounded according to the mode. These two

s/it it, must/to it, it must/

test/hotspot/jtreg/compiler/floatingpoint/TestDenormalDouble.java line 42:

> 40:         for (double x = lastDouble * 2; x <= 0x1.0p1022; x *= 2) {
> 41:             if (x != x || x <= lastDouble) {
> 42:                 throw new AssertionError("TEST FAILED: " + x);

Tests don't normally use AssertionError like this, just a plain Error or RuntimeException.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1361432675
PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1361412874
PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1361427020
PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1361428895