On Mon, 27 Jan 2025 08:35:44 GMT, Jatin Bhateja <jbhat...@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:
> 
>   Copyright header fix

Some more minor comments.

src/hotspot/share/opto/addnode.cpp line 1546:

> 1544: 
> 1545:   // As per IEEE 754 specification, floating point comparison consider 
> +ve and -ve
> 1546:   // zeros as equals. Thus, performing signed integral comparison for 
> max value

Should be "min value detection".

src/hotspot/share/opto/addnode.cpp line 1624:

> 1622:   // As per IEEE 754 specification, floating point comparison consider 
> +ve and -ve
> 1623:   // zeros as equals. Thus, performing signed integral comparison for 
> min value
> 1624:   // detection.

Should be "max value detection".

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

> 846:   // If the dividend is a constant zero
> 847:   // Note: if t1 and t2 are zero then result is NaN (JVMS page 213)
> 848:   // Test TypeF::ZERO is not sufficient as it could be negative zero

Comment should be TypeH:ZERO is not sufficient

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

PR Review: https://git.openjdk.org/jdk/pull/22754#pullrequestreview-2576801437
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1931347845
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1931347430
PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1931376932

Reply via email to