RFR: 8329194: Cleanup Type::cmp definition and usage

Jasmine Karthikeyan jkarthikeyan at openjdk.org
Tue Apr 16 01:15:06 UTC 2024


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

>> Hi all, this patch aims to cleanup `Type::cmp` by changing it from returning a `0` when types are equal and `1` when they are not, to it returning a boolean denoting equality. This makes its usages at various callsites more intuitive. However, as it is passed to the type dictionary as a comparator, a lambda is needed to map the boolean to a comparison value. 
>> 
>> I was also considering changing the name to `Type::equals` as it's not really returning a comparison value anymore, but I felt it would be too similar to `Type::eq`. If this would be preferred though, I can change it.
>> 
>> Tier 1 testing passes on my machine. Reviews and thoughts would be appreciated!
>
> src/hotspot/share/opto/type.hpp line 219:
> 
>> 217:   static const Type *make(enum TYPES);
>> 218:   // Test for equivalence of types
>> 219:   static int cmp( const Type *const t1, const Type *const t2 );
> 
> Was it required to remove the `consts` here?

Since the second `const` is referring to the `t1/t2` it means that the parameters can't be modified, which IIRC in the header would be functionally the same as not having the additional const. I changed it in type.cpp as well because looking at other parts of the code `const Type* val` is used a lot more often than `const Type* const val` for arguments.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18533#discussion_r1566592983


More information about the hotspot-compiler-dev mailing list