RFR: 8342103: C2 compiler support for Float16 type and associated operations

Tue Nov 19 19:57:13 UTC 2024

On Mon, 14 Oct 2024 11:40:01 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/21490#issuecomment-2482867818)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 instructions generally operate over floating point registers, therefore compiler injectes 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
> 6. Auto-vectorization of newly supported scalar operations.
> 7. X86 and AARCH64 backend implementation for all supported intrinsics.
> 9. Functional and Performance validation tests.
> 
> **Missing Pieces:-**
> **-  AARCH64 Backend.**
> 
> Kindly review and share your feedback.
> 
> Best Regards,
> Jatin

Can we add the JMH micro benchmark that you added recently for FP16 as well ? or has it intentionally not been included?

Hi Jatin, could you also include the idealization tests here - test/hotspot/jtreg/compiler/c2/irTests/MulHFNodeIdealizationTests.java and ConvF2HFIdealizationTests.java in this PR?

src/hotspot/share/opto/addnode.hpp line 445:

> 443:   MinHFNode(Node* in1, Node* in2) : MaxNode(in1, in2) {}
> 444:   virtual int Opcode() const;
> 445:   virtual const Type *add_ring(const Type*, const Type*) const;

`Type* ` ? to align with the style used in the constructor.

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

> 750: //=============================================================================
> 751: //------------------------------Value------------------------------------------
> 752: // An DivFNode divides its inputs.  The third input is a Control input, used to

DivHFNode?

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

> 773:   }
> 774: 
> 775:   if( t2 == TypeH::ONE )

should if condition be styled as - `if (<expression not beginning/ending with spaces>)` ? or is this to align with already existing float routines?

src/hotspot/share/opto/mulnode.cpp line 558:

> 556: }
> 557: 
> 558: // Compute the product type of two double ranges into this node.

of two *half-float* ranges?

src/hotspot/share/opto/node.cpp line 1600:

> 1598: 
> 1599: // Get a half float constant from a ConstNode.
> 1600: // Returns the constant if it is a float ConstNode

half float ConstNode?

src/hotspot/share/opto/type.hpp line 530:

> 528: };
> 529: 
> 530: // Class of Float-Constant Types.

Class of Half-float constant Types?

test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java line 122:

> 120:     public static final String VECTOR_SIZE_64  = VECTOR_SIZE + "64";
> 121: 
> 122:     private static final String TYPE_BYTE   = "byte";

Hi Jatin, why have these changes been made? The PrintIdeal output still prints the vector size of the node in this format - `#vectord<S,4>`. This test - `test/hotspot/jtreg/compiler/vectorization/TestFloatConversionsVectorNaN.java` was failing due to this mismatch ..

test/jdk/java/lang/Float/FP16ReductionOperations.java line 25:

> 23: 
> 24: /*
> 25:  * @test

Hi Jatin, is there any reason why these have been kept under the `Float` folder and not a separate `Float16` folder?

test/jdk/jdk/incubator/vector/ScalarFloat16OperationsTest.java line 334:

> 332: 
> 333:     @Test(dataProvider = "ternaryOpProvider")
> 334:     public static void minTest(Object input1, Object input2, Object input3) {

`fmaTest` ?

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

PR Comment: https://git.openjdk.org/jdk/pull/21490#issuecomment-2411381410
PR Comment: https://git.openjdk.org/jdk/pull/21490#issuecomment-2411607884
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1848152453
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1848128281
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1848135401
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1848112186
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1848195342
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1847971311
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1803209988
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1802767337
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1848388981