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

[Vladimir Kozlov]

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

----------------------------------------------------------------------
On Wed, 8 Mar 2023 19:38:56 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove RISC-V port code for float16 intrinsics
>
> 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.

> src/hotspot/share/opto/convertnode.cpp line 244:
> 
>> 242: 
>> 243:   const TypeInt *ti = t->is_int();
>> 244:   if (ti->is_con()) {
> 
> I find it confusing that `ConvHF2FNode::Value()` has `is_con()` check, but `ConvF2HFNode::Value()`doesn't. I'd prefer to see both implementations unified.

It follows the same pattern as other nodes here: `ConvF2INode::Value()` vs `ConvI2FNode::Value()`.
If you want to change it we need to do that in separate RFE for all methods here.
But I don't think we need to do that because  Float/Double does not have range values as Integer types.
Float have only 3 types of value: FloatTop, FloatBot, FloatCon. So we don't need to check for constant if checked for TOP and BOT.  For Integer we need to check `bool is_con() const { return _lo==_hi; }`.

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

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