RFR: 8266550: C2: mirror TypeOopPtr/TypeInstPtr/TypeAryPtr with TypeKlassPtr/TypeInstKlassPtr/TypeAryKlassPtr [v6]

[Tobias Hartmann]

On Tue, 13 Jul 2021 12:04:30 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> This is some refactoring in another attempt to fix JDK-6312651
>> (Compiler should only use verified interface types for
>> optimization). Rather than propose the patch from:
>> 
>> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-May/033803.html
>> 
>> as a single big patch. I've been working on splitting it. The plan is
>> to have this and another refactoring patch that include no change to
>> the way interfaces are handled as preparation. Then only, in a third
>> patch, interface support along the lines of the patch I proposed 2
>> years ago would be introduces.
>> 
>> This patch changes the class hierarchy of types that C2 uses and
>> introduces TypeInstKlassPtr/TypeAryKlassPtr that mirror
>> TypeInstPtr/TypeAryPtr. The motivation for this is that a single:
>> 
>> ciKlass* _klass;
>> 
>> is no longer sufficient to describe a type (a set of interfaces must
>> also be carried around). That's not possible with TypeKlassPtr.
>> 
>> The meet methods for TypeInstPtr/TypeInstKlassPtr and
>> TypeAryPtr/TypeAryKlassPtr are largely similar. I moved the most
>> complicated logic in helper methods:
>> 
>> meet_instptr() and meet_aryptr()
>> 
>> (Thanks to Vladimir I for testing the refactoring patches)
>
> 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 10 additional commits since the last revision:
> 
>  - review
>  - Merge branch 'master' into JDK-8266550
>  - IR framework fix
>  - dump tweaks
>  - Merge branch 'master' into JDK-8266550
>  - Merge branch 'master' into JDK-8266550
>  - missing casts to NotNull
>  - Merge branch 'master' into JDK-8266550
>  - missing cases with base() == KlassPtr
>  - instklassptr/aryklassptr

Hard to review but looks good to me. Just added some minor comments/questions.

src/hotspot/share/opto/library_call.cpp line 5971:

> 5969:   ciInstanceKlass* instklass_AESCrypt = klass_AESCrypt->as_instance_klass();
> 5970:   const TypeKlassPtr* aklass = TypeKlassPtr::make(instklass_AESCrypt);
> 5971:   const TypeOopPtr* xtype = aklass->as_instance_type()->cast_to_ptr_type(TypePtr::NotNull);

I assume you need this because you've changed `as_instance_type` to no longer cast to `TypePtr::NotNull`. Why is that and what about other usages of `as_instance_type`?

src/hotspot/share/opto/node.cpp line 1790:

> 1788:     const TypeInstPtr  *toop = t->isa_instptr();
> 1789:     const TypeKlassPtr *tkls = t->isa_klassptr();
> 1790:     if (toop) {

Why did you remove the interface case?

src/hotspot/share/opto/type.cpp line 5568:

> 5566: //-----------------------------as_instance_type--------------------------------
> 5567: // Corresponding type for an instance of the given class.
> 5568: // It will be NotNull, and exact if and only if the klass type is exact.

Is this comment still valid?

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

> 930:     SUBTYPE,
> 931:     NOT_SUBTYPE,
> 932:     LCA

Please add comments explaining the meaning of these.

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

> 1884: 
> 1885: inline const TypeKlassPtr *Type::is_klassptr() const {
> 1886:   assert( _base >= KlassPtr && _base <= AryKlassPtr, "Not a klass pointer" );

The extra whitespaces should be removed.

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

> 1893: 
> 1894: inline const TypeInstKlassPtr *Type::is_instklassptr() const {
> 1895:   assert( _base == InstKlassPtr, "Not a klass pointer" );

The extra whitespaces should be removed.

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

> 1902: 
> 1903: inline const TypeAryKlassPtr *Type::is_aryklassptr() const {
> 1904:   assert( _base == AryKlassPtr, "Not a klass pointer" );

The extra whitespaces should be removed.

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

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