On Mon, 28 Oct 2024 18:09:41 GMT, Magnus Ihse Bursie <i...@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
