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

[Roland Westrelin]

On Mon, 19 Jul 2021 07:59:02 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

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

Thanks for reviewing this.

> 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`?

I looked at other use of as_instance_type and found that there was not need for the cast to not null. As to why, it makes little sense to me that as_instance_type() would return NotNull. It feels error prone.

> 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?

I brought it back as this doesn't really matter for this PR. The next change in the series removes the klass() accessor and that creates all sort of issues. In some cases where the use of klass() seems to have little value (like here, after all interfaces can't be trusted), I removed the code using the klass() rather than try to fix it.

> 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?

Nice catch. Fixed.

> src/hotspot/share/opto/type.hpp line 932:
> 
>> 930:     SUBTYPE,
>> 931:     NOT_SUBTYPE,
>> 932:     LCA
> 
> Please add comments explaining the meaning of these.

I added a comment. Let me know if it's ok.

> 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.

done.

> 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.

done.

> 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.

done.

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

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