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

[Jatin Bhateja]

On Wed, 29 Jan 2025 00:36:54 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Updating typos in comments
>
> Some more minor comments below. Rest of the PR looks good to me.

Hi @sviswa7 , your comments have been addressed.

> src/hotspot/share/opto/subnode.hpp line 549:
> 
>> 547:   SqrtHFNode(Compile* C, Node* c, Node* in1) : Node(c, in1) {
>> 548:     init_flags(Flag_is_expensive);
>> 549:     C->add_expensive_node(this);
> 
> Do we need to set SqrtHF as expensive node? It translates to a single instruction.

Its latency is around 15 cycles. Thus, GVN commoning, which leads to its placement onto a frequently executed control path, may be costly. In addition, it is also marked as an expensive node by ADLC to prevent rematerialization by the allocator.

> src/hotspot/share/opto/type.hpp line 2031:
> 
>> 2029: 
>> 2030: inline const TypeH* Type::is_half_float_constant() const {
>> 2031:   assert( _base == HalfFloatCon, "Not a Float" );
> 
> Should be "Not a HalfFloat" here.

Fixed this typo error.

> test/hotspot/jtreg/compiler/c2/irTests/TestFloat16ScalarOperations.java line 27:
> 
>> 25: /**
>> 26: * @test
>> 27: * @bug 8308363 8336406
> 
> This should now refer to bug 8342103.

Test was added on lworld+fp16 branch and appropriately reflects the JBS entry. Same, applies to above two comments.

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

PR Comment: https://git.openjdk.org/jdk/pull/22754#issuecomment-2620812050
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1933322657
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1933322637
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1933322576