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

[Emanuel Peter]

On Mon, 25 Nov 2024 07:17:33 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Testpoints for new value transforms + code cleanups
>
> src/hotspot/share/opto/connode.cpp line 49:
> 
>> 47:   switch( t->basic_type() ) {
>> 48:   case T_INT:         return new ConINode( t->is_int() );
>> 49:   case T_SHORT:       return new ConHNode( t->is_half_float_constant() );
> 
> That will be quite confusing.... don't you think?

I mean do we need this? We already have `ConHNode::make` below...?

> src/hotspot/share/opto/divnode.cpp line 765:
> 
>> 763:   if((t1 == bot) || (t2 == bot) ||
>> 764:      (t1 == Type::BOTTOM) || (t2 == Type::BOTTOM))
>> 765:     return bot;
> 
> Suggestion:
> 
>   if((t1 == bot) || (t2 == bot) ||
>      (t1 == Type::BOTTOM) || (t2 == Type::BOTTOM)) {
>     return bot;
>   }
> 
> Again: please always use brackets.

Apply the same below.

> src/hotspot/share/opto/divnode.cpp line 804:
> 
>> 802: 
>> 803: //------------------------------Idealize---------------------------------------
>> 804: Node *DivHFNode::Ideal(PhaseGVN* phase, bool can_reshape) {
> 
> Suggestion:
> 
> Node* DivHFNode::Ideal(PhaseGVN* phase, bool can_reshape) {

Ok, and please add brackets for all the ifs below!

> src/hotspot/share/opto/divnode.cpp line 805:
> 
>> 803: //------------------------------Idealize---------------------------------------
>> 804: Node *DivHFNode::Ideal(PhaseGVN* phase, bool can_reshape) {
>> 805:   if (in(0) && remove_dead_region(phase, can_reshape))  return this;
> 
> Suggestion:
> 
>   if (in(0) != nullptr && remove_dead_region(phase, can_reshape)) { return this; }
> 
> brackets for if and no implicit null checks please!

https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md

`Do not use ints or pointers as (implicit) booleans with &&, ||, if, while. Instead, compare explicitly, i.e. if (x != 0) or if (ptr != nullptr), etc.`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1855959810
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1855985811
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1855995743
PR Review Comment: https://git.openjdk.org/jdk/pull/21490#discussion_r1855999519