On Tue, 7 Mar 2023 02:53:48 GMT, Vladimir Kozlov <k...@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: > > Address review comments We should not allow JIT compilation of `Float.float16ToFloat` and `Float.floatToFloat16` Java methods as we do for other math methods: [abstractInterpreter.hpp#L144](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/interpreter/abstractInterpreter.hpp#L144) What happens with @jatin-bhateja test is `Float` class is still not loaded when we trigger compilation for `Foo::main` method with `-Xcomp` flag. C2 generates Uncommon trap at the very start of `main()` and code is deoptimized and goes to Interpreter which uses intrinsics. C1 on other hand generates calls to `Float.float16ToFloat` and `Float.floatToFloat16` in compiled `Foo::main` method and then compiles `Float.float16ToFloat` and `Float.floatToFloat16` which are called. The fix ix simple: +++ b/src/hotspot/share/interpreter/abstractInterpreter.hpp @@ -159,6 +159,8 @@ class AbstractInterpreter: AllStatic { case vmIntrinsics::_dexp : // fall thru case vmIntrinsics::_fmaD : // fall thru case vmIntrinsics::_fmaF : // fall thru + case vmIntrinsics::_floatToFloat16 : // fall thru + case vmIntrinsics::_float16ToFloat : // fall thru case vmIntrinsics::_Continuation_doYield : // fall thru return false; test now produce the same result: $ java -Xint Foo 2143297536 32257 $ java -Xcomp -XX:-TieredCompilation Foo 2143297536 32257 $ java -Xcomp -XX:TieredStopAtLevel=1 Foo 2143297536 32257 ------------- PR: https://git.openjdk.org/jdk/pull/12869