RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v13]

Kim Barrett kbarrett at openjdk.org
Wed Oct 30 03:44:23 UTC 2024


On Tue, 29 Oct 2024 20:22:03 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86 Port_](https://openjdk.org/jeps/479).
>> 
>> This is the summary of JEP 479:
>>> Remove the source code and build support for the Windows 32-bit x86 port. This port was [deprecated for removal in JDK 21](https://openjdk.org/jeps/449) with the express intent to remove it in a future release.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix 32/64-bit confusion in comment in VirtualMachineImpl.c

I didn't spend much time looking for more places that could use updating.  We can
always do more cleaning up later if more are found.

make/scripts/compare.sh line 79:

> 77: 
> 78: if [ "$OPENJDK_TARGET_OS" = "windows" ]; then
> 79:   DIS_DIFF_FILTER="$SED -r \

This is now being defined for windows-aarch64 too, when it previously wasn't.  Is that intentional?

make/scripts/compare.sh line 1457:

> 1455:         THIS_SEC_BIN="$THIS_SEC_DIR/sec-bin.zip"
> 1456:         if [ "$OPENJDK_TARGET_OS" = "windows" ]; then
> 1457:             JGSS_WINDOWS_BIN="jgss-windows-x64-bin.zip"

This is now being defined for windows-aarch64 too, when it previously wasn't.  That seems wrong,
given the "x64" suffix.

src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp line 1433:

> 1431:   // instructions that are SP relative. After the jni call we switch to FP
> 1432:   // relative instructions instead of re-adjusting the stack on windows.
> 1433:   // ****************************************************************************

I think it might be better to keep this comment.  It might be helpful information for someone who
needs to touch this code between now and when we remove all 32bit x86 support (which might
be soonish, but not immediate).  And this comment will go away when that change happens.

src/hotspot/os/windows/os_windows.cpp line 2592:

> 2590:   ctx->Rdx = (DWORD)0;             // remainder
> 2591:   // Continue the execution
> 2592: #else

Maybe retain `#else` clause as an `#error`?

src/hotspot/share/adlc/main.cpp line 494:

> 492: }
> 493: 
> 494: #if !defined(_WIN32) || defined(_WIN64)

Removing the conditionalization is fine for this change.  But see also 
https://bugs.openjdk.org/browse/JDK-8342639
I've added a note there that this change removed the conditionalization.

src/java.base/windows/native/libjava/gdefs_md.h line 31:

> 29: 
> 30: #include <stddef.h>
> 31: #ifndef _WIN64

I suspect the unix/windows gdefs_md.h files could be eliminated, and just make gdefs.h use portable
headers.  That can be done as a separate cleanup.

src/java.base/windows/native/libjava/jlong_md.h line 66:

> 64: #define jlong_zero_init ((jlong) 0)
> 65: 
> 66: #ifdef _WIN64

After this change I think the differences between the unix and windows variants of this file are trivial
and could be resolved in favor of moving everything directly into jlong.h.  Though note there are some
places in java.desktop that currently directly include jlong_md.h.  This can be done as a separate cleanup.

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

Changes requested by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21744#pullrequestreview-2403283976
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1821670031
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1821671116
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1821680493
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1821684248
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1821796117
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1821806395
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1821809957


More information about the build-dev mailing list