RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter [v3]

Fei Yang fyang at openjdk.org
Wed Mar 8 01:57:31 UTC 2023


On Tue, 7 Mar 2023 21:38:38 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in Interpreter and C1 compiler to produce the same results as C2 intrinsics on x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java methods were implemented originally.
>> 
>> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls to runtime stubs which use the same HW instructions as C2 intrinsics. Only for 64-bit x64 because 32-bit x86 stub does not work: result is passed through FPU register and NaN values become different from C2 intrinsic. This runtime stub is only used to calculate constant values during C2 compilation and can be skipped.
>> 
>> I added new tests based on Tobias's `TestAll.java` And copied `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to make sure code is compiled by C1 or C2. I modified `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
>> 
>> Tested tier1-5, Xcomp, stress
>
> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Do not allow JIT compilation of Float.float16ToFloat and Float.floatToFloat16

Hi, Thanks for handling linux-riscv64 at the same time.
Bad news is that we witnessed test failures when running following test with QEMU (no riscv hardware available with Zfhmin extension for now): test/hotspot/jtreg/compiler/intrinsics/float16/TestAllFloat16ToFloat.java


Exception in thread "main" java.lang.RuntimeException: Inconsistent result for Float.floatToFloat16(NaN/7fc00000): 7e00 != fc01
        at TestAllFloat16ToFloat.verify(TestAllFloat16ToFloat.java:62)
        at TestAllFloat16ToFloat.run(TestAllFloat16ToFloat.java:72)
        at TestAllFloat16ToFloat.main(TestAllFloat16ToFloat.java:94)


It looks like there is a problem when handling NaNs with fcvt.h.s/fmv.x.h and fmv.h.x/fcvt.s.h instructions at the bottom.
It's also possible to be an issue of QEMU as well. It would take quite a while to diagnose. But I don't want this to block this PR.
So I would prefer removing support of this feature for this port and adding back once this is resolved. And I have prepared a patch for that purpose. See attachement.
[12869-revert-riscv.txt](https://github.com/openjdk/jdk/files/10915606/12869-revert-riscv.txt)

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

PR: https://git.openjdk.org/jdk/pull/12869


More information about the hotspot-compiler-dev mailing list