RFR: 8345219: C2: x86_64 should not go to interpreter stubs for NaNs handling [v3]

[Emanuel Peter]

On Wed, 4 Dec 2024 09:42:59 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Found this while cleaning up x86_32 code for removal.
>> 
>> In our current code there is a block added by [JDK-8076373](https://bugs.openjdk.org/browse/JDK-8076373):
>> https://github.com/openjdk/jdk/blob/3b21a298c29d88720f6bfb2dc1f3305b6a3db307/src/hotspot/share/compiler/compileBroker.cpp#L1451-L1473
>> 
>> Ostensibly, that block is for x86_32 handling of signalling NaNs -- x87 FPU has a peculiarity with them. See other funky bugs we seen with it: [JDK-8285985](https://bugs.openjdk.org/browse/JDK-8285985), [JDK-8293991](https://bugs.openjdk.org/browse/JDK-8293991).
>> 
>> But the way current block is coded, it is enabled for X86 wholesale, which also means x86_64! In fact, it is likely even worse on x86_64, because the related "fast" entries are generated only for x86_32:
>> https://github.com/openjdk/jdk/blob/3b21a298c29d88720f6bfb2dc1f3305b6a3db307/src/hotspot/share/interpreter/templateInterpreterGenerator.cpp#L493-L502
>> 
>> This can be solved by checking `IA32` instead of `X86`. This block would be gone completely once we remove x86_32 port. Meanwhile, we can make it right by x86_64, and make eventual x86_32 removal less confusing. This issue seems to only affect the compilation of native methods, while most of the hot code is riding on compiler intrinsics. I'll put performance data in comments.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 server fastdebug, `all`
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Disable IR tests on IA32

I just launched testing, I don't think @vnkozlov did that already. Better to hold off until RDP1 / JDK25 fork anyway.

test/hotspot/jtreg/compiler/c2/irTests/TestFPConversion.java line 33:

> 31:  * @summary Test that code generation for FP conversion works as intended
> 32:  * @library /test/lib /
> 33:  * @requires os.arch != "x86" & os.arch != "i386"

It would have been preferrable to add this to the IR rule, so the test still runs elsewhere.
What about `aarch64`?

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

PR Review: https://git.openjdk.org/jdk/pull/22446#pullrequestreview-2478784069
PR Review Comment: https://git.openjdk.org/jdk/pull/22446#discussion_r1869595147