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

[Xin Liu]

On Tue, 10 May 2022 08:28:15 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 pull request now contains eight commits:
> 
>  - Merge branch 'master' into JDK-8275201
>  - 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

hi, Roland, 

I don't mean that you should remove it. The hotspot code style uses "avoid" instead of "forbid". 
I am curious what we gain when we use this feature.
in some scenarios, we are operating subclass pointers. eg. in flatten_alias_type()
const TypeInstPtr *to = tj->isa_instptr(); 

we can save static_cast<> for them. It has better for expressiveness, doesn't it?

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

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