On Wed, 29 Jan 2025 00:36:54 GMT, Sandhya Viswanathan
<sviswanat...@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
