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

Emanuel Peter epeter at openjdk.org
Tue Apr 16 11:00:01 UTC 2024


On Tue, 16 Apr 2024 02:21:24 GMT, Jasmine Karthikeyan <jkarthikeyan 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!
>
> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rename to Type::equals, changes from code review

src/hotspot/share/opto/type.hpp line 160:

> 158: 
> 159:   static int uhash( const Type *const t );
> 160:   // Structural equality check.  Assumes that equals() has already compared

Is this even correct? Because we use `eq` inside `equals`, (before your patch, we used `eq` inside `cmp`).
Should this not rather mean that we should have already done the `==`?
What do you think?

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

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


More information about the hotspot-compiler-dev mailing list