RFR: 8339783: Implementation of JEP 479: Remove the Windows 32-bit x86 Port
Aleksey Shipilev
shade at openjdk.org
Mon Oct 28 19:37:24 UTC 2024
On Mon, 28 Oct 2024 18:15:38 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.
>
> src/hotspot/cpu/x86/interpreterRT_x86_32.cpp line 47:
>
>> 45: #ifdef AMD64
>> 46: #ifdef _WIN64
>> 47: // FIXME: This is weird. How can we ever have _WIN64 for 32-bit code? I wonder what was meant. /ihse
>
> I think this piece of code will never get compiled and should be removed, and just the `#else` clause kept, but I guess some code archaeology is in place to figure out how and why this was added in the first place.
I think this is a copy-paste error from [JDK-8199809](https://bugs.openjdk.org/browse/JDK-8199809): the code from `interpreterRT_x86_64.cpp` (where `WIN64` makes sense) was copy-pasted here in `interpreterRT_x86_32.cpp`. In fact, `AMD64` in `interpreterRT_x86_64.cpp` makes no sense as well. I'll clean it up: [JDK-8343167](https://bugs.openjdk.org/browse/JDK-8343167).
> src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp line 1478:
>
>> 1476: int frame_complete = ((intptr_t)__ pc()) - start;
>> 1477:
>> 1478: // FIXME: The logic below do not apply anymore. Should we change anything? /ihse
>
> This file is now Linux only, so we should be able to remove any Windows special code. Someone with better knowledge about the product needs to confirm that the comment is indeed correct, and that this was only needed on Windows.
Nah, leave it as is. Let's not regress native stubs unnecessarily, and this whole file would be gone after we deprecate 32-bit port completely.
> src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp line 1714:
>
>> 1712: __ restore_cpu_control_state_after_jni(noreg);
>> 1713:
>> 1714: // FIXME: The logic below do not apply anymore. Should we change anything? /ihse
>
> Same here as above.
Same reply as above :)
> src/hotspot/cpu/x86/x86_32.ad line 3715:
>
>> 3713: %}
>> 3714:
>> 3715: // FIXME: The logic below do not apply anymore. Should we change anything? /ihse
>
> Here too we don't need Windows-specific support, since this is Linux only. But I need confirmation that the comment is correct so this code is really just Windows-specific.
It looks like it is a dusty corner case. But the same logic as above applies: let's not touch it, and instead wait for it to go away with the remaining bits of 32-bit x86 port. I see `eRegP_no_EBP` is used for safepoint polls, so if we are wrong about the scope of this, rewriting these match rules to just `eRegP` might introduce surprising regressions.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1819606745
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1819611536
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1819612008
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1819617067
More information about the nio-dev
mailing list