RFR: 8329194: Cleanup Type::cmp definition and usage [v2]

[Jasmine Karthikeyan]

On Mon, 15 Apr 2024 07:27:59 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Rename to Type::equals, changes from code review
>
> Overall a good improvement and makes it more intuitive. A few comments.

Thanks for the comments @chhagedorn and @eme64! I've pushed a commit that should address the points brought up in review, and renamed the function to `Type::equals`.

> src/hotspot/share/opto/node.cpp line 3014:
> 
>> 3012: }
>> 3013: bool TypeNode::cmp(const Node& n) const {
>> 3014:   return Type::cmp(_type, ((TypeNode&)n)._type);
> 
> While at it, you can replace the cast with `as_Type()`:
> Suggestion:
> 
>   return Type::cmp(_type, (n.as_Type()->_type);

Thanks for the suggestion! I've made this change.

> src/hotspot/share/opto/type.cpp line 444:
> 
>> 442:   };
>> 443: 
>> 444:   _shared_type_dict = new (shared_type_arena) Dict((CmpKey) type_cmp, (Hash) Type::uhash, shared_type_arena, 128);
> 
> Couldn't you make `type_cmp` a `CmpKey` instead of `auto` and then remove the cast here? On the other hand, you could probably also just remove the `CmpKey` cast here but it might be more explicit when also changing the lambda type.

I think that's a good idea, I've changed the type of the lambda to be `CmpKey` to make it clearer.

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

PR Comment: https://git.openjdk.org/jdk/pull/18533#issuecomment-2058112906
PR Review Comment: https://git.openjdk.org/jdk/pull/18533#discussion_r1566624863
PR Review Comment: https://git.openjdk.org/jdk/pull/18533#discussion_r1566625398