RFR: 8342769: HotSpot Windows/gcc port is broken [v9]

[David Holmes]

On Fri, 22 Nov 2024 09:17:53 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> Several areas in HotSpot are broken in the gcc port. These, with the exception of 1 rather big oversight within SharedRuntime::frem and SharedRuntime::drem, are all minor correctness issues within the code. These mostly can be fixed with simple changes to the code. Note that I am not sure whether the SharedRuntime::frem and SharedRuntime::drem fix is correct. It may be that they can be removed entirely
>
> Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 16 additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into hotspot
>  - Reintroduce ifdef removed in the nuking of the Windows 32 bit x86 port in sharedRuntimeRem.cpp
>  - fmod_win64 in sharedRuntime.cpp
>  - Should only declare fmod_win64 on Windows/ARM64 sharedRuntime.hpp
>  - fmod_win64 in src/hotspot/share/runtime/sharedRuntime.hpp
>    
>    Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
>  - No spaces in test_os_windows.cpp
>  - No spaces in test/hotspot/gtest/runtime/test_os.cpp
>    
>    Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
>  - Revise comment in sharedRuntime.hpp
>  - Revise comment in sharedRuntime.cpp
>  - size_t brace initialization instead of unsigned literals in test_os_windows.cpp
>  - ... and 6 more: https://git.openjdk.org/jdk/compare/857311cb...6cf1c83a

So for the sharedRuntime changes, IIUC what was previously only needed for Windows x64 is now only needed for Windows Aarch64 - correct?

src/hotspot/os/windows/sharedRuntimeRem.cpp line 28:

> 26: #include "runtime/sharedRuntime.hpp"
> 27: 
> 28: #ifdef _M_ARM64

As this is ARM64 only now, the comment block at line 34 needs updating

src/hotspot/os/windows/sharedRuntimeRem.cpp line 42:

> 40: static const double one = 1.0, Zero[] = { 0.0, -0.0, };
> 41: 
> 42: double SharedRuntime::fmod_win64(double x, double y)

Wouldn't `fmod_winAarch64` or `fmod_winARM64` make more sense given this is now only used for Aarch64?

src/hotspot/share/runtime/sharedRuntime.cpp line 287:

> 285: 
> 286: 
> 287: #ifdef _M_ARM64

If this is ARM64 only then the `#if !defined(X86)` at line 294 is not needed.

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21627#pullrequestreview-2477063681
PR Review Comment: https://git.openjdk.org/jdk/pull/21627#discussion_r1868566991
PR Review Comment: https://git.openjdk.org/jdk/pull/21627#discussion_r1868569795
PR Review Comment: https://git.openjdk.org/jdk/pull/21627#discussion_r1868568059