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

[Andrew Haley]

On Wed, 11 Oct 2023 17:57:27 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Andrew Haley has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add TestDenormalDouble.java
>
> src/hotspot/os/bsd/os_bsd.cpp line 977:
> 
>> 975: 
>> 976: void *os::Bsd::dlopen_helper(const char *filename, int mode) {
>> 977: #if defined(__GNUC__)
> 
> What's the intention of limiting it to GCC on BSD? `os_linux.cpp` doesn't have it.
> 
> Also, since 3rd party libraries are the root of the problem, does the information about toolchain JDK is built with help in any way?

Good point. I think the reason it's GCC-conditional is that a previous version used GCC extensions. I'll take it out.

> src/hotspot/os/bsd/os_bsd.cpp line 1001:
> 
>> 999:     static const volatile double thresh
>> 1000:       = jdouble_cast(0x0000000000000003); // 0x0.0000000000003p-1022;
>> 1001:     if (unity + thresh == unity || -unity - thresh == -unity) {
> 
> The check is duplicated in 4 places (twice in os_bsd.cpp and twice in os_linux.cpp). Maybe extract it into some central shared location? Also, unity and thresh constants are duplicated in `stubGenerator_x86_64.cpp`. Why don't you place everything on `StubRoutines` instead?

OK.

> test/hotspot/jtreg/compiler/floatingpoint/libfast-math.c line 39:
> 
>> 37: static void __attribute__((constructor)) set_flush_to_zero(void) {
>> 38: 
>> 39: #if defined(__x86_64__) && defined(SSE)
> 
> x86-64 implies SSE2, doesn't it?

I can't remember. I'll take it out.

> test/hotspot/jtreg/compiler/floatingpoint/libfast-math.c line 51:
> 
>> 49:   {  __asm__ __volatile__ ("msr fpcr, %0" : : "r" (fpcr)); }
>> 50:   /* Flush to zero, round to nearest, IEEE exceptions disabled.  */
>> 51:   _FPU_SETCW (_FPU_FPCR_FZ);
> 
> Macros make it a bit harder to decipher what happens there. At least, I'd suggest to change formatting around `_FPU_SETCW`. At first glance, it looks like it is part of the macro definition.

OK, I see.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1356948049
PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1356921508
PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1356919522
PR Review Comment: https://git.openjdk.org/jdk/pull/10661#discussion_r1356953789