RFR: 8275201: C2: hide klass() accessor from TypeOopPtr and typeKlassPtr subclasses [v2]

Vladimir Ivanov vlivanov at openjdk.java.net
Fri May 6 18:30:48 UTC 2022


On Mon, 28 Feb 2022 14:28:40 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Outside the type system code itself, c2 usually assumes that a
>> TypeOopPtr or a TypeKlassPtr's java type is fully represented by its
>> klass(). To have proper support for interfaces, that can't be true as
>> a type needs to be represented by an instance class and a set of
>> interfaces. This patch hides the klass() accessor of
>> TypeOopPtr/TypeKlassPtr and reworks c2 code that relies on it in a way
>> that makes that code suitable for proper interface support in a
>> subsequent change. This patch doesn't add proper interface support yet
>> and is mostly refactoring. "Mostly" because there are cases where the
>> previous logic would use a ciKlass but the new one works with a
>> TypeKlassPtr/TypeInstPtr which carries the ciKlass and whether the
>> klass is exact or not. That extra bit of information can sometimes
>> help and so could result in slightly different decisions.
>> 
>> To remove the klass() accessors, the new logic either relies on:
>> 
>> - new methods of TypeKlassPtr/TypeInstPtr. For instance, instead of:
>> toop->klass()->is_subtype_of(other_toop->klass())
>> the new code is:
>> toop->is_java_subtype_of(other_toop)
>> 
>> - variants of the klass() accessors for narrower cases like
>>   TypeInstPtr::instance_klass() (returns _klass except if _klass is an
>>   interface in which case it returns Object),
>>   TypeOopPtr::unloaded_klass() (returns _klass but only when the klass
>>   is unloaed), TypeOopPtr::exact_klass() (returns _klass but only when
>>   the type is exact).
>> 
>> When I tested this patch, for most changes in this patch, I had the
>> previous logic, the new logic and a check that verified that they
>> return the same result. I ran as much testing as I could that way.
>
> Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - review
>  - Merge branch 'master' into JDK-8275201
>  - Merge branch 'master' into JDK-8275201
>  - build fix
>  - Merge branch 'master' into JDK-8275201
>  - whitespaces
>  - remove klass accessor

Looks very good!

Sorry for the delay with the review.

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

Marked as reviewed by vlivanov (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6717


More information about the hotspot-compiler-dev mailing list