RFR: 8339783: Implementation of JEP 479: Remove the Windows 32-bit x86 Port

David Holmes dholmes at openjdk.org
Tue Oct 29 09:51:10 UTC 2024


On Mon, 28 Oct 2024 18:09:41 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.

Hotspot changes look good. May be some further cleanup possible. A couple of queries.

Thanks

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

> 2613:   Thread* t = Thread::current_or_null_safe();
> 2614: 
> 2615: #if defined(_M_AMD64)

The check for LP64 on line 2622 below seems redundant now

src/hotspot/os_cpu/windows_x86/os_windows_x86.cpp line 87:

> 85:     volatile Thread* wrapperthread = thread;
> 86: 
> 87:     if (os::win32::get_thread_ptr_offset() == 0) {

I think `os::win32::get_thread_ptr_offset` is not needed now and  ./os_cpu/windows_x86/assembler_windows_x86.cpp looks like it can be deleted.

src/hotspot/share/adlc/adlc.hpp line 49:

> 47: #define strdup _strdup
> 48: 
> 49: #ifndef _INTPTR_T_DEFINED

This seems unnecessary these days.

src/hotspot/share/prims/jvm.cpp line 381:

> 379:   {
> 380: #undef CSIZE
> 381: #if defined(_LP64)

Windows is actually LLP64 programming model not LP64. Does Windows x64 define _LP64 or is that something we do in our build?

src/hotspot/share/prims/nativeLookup.cpp line 350:

> 348:   if (entry != nullptr) return entry;
> 349: 
> 350:   // 3) Try JNI short style without os prefix/suffix

Please update comment as there is no os prefix/suffix now

src/hotspot/share/utilities/globalDefinitions_visCPP.hpp line 55:

> 53: #error unsupported platform
> 54: #endif
> 55: 

Does Windows Aarch64 define _LP64?

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

PR Review: https://git.openjdk.org/jdk/pull/21744#pullrequestreview-2401144686
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1820386150
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1820407428
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1820429621
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1820433973
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1820436924
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1820441749


More information about the serviceability-dev mailing list