On Mon, 16 Oct 2023 16:22:27 GMT, Andrew Haley <a...@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
