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

[Vladimir Ivanov]

On Wed, 8 Mar 2023 20:55:29 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> src/hotspot/share/opto/convertnode.cpp line 171:
>> 
>>> 169:   if (t == Type::TOP) return Type::TOP;
>>> 170:   if (t == Type::FLOAT) return TypeInt::SHORT;
>>> 171:   if (StubRoutines::f2hf() == nullptr) return bottom_type();
>> 
>> What's the purpose of this check? My understanding is ConvF2HF/ConvHF2F require intrinsification and on platforms where stubs are absent, intrinsification is disabled.
>
> This code is optimization: use stub to calculate constant value during compilation instead of generating HW  instruction in compiled code.  It is not required to have this stub for intensification to work - `ConvF2HFNode` will be processed normally and will use intrinsics code (HW instruction) defined in .ad file.
> These stubs are used only here, not in C1 and not in Interpreter. As consequence these stubs implementation is optional and I implemented them only on x64. That is why I have this check.
> I debated to not have them at all to not confuse people but they did improve performance a little.

Thanks for the clarifications. Now it makes much more sense.

Still, the mix of `StubRoutines::f2hf()` and `SharedRuntime::f2hf()` looks a bit confusing.

What if you move the wrapper to `StubRoutines` class instead? (`JRT_LEAF` et al stuff looks redundant here. Also, even though there are other arithmetic operations declared on `StubRoutines`, they provide default implementations universally available across all platforms. `f2hf` case is different since it exposes a platform-specific stub and its availability is limited.)  

Or encapsulate the constant folding logic (along with the guard) into `SharedRuntime` and return `Type*` (instead of int/float scalar).

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

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