[lworld+fp16] RFR: 8336406: Add support for FP16 binary operations [v2]
Bhavana Kilambi
bkilambi at openjdk.org
Tue Aug 6 09:02:44 UTC 2024
On Wed, 31 Jul 2024 10:11:29 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:
>> Bhavana Kilambi has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add MulHF::Ideal() and MulHF Idealization tests
>
> src/hotspot/share/opto/mulnode.hpp line 149:
>
>> 147: // Multiply 2 half floats
>> 148: class MulHFNode : public MulFNode {
>> 149: public:
>
> While its good to use MulFNode constant folding (Value) routines and generic Ideal transforms, we should skip any specific Idealization transformations which may end up generating purely [floating point IR](https://github.com/openjdk/valhalla/blob/lworld%2Bfp16/src/hotspot/share/opto/mulnode.cpp#L547)
Hi @jatin-bhateja , I have made the changes to MulHF::Ideal() as suggested. This avoids generation of pure Floating point IR during Ideal optimizations of MulHF. I have checked other binary operations as well to make sure we are not generating any pure floating point IR during optimization. I can see one in DivFNode::Ideal() - https://github.com/openjdk/valhalla/blob/e45febcdb3f6bd8ca139f6c12126d52968c7c2f9/src/hotspot/share/opto/divnode.cpp#L749 which generates MulFNode when dividing with 2. I tried this specific pattern for half float and it generates correct answer but performs the entire optimization in float IR (the input FP16 value is converted to FP32). For now, I don't think we can change this as the constant value 2 is interpreted as a TypeF floating point value and we do not have the corresponding type for Float16 at the moment. The resultant output is of course correct and thus I have not made any changes to it.
Other binary operations seem to be fine to be performed on Float16 values as well.
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/1175#discussion_r1705175485
More information about the valhalla-dev
mailing list