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

[Vladimir Ivanov]

On Thu, 1 Jul 2021 14:15:36 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 eight additional commits since the last revision:
> 
>  - 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

Overall, it looks good.

Testing results (hs-tier1 - hs-tier6) are fine. 

Just a couple of minor comments follow.

src/hotspot/share/opto/callnode.cpp line 380:

> 378:       st->print(" %s%d]=#Ptr" INTPTR_FORMAT,msg,i,p2i(t->isa_oopptr()->const_oop()));
> 379:       break;
> 380:     case Type::KlassPtr:

Why do you remove the case? I'd expect to see 2 new cases added: 

   case Type::KlassPtr:
+  case Type::AryKlassPtr:
+  case Type::InstKlassPtr:

src/hotspot/share/opto/compile.cpp line 1456:

> 1454:     case Type::AryPtr:   // do not distinguish arrays at all
> 1455:     case Type::InstPtr:  tj = TypeInstPtr::BOTTOM;  break;
> 1456:     case Type::AryKlassPtr:

Similar situation here. Why don't you leave `Type::KlassPtr` intact and just extend it to `Type::AryKlassPtr` and `Type::InstKlassPtr` cases?

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

Marked as reviewed by vlivanov (Reviewer).

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