On Wed, 11 Oct 2023 17:57:27 GMT, Vladimir Ivanov <vliva...@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
