On Mon, 14 Oct 2024 11:40:01 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/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
