RFR: 8342103: C2 compiler support for Float16 type and associated scalar operations [v16]

[Emanuel Peter]

On Thu, 30 Jan 2025 11:03:43 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Hi All,
>> 
>> This patch adds C2 compiler support for various Float16 operations added by [PR#22128](https://github.com/openjdk/jdk/pull/22128)
>> 
>> Following is the summary of changes included with this patch:-
>> 
>> 1. Detection of various Float16 operations through inline expansion or pattern folding idealizations.
>> 2. Float16 operations like add, sub, mul, div, max, and min are inferred through pattern folding idealization.
>> 3. Float16 SQRT and FMA operation are inferred through inline expansion and their corresponding entry points are defined in the newly added Float16Math class.
>>       -    These intrinsics receive unwrapped short arguments encoding IEEE 754 binary16 values.
>> 5. New specialized IR nodes for Float16 operations, associated idealizations, and constant folding routines.
>> 6. New Ideal type for constant and non-constant Float16 IR nodes. Please refer to [FAQs ](https://github.com/openjdk/jdk/pull/22754#issuecomment-2543982577)for more details.
>> 7. Since Float16 uses short as its storage type, hence raw FP16 values are always loaded into general purpose register, but FP16 ISA generally operates over floating point registers, thus the compiler injects reinterpretation IR before and after Float16 operation nodes to move short value to floating point register and vice versa.
>> 8. New idealization routines to optimize redundant reinterpretation chains. HF2S + S2HF = HF
>> 9. X86  backend implementation for all supported intrinsics.
>> 10. Functional and Performance validation tests.
>> 
>> Kindly review the patch and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update test/micro/org/openjdk/bench/jdk/incubator/vector/Float16OperationsBenchmark.java
>   
>   Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>

Ooops, I found a few more details. But the C++ VM changes look really good now.

The Java changes I leave to @PaulSandoz

src/hotspot/share/opto/convertnode.cpp line 971:

> 969:       return true;
> 970:     default:
> 971:       return false;

Does this cover all cases? What about `FmaHF`?

src/hotspot/share/opto/convertnode.hpp line 234:

> 232: class ReinterpretHF2SNode : public Node {
> 233:   public:
> 234:   ReinterpretHF2SNode(Node* in1) : Node(0, in1) {}

Suggestion:

  ReinterpretHF2SNode(Node* in1) : Node(nullptr, in1) {}

src/hotspot/share/opto/divnode.cpp line 866:

> 864: // Dividing by self is 1.
> 865: // IF the divisor is 1, we are an identity on the dividend.
> 866: Node* DivHFNode::Identity(PhaseGVN* phase) {

Remove line with `isA_Copy`.

src/hotspot/share/opto/type.cpp line 1106:

> 1104:     if (_base == FloatBot || _base == FloatTop) return FLOAT;
> 1105:     if (_base == HalfFloatTop || _base == HalfFloatBot) return Type::BOTTOM;
> 1106:     if (_base == DoubleTop || _base == DoubleBot) return Type::BOTTOM;

If you already fixing the style, you should use curly braces as I said above ;)

src/hotspot/share/opto/type.cpp line 1472:

> 1470: //------------------------------meet-------------------------------------------
> 1471: // Compute the MEET of two types.  It returns a new Type object.
> 1472: const Type* TypeH::xmeet(const Type* t) const {

Suggestion:

//------------------------------xmeet-------------------------------------------
// Compute the MEET of two types.  It returns a new Type object.
const Type* TypeH::xmeet(const Type* t) const {

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

PR Review: https://git.openjdk.org/jdk/pull/22754#pullrequestreview-2592155651
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1940766035
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1940763403
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1940766624
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1940771256
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1940771662